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

frontend/dockerfile: add RunCommand.FlagsUsed field #1954

Merged
merged 1 commit into from
Feb 4, 2021

Conversation

thaJeztah
Copy link
Member

The FlagsUsed contains a list of flags that were used, which allows the classic (non-buildkit) builder in dockerd to produce an error when non-supported options are used in a Dockerfile.

This is a short-term solution; a more permanent solution will be to keep track of which version of the Dockerfile syntax is supported, and to have the classic builder pass the maximum supported version of the syntax.

Copy link
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

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

SGTM

@thaJeztah
Copy link
Member Author

Let me add the new option to TestBuilderFlags to make codecov happy;

func TestBuilderFlags(t *testing.T) {

The FlagsUsed contains a list of flags that were used, which allows the classic
(non-buildkit) builder in dockerd to produce an error when non-supported options
are used in a Dockerfile.

This is a short-term solution; a more permanent solution will be to keep track
of which version of the Dockerfile syntax is supported, and to have the classic
builder pass the maximum supported version of the syntax.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah force-pushed the dockerfile_export_flags_used branch from 6d96fdf to ebed917 Compare January 27, 2021 12:34
@thaJeztah
Copy link
Member Author

Added some tests

@thaJeztah
Copy link
Member Author

@tonistiigi ptal (let me know if you're ok with this, or prefer to directly go for the "correct" approach)

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.

3 participants