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

Add build argument for debug image #1287

Closed
wants to merge 3 commits into from

Conversation

enumag
Copy link

@enumag enumag commented Apr 4, 2022

I understand that #879 likely won't be solved any time soon but I'd like to at least add a support for an ARG so that I can easily checkout this repository and build a debug image myself like this:

docker build --pull --no-cache 8.1/bullseye/cli --build-arg DEBUG=1

What do you think?

EDIT: Partially reverted. It is now meant to be used together with #1280. This way the condition added in 1280 makes more sense because it can be true without any modifications to the Dockerfile.

@mvorisek
Copy link
Contributor

mvorisek commented Apr 4, 2022

thanks for providing an alternative to #1280

all other templating args are defined in https://github.com/docker-library/php/blob/master/versions.json#L7 and regenerating the DockerFiles is easy, thus I think it should be better to allow variants suffixed by -debug as a debug switch

but this is up too the maintainers to decide, currently, I patched our debug build with #1280 which works well and the condition is 1:1 with the one added in #1278 , so I like my PR slightly more to keep the condition consistency

@enumag
Copy link
Author

enumag commented Apr 4, 2022

In my opinion they should be used together... your PR to prevent stripping and mine to let ppl avoid manually editing the Dockerfiles. It also means that the condition in your PR can be true without any modifications to the Dockerfile.

Extra variants in versions.json are likely not possible because that would essentially be #879.

@enumag
Copy link
Author

enumag commented May 5, 2022

@tianon What do you think about this?

@tianon
Copy link
Member

tianon commented May 5, 2022

I'm definitely not a fan of ARG - it ends up polluting the history of every layer following it in the image metadata. 😞

@enumag
Copy link
Author

enumag commented May 5, 2022

@tianon I see. And what if we put the ARG all the way down before the first layer that will actually use it? It's in fact nearly the last layer so most of them will be untouched.

@yosifkit
Copy link
Member

yosifkit commented May 5, 2022

While that definitely would be a problem, I'd rather not complicate the Dockerfile with parts that make it uncertain how it was built. While build-args are not used in official images and it is unlikely we would ever add them, many new users may not know that. The Dockerfile then becomes uncertain as to what the published images on Docker Hub may contain.

@enumag
Copy link
Author

enumag commented Jun 10, 2022

It's easy to add a comment about that next to the ARG.

@enumag
Copy link
Author

enumag commented Jul 11, 2022

@tianon @yosifkit Okay, if ARG isn't the way to go then how about just having a support in the template to generate debug Dockerfiles? My goal is to have a somewhat easy way to build the debug image I need. Something like:

  1. Clone docker-library/php
  2. Run a script to generate debug Dockerfiles, let's say sh ./apply-templates.sh --debug
  3. Run docker build ... to generate the debug image I need.

I simply want to avoid having to do custom modifications to this repo's sources every time I want a new image. Would something like that be acceptable?

@enumag
Copy link
Author

enumag commented Sep 9, 2022

ping @tianon @yosifkit

@taka-oyama
Copy link

We have been trying to debug a segmentation fault and this would help us alot!

@yosifkit
Copy link
Member

having a support in the template to generate debug Dockerfiles?

I was finally able to come back to this; here is what I've come up with that fits with the current scripts: #1364

@tianon tianon closed this in #1364 Dec 20, 2023
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.

None yet

5 participants