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

(maint) Scoping for Docker ARGs is not intuitive #2275

Merged
merged 3 commits into from Feb 5, 2020

Conversation

underscorgan
Copy link
Contributor

For multi-stage builds, setting ARGs before the FROM makes their
scope global, but you still need to have an ARG foo line in each build
step to use the global ARG foo.

For single-stage builds, it doesn't make sense to define ARGs before
the FROM except for ARGs used in the FROM, since global scope
doesn't really help with only one stage.

@underscorgan underscorgan requested a review from a team as a code owner January 27, 2020 19:34
@underscorgan
Copy link
Contributor Author

See also moby/moby#37345

For multi-stage builds, setting `ARG`s before the `FROM` makes their
scope global, but you still need to have an `ARG foo` line in each build
step to use the global `ARG foo`.

For single-stage builds, it doesn't make sense to define `ARG`s before
the `FROM` except for `ARG`s used in the `FROM`, since global scope
doesn't really help with only one stage.
ARG version="6.0.0"
ARG namespace="puppet"
FROM "$namespace"/puppetserver-base:"$version"
ARG vcs_ref
Copy link
Contributor

Choose a reason for hiding this comment

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

Since there's only one stage in this file, why not also move the version and namespace values below the FROM?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because those are used in the FROM line

Copy link
Contributor

Choose a reason for hiding this comment

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

Hahahaha, I suppose that's a good reason! Sheesh.

Copy link
Contributor

@Iristyle Iristyle left a comment

Choose a reason for hiding this comment

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

ship it

@Iristyle Iristyle merged commit 1beddd8 into puppetlabs:master Feb 5, 2020
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.

None yet

2 participants