-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
dockerfile: expose TARGETSTAGE as builtin argument #5431
Conversation
This probably needs documentation changes to add this to the list of builtin args? 😅 |
bp := po.buildPlatforms[0] | ||
tp := po.targetPlatform | ||
if target == "" { | ||
target = "default" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if we should have default
if target not set. I think best would be to leave it empty.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty value could cause some breakage maybe. We do name stage default in the --call=targets
output as well. Another option would be to use numbers as we still support them for access, but might be quite confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do name stage default in the
--call=targets
output as well.
Ah I forgot about this subrequest, so yeah default
is good. I was just wondering if default
could be used as TARGETSTAGE
but that would not make sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could see it going either way.
--call=targets
annotates whatever the final build stage is with (default)
, regardless of whether the build stage's name is an empty string or not.
I think having TARGETSTAGE
be default
when the Dockerfile's targeted build stage doesn't have a name might help avoid some potentially surprising behaviors. For example, there could be a scenario where the value of TARGETSTAGE
ends up getting used as a directory name.
On the other hand, a Dockerfile can have a build stage named default
which isn't the final build stage, so applying a default value for TARGETSTAGE
here might be surprising in its own way. But that sounds like a pretty unusual Dockerfile.
I think just going with default
will be applicable to the vast majority of use cases.
@ByteNybbler, any input on the comments above? |
Yes, thank you so much for working on this! I'm very excited to have access to this feature! |
Would setting a value by default (if not set) make it harder to do constructs where the user wants to set a default value? |
Yeah, but if you look at the current |
The only time |
Allows capturing what build stage is currently being built for patterns with better base stage reuse. Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
a379c9c
to
6a31a78
Compare
Updated to read the default stage name from Dockerfile if not passed by the user. |
closes docker/buildx#2646
Allows capturing what build stage is currently being built for patterns with better base stage reuse.