Skip to content

Add environment replacement support for chmod option#5151

Merged
crazy-max merged 1 commit intomoby:masterfrom
kariya-mitsuru:add-env-replace-for-chmod
Aug 6, 2024
Merged

Add environment replacement support for chmod option#5151
crazy-max merged 1 commit intomoby:masterfrom
kariya-mitsuru:add-env-replace-for-chmod

Conversation

@kariya-mitsuru
Copy link
Copy Markdown
Contributor

Add environment replacement support for chmod option argument for the ADD and COPY commands.

Copy link
Copy Markdown
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.

I think this is fine to add but we need an integration test in dockerfile_test .

@AkihiroSuda sgty?

@AkihiroSuda
Copy link
Copy Markdown
Member

I think this is fine to add but we need an integration test in dockerfile_test .

@AkihiroSuda sgty?

SGTM

@kariya-mitsuru
Copy link
Copy Markdown
Contributor Author

One build test image crashed with SEGV, but I don't remember making any changes that would cause this to happen, and other combinations of matrix seem to complete normally.
Should I need to take any action?

@kariya-mitsuru kariya-mitsuru force-pushed the add-env-replace-for-chmod branch from a5755b1 to 591c23d Compare July 21, 2024 11:16
Copy link
Copy Markdown
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.

LGTM, but squash the commits please (or fix the commit message for second commit).

@tonistiigi tonistiigi added this to the v0.16.0 milestone Jul 23, 2024
Add environment replacement support for chmod option argument for the
ADD and COPY commands.

Signed-off-by: Mitsuru Kariya <[email protected]>
@crazy-max
Copy link
Copy Markdown
Member

@dvdksn I think we need some docs follow-up for this one.

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