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 substring processing parameter expansions (POSIX) on the Dockerfile #1772

Open
mateusmedeiros opened this issue Nov 1, 2020 · 9 comments

Comments

@mateusmedeiros
Copy link

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

default:
return "", errors.Errorf("unsupported modifier (%c) in substitution", modifier)
}
}
return "", errors.Errorf("missing ':' in substitution")
}
func (sw *shellWord) processName() string {
// Read in a name (alphanumeric or _)

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:

${parameter%[word]}
Remove Smallest Suffix Pattern. The word shall be expanded to produce a pattern. The parameter expansion shall then result in parameter, with the smallest portion of the suffix matched by the pattern deleted. If present, word shall not begin with an unquoted '%'.
${parameter%%[word]}
Remove Largest Suffix Pattern. The word shall be expanded to produce a pattern. The parameter expansion shall then result in parameter, with the largest portion of the suffix matched by the pattern deleted.
${parameter#[word]}
Remove Smallest Prefix Pattern. The word shall be expanded to produce a pattern. The parameter expansion shall then result in parameter, with the smallest portion of the prefix matched by the pattern deleted. If present, word shall not begin with an unquoted '#'.
${parameter##[word]}
Remove Largest Prefix Pattern. The word shall be expanded to produce a pattern. The parameter expansion shall then result in parameter, with the largest portion of the prefix matched by the pattern deleted.

Figured it wouldn't hurt to file this first to make sure it is a valid proposition.

@tonistiigi
Copy link
Member

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

@mateusmedeiros
Copy link
Author

@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 [word] (as in ${parameter%[word]}) but instead just uses it literally. And I could also ignore the ## and %% versions and just do the # and % variations.

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

@thaJeztah
Copy link
Member

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 (${FOO:=default} / FOO=${FOO:-default}, or (added in #1180) by producing an error that a required build-arg is not set.

Unless there's very compelling reasons, I'm a bit concerned that we'd be entering "feature creep" territory;
environment variables (ENV) and build-args (ARG) in Dockerfiles can only come from a limited set of resources; from the Dockerfile itself, a parent image, or passed as --build-arg. All of those can usually be controlled by the user performing the build; if it's passed as --build-arg, stripping the prefix could be done in the shell before it's passed (--build-arg=${VERSION#v}

@mateusmedeiros
Copy link
Author

@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 --build-arg VERSION=v1.2.3 and I was trying to not duplicate that in the command line with a --build-arg VERSION=v1.2.3 and a --build-arg VERSION_WITHOUT_V=1.2.3 or something like that. But I simply renamed the directory in a RUN command to a fixed, version-agnostic one before ending the build stage and put that fixed name in the WORKDIR of the next stage.

Probably most other cases would be inside a RUN, where the task of expanding parameters is delegated to the shell.

So yeah, not really worth it.

Gonna take the liberty of closing this. Thanks for your attention, guys. ✌️

@thaJeztah
Copy link
Member

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

@tonistiigi
Copy link
Member

I just hit a case where I could have really used this. I'm changing my initial skepticism to +1

@tonistiigi
Copy link
Member

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 FROM line for all the architectures. I can't just say, "riscv is special, all others are the same".

With this I can write the same code as:

FROM .. AS base
...

FROM .. AS baseriscv64
...


FROM base${TARGETARCH#riscv64} AS build

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.

@sauljabin
Copy link

Hi, this is going to be very useful, I wanted to do something like:

ARG VERSION
RUN echo "${VERSION:1:5}"

and the build arg --build-arg VERSION=v7.6.1

so un the prompt you will see 7.6.1 instead of v7.6.1

@gy-mate
Copy link

gy-mate commented Dec 19, 2024

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 master branch). 🎉

@tonistiigi Is there any ETA for the general release? Or if not: when was this feature added to the main branch?

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

No branches or pull requests

5 participants