-
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
Add substring processing parameter expansions (POSIX) on the Dockerfile #1772
Comments
@thaJeztah I'm not particularly interested in overcomplicating this but you've been more involved in this code so I leave it for you to decide. |
One simpler alternative if that helps anything would be to instead of following the standard exactly doing just a simpler version that does not do any expansion on (of course, none of that will help if the mentioned overcomplication is more about the scope of the dockerfile frontend itself and other more semantic and conceptual aspects of the change) |
Hmm, yes, I'm a bit on the fence, and also share concerns about things getting overly complicated; we don't want to be implementing a full shell in the Dockerfile for substitutions. I think the current set of supported options make sense for use in Dockerfiles as they concentrate around handling default values, which is useful to either handle "optional" build-args by setting them to a default ( Unless there's very compelling reasons, I'm a bit concerned that we'd be entering "feature creep" territory; |
@thaJeztah I see, that makes sense. Trying to reach parity with an actual shell would be a fool's errand, and I can't think of a case where those expansions would really make that much difference (or in other words, cases where they couldn't be easily controlled / circumvented by the user, like you said). My own case is an example. I already had a Probably most other cases would be inside a So yeah, not really worth it. Gonna take the liberty of closing this. Thanks for your attention, guys. ✌️ |
Thanks for your understanding! I understand your use-case, and can see having the proposed addition would make it a bit nicer to work with. Mostly trying to be a bit conservative for non-critical enhancements. Also, rejecting this feature now doesn't mean we can't revisit in future if important use-cases come up (or, to use the first rule of OSS "no is temporary, yes is forever") |
I just hit a case where I could have really used this. I'm changing my initial skepticism to +1 |
Example case /~https://github.com/tonistiigi/binfmt/blob/7e00ff1e905244f8cdd1013085629a89d4037d3a/Dockerfile#L39-L54 I need an exception to use a different code path for one architecture. All others are the same. Currently I need to copy-paste With this I can write the same code as:
If I need 2 exeptions I could just match the exceptions one by one. There is a proposal for this same issue in Dockerfile-next doc to allow wildcard stage names (eg. like make does) but this looks like a simple first step. |
Hi, this is going to be very useful, I wanted to do something like:
and the build arg so un the prompt you will see 7.6.1 instead of v7.6.1 |
https://docs.docker.com/reference/dockerfile/#:~:text=are%20supported%20in%20a%20pre-release%20version%20of%20Dockerfile%20syntax mentions that variable replacements are now supported in a pre-release version (on the @tonistiigi Is there any ETA for the general release? Or if not: when was this feature added to the |
Hello!
I was having trouble building a Dockerfile that had the following line:
WORKDIR $BUILD_WORKSPACE_BASE/${VERSION#v}
This gave me the error "Unknown desc = missing ':' in substitution" . So I dug up a little and found that the error stemmed from
buildkit/frontend/dockerfile/shell/lex.go
Lines 391 to 399 in 2a6fcae
and seems to be a consequence of the variable expansion parser only treating the
${FOOBAR}
,${FOO:-BAR}
and${FOO?BAR}
cases (and variations).So I was wondering if would it be worth it for me to open a PR adding to that logic the processing of the following expansions as per the POSIX standard:
Figured it wouldn't hurt to file this first to make sure it is a valid proposition.
The text was updated successfully, but these errors were encountered: