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(run-mount): add support for variable expansion #2089

Merged
merged 1 commit into from
May 6, 2021

Conversation

omermizr
Copy link

This PR adds support for variable expansion in RUN mounts.
Resolves #815

Caveats:

  • from expansion is not supported.
  • Expansion is not supported in ONBUILD RUN commands (I don't even think mounts work there).

@omermizr
Copy link
Author

@tonistiigi FYI

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.

Missing DCO

I don't fully get the changes in convert.go. If it is just refactoring with no functional changes it should be in a different PR.

This doesn't fix the issues with output afaics. Fine by me if you want to tackle this in follow-up but should be fixed as well.

frontend/dockerfile/dockerfile2llb/convert_runmount.go Outdated Show resolved Hide resolved
frontend/dockerfile/instructions/commands_runmount.go Outdated Show resolved Hide resolved
frontend/dockerfile/instructions/commands.go Outdated Show resolved Hide resolved
frontend/dockerfile/instructions/commands_runmount.go Outdated Show resolved Hide resolved
frontend/dockerfile/instructions/commands_runmount.go Outdated Show resolved Hide resolved
frontend/dockerfile/instructions/commands_runmount.go Outdated Show resolved Hide resolved
@omermizr omermizr force-pushed the feature/run-mount-variables branch from 4ca939e to 1f39eb8 Compare May 1, 2021 18:10
@omermizr
Copy link
Author

omermizr commented May 1, 2021

Added DCOs.
Removed changes in convert.go.
Regarding output formatting - I think it's beyond the scope of this PR, especially considering it's an existing problem. I also don't wanna mix a feature branch with a fix branch. I can make a separate PR to fix it, but I couldn't figure out where that code is. If you can send me on a path that'd be great :)

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.

Only small comments left. Make sure to also squash commits and verify CI

@omermizr omermizr force-pushed the feature/run-mount-variables branch from 590ab7d to 7dd7a67 Compare May 3, 2021 07:08
Signed-off-by: Omer Mizrahi <ommizrah@microsoft.com>
@omermizr omermizr force-pushed the feature/run-mount-variables branch from 71c8bf4 to 1ac4230 Compare May 5, 2021 06:36
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.

feature request: allow variables in the RUN --mount=type=bind values
2 participants