Skip to content
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

deprecate Dockerfile legacy 'ENV name value' syntax #2743

Merged
merged 1 commit into from
Sep 23, 2020

Conversation

thaJeztah
Copy link
Member

The Dockerfile ENV instruction allows values to be set using either ENV name=value
or ENV name value. The latter (ENV name value) form can be ambiguous, for example,
the following defines a single env-variable (ONE) with value "TWO= THREE=world",
but may have intended to be setting three env-vars:

ENV ONE TWO= THREE=world

This format also does not allow setting multiple environment-variables in a single
ENV line in the Dockerfile.

Use of the ENV name value syntax is discouraged, and may be removed in a future
release. Users are encouraged to update their Dockerfiles to use the ENV name=value
syntax, for example:

ENV ONE="" TWO="" THREE="world"

- Description for the changelog

- deprecate Dockerfile legacy 'ENV name value' syntax

- A picture of a cute animal (not mandatory but encouraged)

The Dockerfile `ENV` instruction allows values to be set using either `ENV name=value`
or `ENV name value`. The latter (`ENV name value`) form can be ambiguous, for example,
the following defines a single env-variable (`ONE`) with value `"TWO= THREE=world"`,
but may have intended to be setting three env-vars:

    ENV ONE TWO= THREE=world

This format also does not allow setting multiple environment-variables in a single
`ENV` line in the Dockerfile.

Use of the `ENV name value` syntax is discouraged, and may be removed in a future
release. Users are encouraged to update their Dockerfiles to use the `ENV name=value`
syntax, for example:

    ENV ONE="" TWO="" THREE="world"

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

@tonistiigi @tiborvass ptal

@codecov-commenter
Copy link

Codecov Report

Merging #2743 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2743   +/-   ##
=======================================
  Coverage   57.15%   57.15%           
=======================================
  Files         297      297           
  Lines       18657    18657           
=======================================
  Hits        10663    10663           
  Misses       7132     7132           
  Partials      862      862           

@tiborvass tiborvass merged commit db411c3 into docker:master Sep 23, 2020
@thaJeztah thaJeztah deleted the deprecate_legacy_env_format branch September 23, 2020 20:42
@tianon
Copy link
Contributor

tianon commented Dec 9, 2020

What's the end goal of deprecating this (very widely used) syntax? Do we actually think we can remove support for it in the future without causing massive breakage? (Again, to what end?)

Why not instead detect the ambiguous case and provide a warning?

@thaJeztah
Copy link
Member Author

Some more details in moby/buildkit#1692 (comment)

@tonistiigi
Copy link
Member

There is no timeline to remove it atm. and we'll not do it if it can cause a big breakage. This is phase 1 where we need to send a clear message that this syntax should not be used anymore.

@tanzislam
Copy link

I'd recommend not removing the without-= syntax, as I find that much more readable when my variable values need to have = and space characters in them. (The cost of the extra image/builder layers due to separate ENV statements is minimal compared to the readability improvement.)

@tianon
Copy link
Contributor

tianon commented Apr 3, 2024

Reiterating for the record that I think deprecating this syntax is a mistake. 😞

Coming from the context of moby/buildkit#4759, I would re-iterate "Why not instead detect the ambiguous case and provide a warning?"

Helping people find Dockerfiles that are actually problematic/ambiguous/possibly not doing what they expected seems a lot more fruitful than trying to remove this syntax entirely.

@tianon
Copy link
Contributor

tianon commented Jul 8, 2024

Now I'm seeing a lot of useless Dockerfile churn/noise for completely 100% unambiguous ENV foo bar lines just to silence the new warning 🙃

Can we please remove that linter warning? (or at least disable it by default somehow so users can opt into it instead?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants