Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modify carton package to work with targets #323

Merged
merged 1 commit into from
Apr 4, 2024
Merged

Conversation

dmikusa
Copy link
Contributor

@dmikusa dmikusa commented Apr 4, 2024

Summary

We recently added targets to the include-files list in buildpack.toml. The referenced files get packaged up into the buildpack. What we added were some files prefixed with linux/<arch>/.... These files get put into that location within the generated buildpack directory. This is to work with a new version of pack that packages multi-arch images. It packages up whatever is under each architecture directory into the image specific to that architecture.

What's happened is that we now have non-architecture dependent files like the README and LICENSE which are not being included in the images. These files also need to be prefixed with the path for each architecture. Instead of putting a lot of duplicate entries into the include-files list, what this PR does is to take any file not prefixed with linux/ and duplicate it for all of the detected architectures.

The PR determines the list of target architectures by looking at the include-files list and pulling out all of the distinct linux/<arch>'s that it sees. This isn't the best implementation but it's all we can do with the v1.x branch of libpak because the target metadata is not present in that version of libcnb. In libpak 2.x, we'll be able to read the target list directly from buildpack.toml and use that instead, which is a better option.

Use Cases

Multi-arch support.

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).

We recently added targets to the `include-files` list in `buildpack.toml`. The referenced files get packaged up into the buildpack. What we added were some files prefixed with `linux/<arch>/...`. These files get put into that location within the generated buildpack directory. This is to work with a new version of pack that packages multi-arch images. It packages up whatever is under each architecture directory into the image specific to that architecture.

What's happened is that we now have non-architecture dependent files like the README and LICENSE which are not being included in the images. These files also need to be prefixed with the path for each architecture. Instead of putting a lot of duplicate entries into the `include-files` list, what this PR does is to take any file not prefixed with `linux/` and duplicate it for all of the detected architectures.

The PR determines the list of target architectures by looking at the `include-files` list and pulling out all of the distinct `linux/<arch>`'s that it sees. This isn't the best implementation but it's all we can do with the v1.x branch of libpak because the target metadata is not present in that version of libcnb. In libpak 2.x, we'll be able to read the target list directly from `buildpack.toml` and use that instead, which is a better option.

Signed-off-by: Daniel Mikusa <dan@mikusa.com>
@dmikusa dmikusa added type:bug A general bug semver:patch A change requiring a patch version bump labels Apr 4, 2024
@dmikusa dmikusa requested a review from a team as a code owner April 4, 2024 18:38
@dmikusa
Copy link
Contributor Author

dmikusa commented Apr 4, 2024

To test, I ran this code on a buildpack with no targets. It produced this build, which is correct.

Paketo Buildpack for Apache Tomcat 1.2.3
  https://github.com/paketo-buildpacks/apache-tomcat
  Creating package in ./out
  Pre-package with scripts/build.sh
    Adding LICENSE
    Adding NOTICE
    Adding README.md
    Adding bin/build
    Adding bin/detect
    Adding bin/helper
    Adding bin/main
    Adding buildpack.toml
    Adding resources/context.xml
    Adding resources/logging.properties
    Adding resources/server.xml
    Adding resources/web.xml

I ran it against a buildpack with targets and it produced this output, which is also correct. Notice how it fans out the non-arch specific files into each arch-specific folder.

Paketo Buildpack for Apache Tomcat 1.2.3
  https://github.com/paketo-buildpacks/apache-tomcat
  Creating package in ./out
  Pre-package with scripts/build.sh
    Adding buildpack.toml
    Adding linux/amd64/LICENSE
    Adding linux/amd64/NOTICE
    Adding linux/amd64/README.md
    Adding linux/amd64/bin/build
    Adding linux/amd64/bin/detect
    Adding linux/amd64/bin/helper
    Adding linux/amd64/bin/main
    Adding linux/amd64/resources/context.xml
    Adding linux/amd64/resources/logging.properties
    Adding linux/amd64/resources/server.xml
    Adding linux/amd64/resources/web.xml
    Adding linux/arm64/LICENSE
    Adding linux/arm64/NOTICE
    Adding linux/arm64/README.md
    Adding linux/arm64/bin/build
    Adding linux/arm64/bin/detect
    Adding linux/arm64/bin/helper
    Adding linux/arm64/bin/main
    Adding linux/arm64/resources/context.xml
    Adding linux/arm64/resources/logging.properties
    Adding linux/arm64/resources/server.xml
    Adding linux/arm64/resources/web.xml

@dmikusa
Copy link
Contributor Author

dmikusa commented Apr 4, 2024

I built an image from the output and it's including all the files now:

Screenshot 2024-04-04 at 2 41 43 PM

@anthonydahanne
Copy link
Member

All good, here's how I tested:

go install -ldflags="-s -w" github.com/paketo-buildpacks/libpak/cmd/create-package@package-arches
cd ~/paketo-buildpacks/apache-tomcat
create-package --source . --destination ~/buildpack  --version 9.9.9
Paketo Buildpack for Apache Tomcat 9.9.9
  https://github.com/paketo-buildpacks/apache-tomcat
  Creating package in /home/ubuntu/buildpack
  Pre-package with scripts/build.sh
    go: downloading github.com/paketo-buildpacks/libpak v1.69.0
    Adding buildpack.toml
    Adding linux/amd64/LICENSE
    Adding linux/amd64/NOTICE
    Adding linux/amd64/README.md
    Adding linux/amd64/bin/build
    Adding linux/amd64/bin/detect
    Adding linux/amd64/bin/helper
    Adding linux/amd64/bin/main
    Adding linux/amd64/resources/context.xml
    Adding linux/amd64/resources/logging.properties
    Adding linux/amd64/resources/server.xml
    Adding linux/amd64/resources/web.xml
    Adding linux/arm64/LICENSE
    Adding linux/arm64/NOTICE
    Adding linux/arm64/README.md
    Adding linux/arm64/bin/build
    Adding linux/arm64/bin/detect
    Adding linux/arm64/bin/helper
    Adding linux/arm64/bin/main
    Adding linux/arm64/resources/context.xml
    Adding linux/arm64/resources/logging.properties
    Adding linux/arm64/resources/server.xml
    Adding linux/arm64/resources/web.xml

cd ~/buildpack
~/my-exp-pack buildpack package  anthonydahanne/apache-tomcat:dual --publish
Successfully published package anthonydahanne/apache-tomcat:dual and saved to registry

and then I dive'd into it
Screenshot 2024-04-04 at 16 07 43

@anthonydahanne anthonydahanne merged commit e0627f1 into main Apr 4, 2024
8 checks passed
@anthonydahanne anthonydahanne deleted the package-arches branch April 4, 2024 20:08
dmikusa added a commit to paketo-buildpacks/libpak-tools that referenced this pull request Nov 9, 2024
Signed-off-by: Daniel Mikusa <dan@mikusa.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:patch A change requiring a patch version bump type:bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants