dockerfile: allow multiple values for ARG

[tonistiigi]

ENV accepts multiple definitions with a single command, no reason why ARG should behave any differently.

Signed-off-by: Tonis Tiigi tonistiigi@gmail.com

[thaJeztah]

So, if I recall correctly, we didn't do this, because ARG and ENV are very similar, but (unlike ARG) ENV does not enforce = to set a value (similar discussion in moby/moby#17344).

Because of that, the following would be very confusing as they have completely different meanings:

# declare three ARGs: ONE, TWO, and THREE
ARG ONE TWO THREE

# define one ENV (ONE) with value "TWO THREE"
ENV ONE TWO THREE

So, the ARG syntax, by enforcing = to set a value, is more consistent and flexible as it would allow combinations of "declaring" args with and without a value in a single ARG:

ARG ONE=hello

FROM busybox AS stage-foo

# declare three ARGs:
ARG ONE TWO= THREE=world

In the example above;

• arg ONE doesn't have a value set, and in this case inherits its value from the global ARG ONE
• arg TWO is explicitly set to an empty string as default value (quotes are optional, so equivalent to ARG TWO="")
• arg THREE has "world" as default value

Env vars did get some validation to try to prevent some accidental mistakes;

ENV ONE=one TWO two THREE three

Syntax error - can't find = in "TWO". Must be of the form: name=value

But will only produce an error if the first env-var uses an =, so the ARG example above will silently "succeed":

ENV ONE TWO= THREE=world

And produce a single env-var (ONE) with value "TWO= THREE=world"

So not sure what to do with this. I realize ARG and ENV are separate things, but they are so similar that I anticipate users to have them work the same, and trip over the differences.

My ideal would be to use the same syntax for ENV, but changing would be a breaking change, and likely will hit many existing Dockerfiles.

[thaJeztah]

leaving a "request changes" review, as this may need some discussion (see my comment)

[tonistiigi]

If you are annoyed about the ambiguity of ENV we should start to phase out ENV key value. Not to remove it yet but deprecate, remove docs, make sure linters/language-servers warn against it etc. Our own Dockerfiles already don't use it. ARG should not use this, first it is just confusing, and ARG inheritance is an important feature.

[thaJeztah]

After discussing above, and docker/cli#2741, docker/cli#2743 to reduce confusion, I think it's ok to support this

LGTM

[thaJeztah]

We should probably update the legacy builder as well to support the format?

[cpuguy83]

I would argue that allowing ARG key value is nicer than ARG key=value key2=value2 or even ARG key1 key2... and possibly less inconsistent.

The whole reason ENV foo=bar exists is because we wanted to be able to ENV foo=bar baz=quux in a single layer... which is no longer a problem since these don't get layers.

[tonistiigi]

I would argue that allowing ARG key value

ARG key value is just impossible syntax as ARG key= and ARG key have different meanings.

and possibly less inconsistent.

Why? The whole Dockerfile language is modeled after shell, and KEY=value [KEY2=value] is what you do in shell. Also consistent with RUN KEY=value command.

wanted to be able to ENV foo=bar baz=quux in a single layer

Where I've seen it, it has always looked like a logical grouping of related values. That is also why having same for ARG is nice. So you can do ARG TARGETOS TARGETARCH TARGETVARIANT in a single line.