Thanks @runcom, yes it was the intent to be able to set a label without value (using an empty string "" as a default value)

@thaJeztah the only issue I see here now is that we allowed this:

LABEL key1 key2=value2

which produces key1="key2=value2"
now this will be key1="", key2="value2" (which is correct to me now)

Hm, good one. I never really liked LABEL key value, but you're right that this would be a breaking change because of that

I'd say it's still a fix to a wrong behavior in LABEL, ppl relying on this bug should change their code possibly

I agree that this is a more user-friendly UX.

I think that at some point in the (build time) ARG option, there was a preference for name= to define an empty "arg", so I'm interested in other maintainers opinion.

I think that at some point in the (build time) ARG option, there was a preference for name= to define an empty "arg", so I'm interested in other maintainers opinion.

oh, that would definitely help here

/cc @tiborvass @duglin ^^

I'll fix the failing tests (as they rely on the old behavior) as soon as we decide what's good to do here

ping @rhatdan since I think he added LABEL support and might be able to say whether its ok to do this, or not.

I'm not a fan of the LABEL name value format, same for ENV, but I believe the intent was to make LABEL and ENV work the same way. Note that LABEL foo="" works just fine.

As you said @duglin I think this has been done to maintain the same behavior as ENV but we're actually allowing docker run with just a label key. Either we edit the doc and add ="" or fix this for LABEL :(

ping @rhatdan since I think he added LABEL support and might be able to say whether its ok to do this, or not.

@rhatdan started the initial implementation, and @ibuildthecloud completed it :-)

I just intended to match the --env behaviour. Although I would prefer name=value. And I would recomend

LABEL NAME=""

LABEL="" could work and that will need only doc update. However, there's currently a difference between docker run allowing us to define a label w/o ="" and the Dockerfile which requires to use ="" to define a label w/o a key.

runcon could you build a patch to prevent use of no ="" or no NAME VALUE

ping @cpuguy83

any thoughts on this? O:)

This is supported for --label on docker run, so it seems to make sense here.

However, there's currently a difference between docker run allowing us to define a label w/o ="" and the Dockerfile which requires to use ="" to define a label w/o a key.

True, it's a difference, but cli is a different UX in general, i.e. -e foo bar will not create two env-vars, or assign foo=bar.

@runcom will this PR break backward compatibility? i.e., will LABEL name value still work?

I do prefer the name=value notation, as it's less ambiguous.

@runcom will this PR break backward compatibility? i.e., will LABEL name value still work?

it will right now..

Makes sense I think

rebased :)

@runcom will this PR break backward compatibility? i.e., will LABEL name value still work?

@thaJeztah this PR break backward compatibility and LABEL name value won't work the same way it was.

maybe it is better to just update the doc, but I really believe this is the correct way to handle LABEL(s) even if you can do LABEL="".

ping @tiborvass @duglin @LK4D4

I looked at the documentation; https://docs.docker.com/engine/userguide/labels-custom-metadata/ and https://docs.docker.com/engine/reference/builder/#label

All examples and the documentation mention that = should be used to set a value, and can be omitted to use an "empty" string. For example this section;

LABEL [<namespace>.]<key>[=<value>] ...

We may want to update this line to more explicitly mention that =<value> is optional (i.e. change it to [=<value>]);

LABEL <key>=<value> <key>=<value> <key>=<value> ...

Reading those; I agree, and I think we should use = to use a value, and if omitted, create an empty label (using "" as value). People currently using LABEL key value are relying on undocumented behavior.

@runcom yeah, however making breaking changes is worse :/
Maybe @ibuildthecloud as active LABEL user can say his word too.

@runcom yeah, however making breaking changes is worse :/

I'm fine updating docs then, really :)

I'm not particularly a fan of this. As @rhatdan pointed out the intended behavior was just to follow the same logic as ENV. Personally I'd rather not have to remember that ENV A B and LABEL A B are parsed differently. I thinks LABEL A="" is perfectly fine.

Oh wait. will LABEL A B now become two keys? If so, then that is way too breaking of a change. But then what will LABEL A B C become?

Ignoring any inconsistency between the docker cmd line and what people see in the Dockerfile, I think

ENV env1 env2
LABEL label1 label2

should be parsed the same way. Which, right now, will produce one env var (env1) with a value of "env2" and one label (label1) with a value of "label2".

Now, to the -l on the command line. I might be doing it wrong but I'm not seeing an inconsistent there. When I do -l foo it doesn't set foo to "", it sets it to the value of the env var "foo", which might be empty but that's a side effect of using the env var.

So, while it might appear that we allow labels and env vars to be specified on the cmd line w/o values, I don't think we do. Instead what we allow people to say is "grab the value from an env var".

Net: I think this might be a doc issue.

@duglin Doing -l foo and having it read and env variable I would think is a bug. That was never intended to be supported. The parsing logic was just supposed to the simple in that if you didn't put a = then the value is ""

@ibuildthecloud that could be - I don't use labels so I'm not sure what the expected behavior should be, but even if we did change it so that it used "" instead of the corresponding env var I think keeping the current Dockerfile syntax/behavior is probably correct. IOW, forcing them to add ="" seems ok to me.

Alright, looks like the majority is for making this a documentation-only change, and keep the current behavior.

@runcom shall we close this one, and create a new, fresh PR?

@thaJeztah I'll update docs to mention that users should add ="" to specify a label w/o a value in Dockerfiles, sounds good?

@runcom sounds good, also https://docs.docker.com/engine/userguide/labels-custom-metadata/#add-labels-to-images should be updated, changing

LABEL [<namespace>.]<key>[=<value>] ...

to

LABEL [<namespace>.]<key>=<value> ...

(I don't think we should explicitly mention <key> <value> without =)