Skip to content

[implementation][scripts] Add config::build function to copy php-dist…#1321

Open
benjaminguttmann-avtq wants to merge 1 commit intopaketo-buildpacks:mainfrom
benjaminguttmann-avtq:add-copy-config-function
Open

[implementation][scripts] Add config::build function to copy php-dist…#1321
benjaminguttmann-avtq wants to merge 1 commit intopaketo-buildpacks:mainfrom
benjaminguttmann-avtq:add-copy-config-function

Conversation

@benjaminguttmann-avtq
Copy link
Copy Markdown
Contributor

… config

Summary

This adds a config::build function to ensure the config needed for php-dist is present under the path /os/arch/config/XY as the build otherwise fails as can be seen in this test run: https://github.com/paketo-buildpacks/php-dist/actions/runs/22901160138/job/66447799817?pr=1025

Adding the function fixes that issue as you can in this run: https://github.com/paketo-buildpacks/php-dist/actions/runs/22906721803/job/66467450176?pr=1026

The noble tests is what I am actually trying to fix in my PR in php-dist, but that does not have anything to do with the build.sh

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

@dmikusa
Copy link
Copy Markdown
Contributor

dmikusa commented Mar 10, 2026

@pacostas @jericop - Do you know what is interpreting the include-files config with the github-config build scripts?

This is a difference I've noticed between this build system and libpak. With libpak, there is support to include files like LICENSE and NOTICE. You don't have to list them a bunch of different times for each arch. You list it once and the tooling knows that you want it in all the arch's.

I'm just trying to understand how this gets handled with github-config.

The issue here with PHP config files is similar. @benjaminguttmann-avtq had them originally listed like config/php.ini in the include-files, but that didn't work. They didn't end up making it into the image. The solution in this PR certainly works, but it would require customization for every different type of file that we want to add in (in this case, it's specific to a nested config/ directory, other buildpacks could have different needs and this script doesn't work then).

I'd rather have a rule like, if you have a file in include-files that is not prefixed with linux/<arch> then that means it gets copied into the resulting buildpack at the right path. That way the php-dist buildpack can list config/php.ini, and my other buildpack can list foo/bar.txt and it doesn't matter. They'll all just be copied in like you'd expect.

@dmikusa
Copy link
Copy Markdown
Contributor

dmikusa commented Mar 10, 2026

For what it's worth, the node-engine buildpack does seem to do something similar, https://github.com/paketo-buildpacks/node-engine/blob/a8e19c0b115e9ffeab77410b11e102b1d795c94e/scripts/build.sh#L108.

It may not be something a lot of buildpacks use, but I think there is some sort of pattern here that would be nice to cover in a generic way.

@pacostas
Copy link
Copy Markdown
Member

Do you know what is interpreting the include-files config with the github-config build scripts?

The include files is used by jam and by pack CLI, to properly first build the .tgz file, which has the following structure

├── buildpack.toml
└── linux
    ├── amd64
    │   └── bin
    │       ├── build -> run
    │       ├── detect -> run
    │       ├── inspector
    │       ├── optimize-memory
    │       └── run
    └── arm64
        └── bin
            ├── build -> run
            ├── detect -> run
            ├── inspector
            ├── optimize-memory
            └── run

and then from the .tgz file pack cli will create the .cnb files, with the help of the include-files and the buildpack.toml in general.

This is a difference I've noticed between this build system and libpak. With libpak, there is support to include files like LICENSE and NOTICE. You don't have to list them a bunch of different times for each arch. You list it once and the tooling knows that you want it in all the arch's.

Do you have an example, as I see that almost all the java repos use the same pattern on include as on the node-engine :

https://github.com/paketo-buildpacks/spring-boot/blob/077ff245e19c7a21e1ec8b2694f1f396c5352334/buildpack.toml#L30-L32

Also the Redme file on the e.g. spring-boot, I believe will not be on the .cnb file but only on the .tgz file (if libpak uses the .tgz as a middle process kind of thing) Can you please confirm that?

I'd rather have a rule like, if you have a file in include-files that is not prefixed with linux/ then that means it gets copied into the resulting buildpack at the right path. That way the php-dist buildpack can list config/php.ini, and my other buildpack can list foo/bar.txt and it doesn't matter. They'll all just be copied in like you'd expect.

Yes, this is possible. In general we can support wildcards for copying files, or we can even add another metadata called excluded files. So for example we could do

include-files=[detect, build]
exclude-files=[optimizer]

This definitely will be more handy when we create a buildpack.toml file, although, this does not mean that what this PR does, wouldnt be necessasry, as jam or pack, does not know how to build the specific dependencies of each buildpacks, unless we could put some instructions on the buildpack.toml on how to build the specific buildpack.

@pacostas
Copy link
Copy Markdown
Member

Also, what we specify on the include-files, is not what exists on the repo, is what exists under the .tgz file, which is what the build script produced. So practically, on the include-files we specify the files after the build script has run, and this makes it a little bit confusing on what we should write on the include-files, as the paths (specifically the levels of the directories) do not exactly match with what the repo has, but what the .tgz has.
e.g. this

  include-files = [
"buildpack.toml", 
"linux/amd64/bin/build",
"linux/amd64/bin/detect",
"linux/arm64/bin/build", 
"linux/arm64/bin/detect", 
]

we would expect to have this output on the .cnb file

./buildpack.toml
./linux/amd64/bin/build
./linux/amd64/bin/detect
./linux/arm64/bin/build
./linux/arm64/bin/detect

but instead we have this e.g. on the amd64 or on the arm64

./buildpack.toml
./bin/build
./bin/detect

@pacostas
Copy link
Copy Markdown
Member

Also previously I mentioned

unless we could put some instructions on the buildpack.toml on how to build the specific buildpack

practically we have that, is the

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

although, I believe the problem is that the ./scripts/build.sh is a generic one and specifically should be different for each repo, or at least the parts that are different on each buildpack script should be on another .sh file and the build script shoud call the specific.sh to execute the different things that each buildpack might need. The output of these two scripts could be a folder that will be included on the .cnb, and any extra files that we might want, will be added through the include-files attribute.

@dmikusa
Copy link
Copy Markdown
Contributor

dmikusa commented Mar 12, 2026

This is a difference I've noticed between this build system and libpak. With libpak, there is support to include files like LICENSE and NOTICE. You don't have to list them a bunch of different times for each arch. You list it once and the tooling knows that you want it in all the arch's.

Do you have an example, as I see that almost all the java repos use the same pattern on include as on the node-engine :

https://github.com/paketo-buildpacks/spring-boot/blob/077ff245e19c7a21e1ec8b2694f1f396c5352334/buildpack.toml#L30-L32

Yes, so that is a good example:

include-files = ["LICENSE", "NOTICE", "README.md", "linux/amd64/bin/build", "linux/amd64/bin/detect", "linux/amd64/bin/main", "linux/amd64/bin/helper", "linux/arm64/bin/build", "linux/arm64/bin/detect", "linux/arm64/bin/main", "linux/arm64/bin/helper", "buildpack.toml", "spring-generations.toml"]

The way this actually reads is linux/amd64/LICENSE, linux/arm64/LICENSE, linux/amd64/NOTICE, linux/arm64/NOTICE, etc...

The create-package tool is look at the declared target archs from buildpack.toml, it sees that these three files have no prefix an says, OK, I will put those files into each of the arch folders that pack reads, so they show up in all of the buildpacks.

So if you look at what's in the buildpack image for spring-boot, you'll see those files:

Screenshot 2026-03-12 at 9 08 11 AM

Also the Redme file on the e.g. spring-boot, I believe will not be on the .cnb file but only on the .tgz file (if libpak uses the .tgz as a middle process kind of thing) Can you please confirm that?

With the libpak-based buildpacks, we do not create the .cnb files or tgz files on the releases. Our GH releases only have source code assets, see https://github.com/paketo-buildpacks/spring-boot/releases/tag/v5.36.0. We only publish images to the registries.

I think the packit based buildpacks have done this because historically tooling they use in CI looked at those assets, but I think we've moved away from that recently (see https://github.com/paketo-buildpacks/nginx/blob/main/integration.json#L6 which now points to images instead of the cnb files).

This definitely will be more handy when we create a buildpack.toml file, although, this does not mean that what this PR does, wouldnt be necessasry, as jam or pack, does not know how to build the specific dependencies of each buildpacks, unless we could put some instructions on the buildpack.toml on how to build the specific buildpack.

With the libpak based buildpacks, our build.sh script only compiles the buildpack. It is very simple, which is something I like as it's easy tos ee what it's doing. The create-package, the tool handles all of the rest of getting the files for the buildpack into the right locations so that we can run pack buildpack package. I like this as well, because it puts the logic of moving files & things around into Go which can be easily tested.

My preference would be to do the same with jam, if we're in agreement to add this support to it. i.e. not add it into the scripts. It's in one place, well tested, and keeps the build script simpler.

@pacostas
Copy link
Copy Markdown
Member

The create-package tool is look at the declared target archs from buildpack.toml, it sees that these three files have no prefix an says, OK, I will put those files into each of the arch folders that pack reads, so they show up in all of the buildpacks.

I see
https://github.com/paketo-buildpacks/libpak-tools/blob/9d257ad659ba33fbe5d2f1a4e54ca48bbadc12af/carton/package.go#L144-L156

With the libpak-based buildpacks, we do not create the .cnb files or tgz files on the releases. Our GH releases only have source code assets, see https://github.com/paketo-buildpacks/spring-boot/releases/tag/v5.36.0. We only publish images to the registries.

That is one difference between jam and create-package, create-package calls pack from inside, while jam does not call pack instead outpus a .tgz and then another external command has to call pack

The create-package, the tool handles all of the rest of getting the files for the buildpack into the right locations so that we can run pack buildpack package. I like this as well, because it puts the logic of moving files & things around into Go which can be easily tested.

I think the only change in order to do that, is to add the function on the jam CLI that will tell, if its not under linux/***/binary_file, then put it under /linux/***/ .

For removing the .cnb files, I think is not necessary, as we already support it, and as far as i remember we also use it for validation between when the release got created and when the release is being pushed to the registry. Also, is easier to explain the /linux/arm64, /linux/amd64, etc on the include files from where they are coming from.

I can create a PR, but I might need some time (like a week) for that as it includes quite some testing. I'm happy to merge this PR though, not to block the python progress.

@dmikusa
Copy link
Copy Markdown
Contributor

dmikusa commented Mar 12, 2026

I think the only change in order to do that, is to add the function on the jam CLI that will tell, if its not under linux//binary_file, then put it under /linux// .

👍

For removing the .cnb files, I think is not necessary, as we already support it, and as far as i remember we also use it for validation between when the release got created and when the release is being pushed to the registry. Also, is easier to explain the /linux/arm64, /linux/amd64, etc on the include files from where they are coming from.

I don't think we need to remove them. They're not hurting anything and like you said may still be used by other things. My only concern is that you mentioned some things possibly being out-of-sync between the image/cnb/tgz files. That would not be great, ideally we'd want those to all be the same.

I can create a PR, but I might need some time (like a week) for that as it includes quite some testing. I'm happy to merge this PR though, not to block the python progress.

Thank you @pacostas. I'm fine with that time frame, but I'll let @benjaminguttmann-avtq decide on how to proceed.

We can merge this to unblock, wait for the jam fix, and then revert this change or we can wait for the jam fix and close this.

Also, @benjaminguttmann-avtq if you have interest in submitting a PR for jam that's an option too. That's an option too.

Thanks all!

@benjaminguttmann-avtq
Copy link
Copy Markdown
Contributor Author

I am happy to wait.. I will focus meanwhile on getting the ruby CNB side of things up-to-date. And yeah, not sure if I understood enough already what needs to be done in jam .. so @pacostas for providing that fix :)

@pacostas
Copy link
Copy Markdown
Member

Related PR paketo-buildpacks/jam#525

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants