Docs LGTM - @fredlf @jamtur01

oh sneaky (copying from the original PR)

I think the docs need more info about side effects, or confirming that there are none - does the instruction run in the container, and so will persist if i docker start it again, or is the container commited, then a new one made based on that image, and the instruciton run in that temporary container..

I think the docs need more info about side effects, or confirming that there are none
Not sure what you mean about side effects. Basically what happens in a docker --commit or docker --import is you end up with an image and a json file containing any additional instructions given on --change line.

It will have the same output as if you had done your changes with a Dockerfile.

docker run -ti fedora /bin/sh
yum -y update
exit
docker commit --change "ENV foo bar" --change CMD /bin/init

Should give the same output as

cat Dockerfile
FROM fedora
RUN yum -y update
ENV foo bar
CMD /bin/init

does the instruction run in the container, and so will persist if i docker start it again, or is the container commited, then a new one made based on that image,
The commit/import and the change happen at the same layer their is no special commands, these are just adding data to the json file about the layer.

mmm, @rhatdan that needs to be explained in the docs. So to confirm, each --change flag creates a new image layer?

ie, it needs to be clear that it does not modify the original container, but actually commits it, then applies each --change in turn in a new layer, tagging the final one...

@SvenDowideit iirc, all --change will be combined together and create only 1 layer ( that's how docker commit --change does it ).

@crosbymichael i think import will still having the same problem as commit because the current path is still passing --change data as URL params as we discussed before right ?

@dqminh @crosbymichael Well the import code is basically a cut and paste of the commit code. What is the problem with passing the --change data in the URL?

@dqminh don't tell me :) it needs to be in the documentation!

ALSO. is there a limitation on the set of Dockerfile instructions that can be done via --change? because there seems to be nothing documenting which instructions can work.

I can't see any tests showing if 2 new layers gets made (which imo would be the expectation most users would have), or there's only one layer, and then that different combinations of --change params get applied as expected.

I also don't see tests that make clear which instructions do and don't get applied - I'm assuming from the references to image JSON (which is new in the docs and would need more info too) that i can't --change RUN apt-get update && apt-get install -yq yum - but you havn't said.

• Supported Dockerfile instructions: CMD, ENTRYPOINT, ENV, EXPOSE, ONBUILD, USER, VOLUME, WORKDIR

RUN/ADD are not supported.

When this patch gets applied, I plan on working on another patch to allow MAINTAINER to be applied. But I want this to get in first.

@SvenDowideit Please give me the wording you would like for the describing the one layer that gets created.

@rhatdan please see rhatdan#1

and once that's merged - LGTM - @fredlf @jamtur01

Any update?

@rhatdan I have this on my todo list, I'll review it today. I really want this too :)

This has the update to pass the changes via the url params via

for _, change := range flChanges.GetAll() {
        v.Add("changes", change)
}

This should be good to merged now if you can take another look.

@tiborvass @LK4D4

LGTM

Two more LGTM to make my day...
@tiborvass @LK4D4

LGTM

Hey folks this breaks if the busybox image in the test environment is not built from jpettazzo's busybox (https://github.com/docker/docker/blob/309eec23787d9e1e4de58b6475467a1af73c4c26/Dockerfile#L111) but rather pulled from docker hub (using docker pull busybox) because in the case of non-linux CLI tests we're running outside a container.

The busybox image on Docker Hub we pull in this case adds an extra PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin regardless of the --change parameter and the the test fails because inspected Config.Env has an extra PATH field. (https://github.com/docker/docker/blob/4a9fa9650b154e70d55f750c3674c2d6dd390bef/integration-cli/docker_cli_commit_test.go#L228)

How does it sound to add an extra --change ENV PATH /foo (and same in the expected string)? I tested it locally and it works fine. Is there any other way to get rid of that PATH added by busybox image on Docker Hub?

@tiborvass @dqminh @tianon