Skip to content

Packaging top level artifacts#525

Open
pacostas wants to merge 20 commits intopaketo-buildpacks:mainfrom
pacostas:packaging-top-level-artifacts-multi-arch
Open

Packaging top level artifacts#525
pacostas wants to merge 20 commits intopaketo-buildpacks:mainfrom
pacostas:packaging-top-level-artifacts-multi-arch

Conversation

@pacostas
Copy link
Member

@pacostas pacostas commented Mar 19, 2026

Summary

This PR, adds functionality for packaging top level artifacts if they are included on the include-scripts attribute on the buildpack/extension.toml . E.g. the a Readme.md file, in case of multi-arch will end up under the /os/arch directories and in case of single arch will end up on the root directory.

This PR:

  • Succesfully copies the files that exist on the include files
  • Alters the dependencies metadata of the test data of the buildpacks in order to be consistent and properly test code base based on correct data [unrelated to the scope of the PR]
  • Adds multi-arch tests for the extension [unrelated to the scope of the PR]
  • A little bit of code reusability between the buildpack and the extension
  • Removes the language packaging extension tests as we do not support this scenario [unrelated to the scope of the PR]
  • Deprecating extension functionality, for packaging offline extensions [unrelated to the scope of the PR]
  • Adds an extra flag on the package.sh script to select for which platforms we want to build the jam cli [unrelated to the scope of the PR]

Use Cases

Checklist

  • I have viewed, signed, and submitted the Contributor License Agreement.
  • I have linked issue(s) that this PR should close using keywords or the Github UI (See docs)
  • I have added an integration test, if necessary.
  • I have reviewed the styleguide for guidance on my code quality.
  • I'm happy with the commit history on this PR (I have rebased/squashed as needed).

@pacostas pacostas requested review from a team as code owners March 19, 2026 11:12
@pacostas pacostas added the semver:minor A change requiring a minor version bump label Mar 19, 2026
@pacostas pacostas force-pushed the packaging-top-level-artifacts-multi-arch branch from f67b715 to 5715410 Compare March 20, 2026 12:32
Description: "This extension installs the appropriate nodejs runtime via dnf",
Keywords: []string{"keyword1", "keyword2"},
Licenses: []cargo.ConfigExtensionLicense{
Licenses: []cargo.ConfigBuildpackLicense{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why this changed? Everything else here is referencing "extension".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unrelated to the scope of the PR, is just some code re-usability. The structure about the license of the extension, is no different from the buildpack's one.

for dependencyIndex, dependency := range config.Metadata.Dependencies {
if isMultiArch {
if dependency.OS == "" || dependency.Arch == "" {
return fmt.Errorf("dependency %s has no OS or Arch", dependency.ID)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this indicate a binary that's not specific to an arch? i.e. it should be included for all arches? Or is that handled differently?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the buildpack is multi-arch (which means has more than one target) and there is not os and arch on the dependency it will fail to package it. Otherwise, it will add it under the dependencies directory under the correct platform directory, based on the os and arch that is specified for this dependency.

Copy link
Contributor

@dmikusa dmikusa Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so this means that there is no such thing as a "noarch" dependency, right? It means if we want to have a dependency for all the architectures, then we need to list it multiple times in buildpack.toml.

I don't think a lot of our buildpacks would need this, and I'm thinking ahead a little bit as I want to move the java buildpacks to use this tooling eventually. For the Java buildpacks, we have JAR dependencies that are "noarch", it doesn't matter. Ex: here. In this case, what we want is to have the tooling add that to all the architectures.

Rather than error here, I think we should follow the logic from RFC 0059...

Note: If os or arch are not given it should be assumed that the dependency is OS or Architecture agnostic and is compatible to run on any given OS or Architecture.

Which I believe would mean if we're in a multi-arch buildpack (i.e. we have targets), then we should copy a dependency without OS or architecture into each of the OS & architectures.

Copy link
Member Author

@pacostas pacostas Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so this means that there is no such thing as a "noarch" dependency, right? It means if we want to have a dependency for all the architectures, then we need to list it multiple times in buildpack.toml.

What you describe is correct, this is the current behavior of jam but only for the offline mode, because on the offline mode we download the dependencies, otherwise we dont touch the buildpack.toml. Still I have to validate that. I'll verify and I'll let you know.
Ok, I didnt know there is a spec for that! Thanks, I'll patch also the offline mode ASAP :)

@pacostas pacostas force-pushed the packaging-top-level-artifacts-multi-arch branch from 60d83e5 to 621e872 Compare March 20, 2026 17:20
@pacostas pacostas force-pushed the packaging-top-level-artifacts-multi-arch branch from 621e872 to 4106906 Compare March 20, 2026 17:58
@pacostas
Copy link
Member Author

@benjaminguttmann-avtq
I added this to the buildpack.toml of the php-dist

[metadata]
  include-files = [
  "buildpack.toml",
  "config/default.ini",
  "config/buildpack.ini",
  "linux/amd64/bin/build",
  "linux/amd64/bin/detect",
  "linux/amd64/bin/run",
  "linux/arm64/bin/build",
  "linux/arm64/bin/detect",
  "linux/arm64/bin/run",
  ]

  pre-package = "./scripts/build.sh --target linux/amd64 --target linux/arm64"

and without any other change on the php-dist repo, i managed to include the files under the config directory for all the architectures.

Feel free to validate, based on the implementation of jam from the currect PR.

@pacostas
Copy link
Member Author

Sorry for the huge PR, as it ended up a rabbit hole, due to the implementation of multi-arch needed some fixes, on the extension side and on the buildpack side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver:minor A change requiring a minor version bump

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants