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

feat: support for multi-stage builds #84

Merged
merged 2 commits into from
Sep 13, 2017
Merged

feat: support for multi-stage builds #84

merged 2 commits into from
Sep 13, 2017

Conversation

nexdrew
Copy link
Collaborator

@nexdrew nexdrew commented Sep 1, 2017

Fixes #74.

The first commit adds tests for multi-stage builds (examples taken from Docker's documentation), which should fail.

The subsequent commits modify logic to get the tests to pass.

Instead of failing on each subsequent FROM command, the logic now considers this as the beginning of a new build stage (as modeled by internal state during instructions processing).

NOTE: This should probably be considered a BREAKING CHANGE, since Dockerfiles containing more than one FROM command will no longer fail linting, which will be problematic if the (unspecified) target Docker version is less than 17.05. We could potentially get around this by introducing a way (e.g. CLI/config option or auto env detection) to specify/detect the Docker version, but this seems out-of-scope for this PR.

Also note that this change does not add any new checks specific to multi-stage Dockerfiles, such as potentially checking that a stage has more than just a single FROM command. Even so, the changes represented here could be considered a necessary first step, allowing stage-specific checks to be added later.

@coveralls
Copy link

coveralls commented Sep 1, 2017

Coverage Status

Coverage increased (+0.004%) to 99.16% when pulling bde16c3 on multi-stage into 2976eaa on master.

@nexdrew nexdrew changed the title [WIP] feat: support for multi-stage builds feat: support for multi-stage builds Sep 1, 2017
@nexdrew
Copy link
Collaborator Author

nexdrew commented Sep 1, 2017

@marccampbell @emosbaugh Would love a review of this if you get a chance. 😎

Copy link
Member

@emosbaugh emosbaugh left a comment

Choose a reason for hiding this comment

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

lgtm

@nexdrew
Copy link
Collaborator Author

nexdrew commented Sep 1, 2017

@emosbaugh Thanks for the quick turnaround! Will wait to hear from #74 commenters before I proceed.

@nexdrew
Copy link
Collaborator Author

nexdrew commented Sep 12, 2017

@marccampbell @emosbaugh Thoughts on making this a major version bump? This question is the only reason I'm hesitant to merge and release this right now.

@emosbaugh
Copy link
Member

i see no reason to bump major version. no big refactor or anything and nothing incompatible or api changes

@coveralls
Copy link

coveralls commented Sep 12, 2017

Coverage Status

Coverage increased (+0.004%) to 99.16% when pulling ed00203 on multi-stage into 78b58e2 on master.

@coveralls
Copy link

coveralls commented Sep 12, 2017

Coverage Status

Coverage increased (+0.004%) to 99.16% when pulling 9970cce on multi-stage into 9bfbd62 on master.

@nexdrew
Copy link
Collaborator Author

nexdrew commented Sep 13, 2017

@emosbaugh

i see no reason to bump major version. no big refactor or anything and nothing incompatible or api changes

Fair enough. I'll merge and release as a minor update then. Thanks!

@nexdrew nexdrew merged commit 72d2087 into master Sep 13, 2017
@nexdrew nexdrew deleted the multi-stage branch September 13, 2017 00:56
@nexdrew
Copy link
Collaborator Author

nexdrew commented Sep 13, 2017

Published to npm as 1.4.0.

@vladkras vladkras mentioned this pull request Dec 25, 2018
svl7 pushed a commit to svl7/dockerfilelint that referenced this pull request Jun 22, 2020
interpret multiple FROM commands as multi-stage builds
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.

3 participants