Skip to content

build: automatically set correct $TARGETPLATFORM where expected.#3899

Merged
openshift-merge-robot merged 3 commits intocontainers:mainfrom
flouthoc:set-correct-targetplatform
May 3, 2022
Merged

build: automatically set correct $TARGETPLATFORM where expected.#3899
openshift-merge-robot merged 3 commits intocontainers:mainfrom
flouthoc:set-correct-targetplatform

Conversation

@flouthoc
Copy link
Copy Markdown
Collaborator

@flouthoc flouthoc commented Apr 5, 2022 •
edited
Loading

Automatically set correct value of TARGETPLATFORM if one or more
--platform are selected for the build. This ensures that behaviour
of buildah is equivalent to buildkit for the args $BUILDPLATFORM and
$TARGETPLATFORM.

Where $BUILDPLATFORM is set always native to host and $TARGETPLATFORM
is foreign selected os/arch via --platform.

Example

$ buildah build --platform linux/arm64 .
FROM --platform=$BUILDPLATFORM alpine
ARG TARGETPLATFORM
ARG BUILDPLATFORM
RUN echo "I'm compiling for $TARGETPLATFORM on $BUILDPLATFORM and tagging for $TARGETPLATFORM"
  • PR also adds a test to verify correct behaviour of $BUILDPLATFORM and $TARGETPLATFORM for cases using inline FROM --platform=

Wait for: openshift/imagebuilder#224
See also: https://docs.docker.com/buildx/working-with-buildx/#build-multi-platform-images

@openshift-ci openshift-ci bot added the approved label Apr 5, 2022
@flouthoc flouthoc mentioned this pull request Apr 5, 2022
Merged
nalind
nalind reviewed Apr 5, 2022
View reviewed changes
@flouthoc flouthoc force-pushed the set-correct-targetplatform branch from 9935651 to b6d93da Compare April 5, 2022 16:17
@TomSweeneyRedHat
Copy link
Copy Markdown
Member

TomSweeneyRedHat commented Apr 5, 2022

LGTM, once a new IB is built and vendored. I've thrown a WIP tag on this for now.

@flouthoc flouthoc added the 4.1 podman 4.1 priorities label Apr 12, 2022
@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 22, 2022
@flouthoc flouthoc force-pushed the set-correct-targetplatform branch from b6d93da to ef9a9fd Compare May 2, 2022 18:33
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 2, 2022
@flouthoc
Copy link
Copy Markdown
Collaborator Author

flouthoc commented May 2, 2022

Since this is merged openshift/imagebuilder#224 , pointing to latest imagebuilder and marking this PR ready for review.

@nalind
Copy link
Copy Markdown
Member

nalind commented May 2, 2022

The imagebuilder logic also predefines TARGETOS, TARGETARCH, and TARGETVARIANT args. Are those already correct, or do they likewise need to be fixed up at the same time?

@flouthoc flouthoc force-pushed the set-correct-targetplatform branch from ef9a9fd to 50249ef Compare May 2, 2022 19:45
@flouthoc
Copy link
Copy Markdown
Collaborator Author

flouthoc commented May 2, 2022

@nalind yes we were also not setting TARGETOS, TARGETARCH and TARGETVARIANT correctly added it now.

@flouthoc flouthoc force-pushed the set-correct-targetplatform branch from 50249ef to 23f9c28 Compare May 2, 2022 19:50
rhatdan
rhatdan reviewed May 2, 2022
View reviewed changes
rhatdan
rhatdan reviewed May 2, 2022
View reviewed changes
rhatdan
rhatdan reviewed May 2, 2022
View reviewed changes
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci bot commented May 2, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@flouthoc flouthoc requested a review from rhatdan May 2, 2022 20:10
@flouthoc flouthoc force-pushed the set-correct-targetplatform branch 3 times, most recently from 69c7e6e to 67ae077 Compare May 3, 2022 10:11
flouthoc added 3 commits May 3, 2022 15:52
@flouthoc
build: automatically set correct TARGETPLATFORM where expected
bee0a1f 
Automatically set correct value of TARGETPLATFORM if one or more
`--platform` are selected for the build. This ensures that behaviour
of buildah is equivalent to `buildkit` for the args `$BUILDPLATFORM` and
`$TARGETPLATFORM`, `$TARGETOS`, `$TARGETARCH` and `$TARGETVARIANT`

Where $BUILDPLATFORM is set always native to host and $TARGETPLATFORM
is foreign selected via `--platform`.

Example

```console
$ buildah build --platform linux/arm64 .
```

```Dockerfile
FROM --platform=$BUILDPLATFORM alpine
ARG TARGETPLATFORM
ARG BUILDPLATFORM
RUN echo "I'm compiling for $TARGETPLATFORM on $BUILDPLATFORM"
```

Signed-off-by: Aditya R <arajan@redhat.com>
@flouthoc
vendor: bump imagebuilder to master/009dbc6
d3cbd41 
Use imagebuilder which sets correct behaviour for default builtinargs
while processing inline `FROM --platform=` after openshift/imagebuilder#224

Signed-off-by: Aditya R <arajan@redhat.com>
@flouthoc
test: inline platform and builtinargs behaviour
01b65b3 
Following test verifies the behaviour of inline `--platform` with `FROM`
statement and usage of builtinargs with it, specifically the behaviour
of `$TARGETPLATFORM` and `$BUILDPLATFORM`

Signed-off-by: Aditya R <arajan@redhat.com>
@flouthoc flouthoc force-pushed the set-correct-targetplatform branch from 67ae077 to 01b65b3 Compare May 3, 2022 10:23
@flouthoc
Copy link
Copy Markdown
Collaborator Author

flouthoc commented May 3, 2022

@rhatdan PTAL updated comments as requested.

@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented May 3, 2022

/lgtm
/hold

@flouthoc
Copy link
Copy Markdown
Collaborator Author

flouthoc commented May 3, 2022

/hold cancel

@openshift-merge-robot openshift-merge-robot merged commit d6fd289 into containers:main May 3, 2022
@edsantiago
Copy link
Copy Markdown
Member

edsantiago commented May 3, 2022

This breaks podman: SEGV in bud-multiple-something (also remote). Stack trace, so it should be easy to identify and fix.

@flouthoc
Copy link
Copy Markdown
Collaborator Author

flouthoc commented May 3, 2022

@edsantiago Checking.

@edsantiago
Copy link
Copy Markdown
Member

edsantiago commented May 3, 2022 •
edited
Loading

Thank you. From my untrained glance it makes no sense, I see nothing in these diffs that could cause it; but I ran the treadmill on the previous commit and everything passed, so this is the likely first place to look.

UPDATE: reproduces easily under buildah itself, so it's not a podman-vendoring thing

@flouthoc flouthoc mentioned this pull request May 3, 2022
Merged
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@nalind nalind nalind left review comments

@rhatdan rhatdan Awaiting requested review from rhatdan

Assignees

@rhatdan rhatdan

Labels

4.1 podman 4.1 priorities approved buildkit lgtm locked - please file new issue/PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@flouthoc @TomSweeneyRedHat @nalind @rhatdan @edsantiago @openshift-merge-robot