8000 Distinguish ENV from setting environment inline by kojiromike · Pull Request #10273 · moby/moby · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

Merged
merged 1 commit into from
Feb 6, 2015

Conversation

kojiromike
Copy link
Contributor

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.

@jessfraz
Copy link
Contributor

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:

git commit --amend -s --no-edit && git push -f

@kojiromike
Copy link
Contributor Author

Signed off

@thaJeztah
Copy link
Member

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>`.
Copy link
Contributor

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

Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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, and CMD instructions. This is
-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>.
@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.

Copy link
Contributor

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

Copy link
Contributor Author

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.

Copy link
Contributor

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, and CMD instructions. This is
-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>.
@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.

@SvenDowideit
Copy link
Contributor

@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 prefix the command with <key>=<value> might be more obvious if we give and example.

@kojiromike
Copy link
Contributor Author

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

@kojiromike
Copy link
Contributor Author

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 RUN x=y cmd app here or in best practices, or not at all.

@SvenDowideit
Copy link
Contributor

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.

@kojiromike kojiromike force-pushed the patch-1 branch 2 times, most recently from 2a5c3a5 to fe21a3f Compare February 1, 2015 23:11
@kojiromike
Copy link
Contributor Author

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
Copy link
Contributor

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.

@duglin
Copy link
Contributor
duglin commented Feb 2, 2015

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>
@kojiromike
Copy link
Contributor Author

Squashed as requested.

@crosbymichael
Copy link
Contributor

LGTM

@SvenDowideit feel free to make any edits post merge

crosbymichael added a commit that referenced this pull request Feb 6, 2015
Distinguish ENV from setting environment inline
@crosbymichael crosbymichael merged commit 7d4bbc2 into moby:master Feb 6, 2015
@kojiromike kojiromike deleted the patch-1 branch February 7, 2015 03:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants
0