-
Notifications
You must be signed in to change notification settings - Fork 18.8k
Distinguish ENV from setting environment inline #10273
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
Conversation
Can you please sign your commits following these rules: https://github.com/docker/docker/blob/master/CONTRIBUTING.md#sign-your-work The easiest way to do this is to amend the last commit:
|
Signed off |
Well spotted, I agree this makes it a bit clearer :) (but I am no maintainer) |
8000 | functionally equivalent to prefixing the command with `<key>=<value>` | |
`RUN`, `ENTRYPOINT`, and `CMD` instructions. To set a value for a single | ||
command without affecting future commands or containers, prefix the command | ||
with `<key>=<value>`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this isn't really due to your proposed change, but since you're here anyway.... :-) perhaps you could tweak this paragraph a little more. Its a bit misleading to say that it just impacts RUN, ENTRYPOINT and CMD because I believe that @erikh made it so that these env vars are also used in variable substitutions as well. See https://github.com/docker/docker/blob/master/builder/evaluator.go#L46 for the list of commands that we do env var substitutions. If you don't want to make this change let me know and I'll do it in a different PR.
As for your text, you may need to make it clear that prefixing only works for the sh version, not the JSON version (I think).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It also does not explain how the environment actually works in RUN vs. ENV, which I find a bit misleading.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@erikh I'm not sure how precise you want me to be. IIRC the RUN
command uses something like /bin/sh -c
(assuming you're not using exec-style braces), right? So RUN var=val cmd
should behave as described by XCU Shell Command Language. The value is exported only for cmd and not for "the current execution environment" (which would be /bin/sh
). In other words, RUN var=val cmd
is nothing special to Docker, it's just a normal simple shell command.
I'm trying to walk the line and not explain basic UNIX to documentation readers, but at the same time, as much as possible make the documentation accessible for people who may not completely understand basic UNIX.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps: “RUN, CMD, and ENTRYPOINT populate the environment of the running process but not the environment of any running containers”
Or something like that.
-Erik
On Jan 22, 2015, at 5:16 PM, Michael A. Smith notifications@github.com wrote:
In docs/sources/reference/builder.md #10273 (comment):
@@ -350,8 +350,9 @@ accessible from the host by default. To expose ports to the host, at runtime,
The
ENV
instruction sets the environment variable<key>
to the value
<value>
. This value will be passed to all future
-RUN
,ENTRYPOINT
, andCMD
instructions. This is
-functionally equivalent to prefixing the command with<key>=<value>
+RUN
,ENTRYPOINT
, andCMD
instructions. To set a value for a single
+command without affecting future commands or containers, prefix the command
+with<key>=<value>
.
@erikh https://github.com/erikh I'm not sure how precise you want me to be. IIRC the RUN command uses something like /bin/sh -c (assuming you're not using exec-style braces), right? So RUN var=val cmd should behave as described by XCU Shell Command Language http://pubs.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_09_01. The value is exported only for cmd and not for "the current execution environment". In other words, RUN var=val cmd is nothing special to Docker, it's just a normal simple shell command.I'm trying to walk the line and not explain basic UNIX to documentation readers, but at the same time, as much as possible make the documentation accessible for people who may not completely understand basic UNIX.
—
Reply to this email directly or view it on GitHub https://github.com/docker/docker/pull/10273/files#r23425652.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is that true? In one window I did:
docker run -ti -e foo=bar ubuntu bash
and in another I did:
docker exec -ti ce0fd7706d0a bash
ce0f.. is the 1st container ID and I see foo set to bar.
This also works if I use ENV from the Dockerfile used to create the first container/image and then left off the -e on the run.
Personally, I wouldn't talk about "RUN foo=bar do-something...". That's just normal shell processing.
How about:
The ENV
instruction sets the environment variable <key>
to the value <value>
. This variable will not only be visible within the container but also available for environment variable expansion within the following Dockerfile commands: ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@duglin It may not be obvious to everyone that you can set a value in the environment just for a single command. And once you set an ENV
you can't really unset it. So I still think RUN foo=bar do-something
should exist. But maybe it doesn't belong here. Maybe it belongs in the best practices.
Also, I realized the Environment Replacement section starting on line 108 covers your original question. Maybe it just needs to be better tied to the actual ENV
documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that’s my fault. :)
If you want to link/reorganize the two sections, I won’t object, but Sven or Fred might. :P
On Jan 22, 2015, at 6:47 PM, Michael A. Smith notifications@github.com wrote:
In docs/sources/reference/builder.md #10273 (comment):
@@ -350,8 +350,9 @@ accessible from the host by default. To expose ports to the host, at runtime,
The
ENV
instruction sets the environment variable<key>
to the value
<value>
. This value will be passed to all future
-RUN
,ENTRYPOINT
, andCMD
instructions. This is
-functionally equivalent to prefixing the command with<key>=<value>
+RUN
,ENTRYPOINT
, andCMD
instructions. To set a value for a single
+command without affecting future commands or containers, prefix the command
+with<key>=<value>
.
@duglin https://github.com/duglin It may not be obvious to everyone that you can set a value in the environment just for a single command. And once you set an ENV you can't really unset it. So I still think RUN foo=bar do-something should exist. But maybe it doesn't belong here. Maybe it belongs in the best practices https://github.com/docker/docker/blob/master/docs/sources/articles/dockerfile_best-practices.md.Also, I realized the Environment Replacement section starting on line 108 covers your original question. Maybe it just needs to be better tied to the actual ENV documentation.
—
Reply to this email directly or view it on GitHub https://github.com/docker/docker/pull/10273/files#r23428928.
@kojiromike thank you :) I'd be happy to take your commit as is, and I can address the comments in a follow on PR, or if you're happy to iterate, awesome! I think that the |
@SvenDowideit I don't mind working on it some more, but I'd prefer this tidbit get merged in and do the rest in another PR if it's all the same. |
I'm a little lost as to what to tackle next. I'd feel better if I could get some semi-authoritative direction on whether to tackle the ENV vs |
I think I'd rather cover it here - best practices are more for when there are more than one way to achieve the same outcome. |
2a5c3a5
to
fe21a3f
Compare
Attempt two. I didn't squash in case you want to go back to the original, but I can squash before merge if you like; just let me know. |
`<value>`. This value will be passed to all future | ||
`RUN`, `ENTRYPOINT`, and `CMD` instructions. This is | ||
functionally equivalent to prefixing the command with `<key>=<value>` | ||
`<value>`. This value will be in the environment of all "descendent" builds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/builds/Dockerfile commands/
The first time I read "descendent builds" I interpreted it as "the next time I run 'docker build' ". Could just be me though.
yea, if possible, please squash commits. |
It's ambiguous to say that `ENV` is _functionally equivalent to prefixing the command with `<key>=<value>`_. `ENV` sets the environment for all future commands, but `RUN` can take chained commands like `RUN foo=bar bash -c 'echo $foo' && bash -c 'echo $foo $bar'`. Users with a solid understanding of `exec` may grok this without confusion, but less experienced users may need this distinction. Signed-off-by: Michael A. Smith <msmith3@ebay.com> Improve Environment Handling Descriptions - Link `ENV` and `Environment Replacement` - Improve side-effects of `ENV` text - Rearrange avoiding side effects text Signed-off-by: Michael A. Smith <msmith3@ebay.com>
Squashed as requested. |
LGTM @SvenDowideit feel free to make any edits post merge |
Distinguish ENV from setting environment inline
It's ambiguous to say that
ENV
is functionally equivalent to prefixing the command with<key>=<value>
.ENV
sets the environment for all future commands, butRUN
can take chained commands likeRUN foo=bar bash -c 'echo $foo' && bash -c 'echo $foo $bar'
. Users with a solid understanding ofexec
may grok this without confusion, but less experienced users may need this distinction.