Skip to content

builder: rephrase ENV section, remove examples for ENV key value without '='#2741

Merged
thaJeztah merged 3 commits intodocker:masterfrom
thaJeztah:rewrite_build_env
Sep 23, 2020
Merged

builder: rephrase ENV section, remove examples for ENV key value without '='#2741
thaJeztah merged 3 commits intodocker:masterfrom
thaJeztah:rewrite_build_env

Conversation

@thaJeztah
Copy link
Member

relates to moby/buildkit#1692 (comment)

- What I did

The ENV key value form can be ambiguous, for example, the following defines
a single env-variable (ONE) with value "TWO= THREE=world":

ENV ONE TWO= THREE=world

While we cannot deprecate/remove that syntax (as it would break existing
Dockerfiles), we should reduce exposure of the format in our examples.

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

…out '='

The `ENV key value` form can be ambiguous, for example, the following defines
a single env-variable (`ONE`) with value `"TWO= THREE=world"`:

    ENV ONE TWO= THREE=world

While we cannot deprecate/remove that syntax (as it would break existing
Dockerfiles), we should reduce exposure of the format in our examples.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@codecov-commenter
Copy link

Codecov Report

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

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

@thaJeztah
Copy link
Member Author

@silvin-lubecki @tonistiigi ptal

Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, with just one comment

RUN DEBIAN_FRONTEND=noninteractive apt-get update && apt-get install -y ...
```

Or using [`ARG`](#arg), which is not persisted in the final image:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should enforce that, I consider it more a trick than a real feature.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a build-time argument/option, so I think it's a valid use, not different from, e.g.

ARG VERSION=v1.0.0
RUN install foo-${VERSION}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me the ARG semantic is to expose a customization point to the user, not to set an env var only during build step. So yes it works, but should we advertise it in the doc?

> ```
>
> The alternative syntax is supported for backward compatibility, but discouraged
> for the reasons outlined above.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and may be removed in a future version.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated; PTAL

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened #2743 if we want to officially deprecate

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah merged commit 616e068 into docker:master Sep 23, 2020
@thaJeztah thaJeztah deleted the rewrite_build_env branch September 23, 2020 20:30
@thaJeztah thaJeztah added this to the 20.10.0 milestone Jan 5, 2021
libvirtmirror pushed a commit to libvirt/libvirt-ci that referenced this pull request Sep 22, 2025
The endorsed format for specifying environment variables in
Dockerfile is:

  ENV key="value"

The old format is deprecated as of 2020 [1][2]. Fix our formatter
to follow correct format.

1: docker/cli#2743
2: docker/cli#2741

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
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.

4 participants