Skip to content

dockerfile: add support for ONBUILD in combination with from#5357

Merged
tonistiigi merged 2 commits intomoby:masterfrom
tonistiigi:onbuild-from-support
Oct 1, 2024
Merged

dockerfile: add support for ONBUILD in combination with from#5357
tonistiigi merged 2 commits intomoby:masterfrom
tonistiigi:onbuild-from-support

Conversation

@tonistiigi
Copy link
Copy Markdown
Member

This adds support for the ONBUILD commands to refer to
other stages in commands like COPY --from= or
RUN --mount=from= . The source may be a stage in the
calling Dockerfile, implicit image or named build context.

depends on #5349
fixes #4568 (comment)
fixes #959
fixes #816
fixes #817

This avoids many temporary conversion between maps/slices
and shell.EnvGetter.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
@tonistiigi tonistiigi changed the title dockerfile: add support for ONBUILD in combination to from dockerfile: add support for ONBUILD in combination with from Sep 24, 2024
@tonistiigi tonistiigi force-pushed the onbuild-from-support branch 2 times, most recently from 98e9189 to cac7523 Compare September 25, 2024 00:55
This adds support for the ONBUILD commands to refer to
other stages in commands like `COPY --from=` or
`RUN --mount=from=` . The source may be a stage in the
calling Dockerfile, implicit image or named build context.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
@tonistiigi tonistiigi marked this pull request as ready for review September 25, 2024 04:56
@thompson-shaun thompson-shaun added this to the v0.17.0 milestone Sep 25, 2024
}

func testOnBuildNamedContext(t *testing.T, sb integration.Sandbox) {
integration.SkipOnPlatform(t, "windows")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We will try it on Windows once #5289 lands. /cc. @billywr

Copy link
Copy Markdown
Member

@crazy-max crazy-max left a comment

Choose a reason for hiding this comment

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

As far as I hate ONBUILD pattern, this LGTM after testing with several use cases (mounts, cache, named context). The nested onbuild implementation is as good as cursed though 🙈

Follow-ups to consider:

d.init()

if len(d.image.Config.OnBuild) > 0 {
if b, err := initOnBuildTriggers(d, d.image.Config.OnBuild, allDispatchStates); err != nil {
Copy link
Copy Markdown
Member

@crazy-max crazy-max Oct 1, 2024

Choose a reason for hiding this comment

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

Should we set a prefix to show this is coming from ONBUILD?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If it has a prefix then I think it means "add the onbuild rule to current image config" (this is what is shown in legacy builder I think). Here it is actually running the command.

@tonistiigi tonistiigi merged commit 8193e88 into moby:master Oct 1, 2024
@thaJeztah thaJeztah added the area/feature-parity Feature parity with classic builder label Oct 2, 2024
ledermann added a commit to ledermann/docker-rails-base that referenced this pull request Feb 18, 2025
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.

Buildkit does not allow ONBUILD RUN --mount... buildkit and experimental frontend bug: buildkit does not consider ONBUILD COPY --from

5 participants