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

Dockerfile: use TARGETPLATFORM to build Docker #44546

Merged
merged 13 commits into from
Jan 2, 2023

Conversation

crazy-max
Copy link
Member

@crazy-max crazy-max commented Nov 29, 2022

follow-up #43529
needs #44513

fixes #41590
fixes #41862
fixes #41519
fixes #39380
closes #43492
closes #43613

Better support for cross compilation so we can fully rely on --platform flag of buildx for a seamless integration.

This removes unnecessary extra cross logic in the Dockerfile, removes DOCKER_CROSSPLATFORMS and CROSS vars and some hack scripts as well.

Non-sandboxed build invocation is still supported and dev stages in the Dockerfile have been updated accordingly.

Bake definition and GitHub Actions workflows have been updated accordingly as well.

cc @thaJeztah @rumpl @vvoland

@thaJeztah
Copy link
Member

@crazy-max this one can be rebased 👍

@crazy-max crazy-max force-pushed the dockerfile-cross branch 4 times, most recently from 0dd8bb2 to 82322a1 Compare December 16, 2022 07:46
@crazy-max crazy-max marked this pull request as ready for review December 16, 2022 08:00
@thaJeztah thaJeztah added this to the v-next milestone Dec 16, 2022
@crazy-max crazy-max force-pushed the dockerfile-cross branch 3 times, most recently from b567f53 to 8d0600f Compare December 19, 2022 08:32
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

was still reviewing, but had one comment pending already; let me submit that one

Dockerfile Show resolved Hide resolved
@crazy-max crazy-max force-pushed the dockerfile-cross branch 3 times, most recently from e153e02 to bd076dd Compare December 20, 2022 19:42
Copy link
Contributor

@corhere corhere left a comment

Choose a reason for hiding this comment

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

I didn't get past the first commit. There are so many behaviour changes to the hack/make scripts which have nothing to do with the PR description or commit message.

hack/make.sh Outdated
Comment on lines 134 to 135
export DOCKER_BUILDFLAGS=(-tags "${DOCKER_BUILDTAGS}" -installsuffix netgo)
# see https://github.com/golang/go/issues/9369#issuecomment-69864440 for why -installsuffix is necessary here
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like -installsuffix was necessary to work around bugs in go build which appear to have been fully resolved in go1.5.

hack/make/.binary Outdated Show resolved Hide resolved
hack/make.sh Outdated Show resolved Hide resolved
hack/make.sh Outdated Show resolved Hide resolved
hack/make.sh Outdated Show resolved Hide resolved
hack/make/.binary Outdated Show resolved Hide resolved
hack/make/.mkwinres Outdated Show resolved Hide resolved
hack/make/dynbinary-daemon Outdated Show resolved Hide resolved
hack/make/binary-proxy Outdated Show resolved Hide resolved
hack/make/dynbinary Outdated Show resolved Hide resolved
@crazy-max crazy-max force-pushed the dockerfile-cross branch 3 times, most recently from 0bc5c53 to 01b9872 Compare December 26, 2022 18:37
@crazy-max
Copy link
Member Author

crazy-max commented Dec 26, 2022

I didn't get past the first commit. There are so many behaviour changes to the hack/make scripts which have nothing to do with the PR description or commit message.

@corhere Yes this was indeed ahead of the expectations, sorry about that. It should be easier to review now.

@crazy-max crazy-max force-pushed the dockerfile-cross branch 2 times, most recently from d5e391c to 00c980e Compare December 27, 2022 21:11
@thaJeztah thaJeztah merged commit b9fe30d into moby:master Jan 2, 2023
@crazy-max crazy-max deleted the dockerfile-cross branch January 2, 2023 16:58
crazy-max added a commit to crazy-max/docker-ce-packaging that referenced this pull request Jan 5, 2023
follow-up moby/moby#44546

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
crazy-max added a commit to crazy-max/buildkit that referenced this pull request Jan 9, 2023
related to moby/moby#44546

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
crazy-max added a commit to crazy-max/buildkit that referenced this pull request Jan 9, 2023
related to moby/moby#44546

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
crazy-max added a commit to crazy-max/buildkit that referenced this pull request Jan 9, 2023
related to moby/moby#44546

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
crazy-max added a commit to crazy-max/buildkit that referenced this pull request Jan 9, 2023
related to moby/moby#44546

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
(cherry picked from commit 489c823)
crazy-max added a commit to crazy-max/buildkit that referenced this pull request Jan 10, 2023
related to moby/moby#44546

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
(cherry picked from commit 489c823)
crazy-max added a commit to crazy-max/moby that referenced this pull request Jan 12, 2023
Following changes for cross-compilation in moby#44546
the cross script setting GOARM to the expected value has been
removed but changes not carried in .binary script

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
crazy-max added a commit to crazy-max/moby that referenced this pull request Jan 12, 2023
Following changes for cross-compilation in moby#44546
the cross script setting GOARM to the expected value has been
removed but changes not carried in .binary script

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
crazy-max added a commit to crazy-max/moby that referenced this pull request Jan 16, 2023
Following changes for cross-compilation in moby#44546,
we forgot to remove the toolchain configuration that is
not used anymore as xx already sets correct cc/cxx envs already.

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
crazy-max added a commit to crazy-max/moby that referenced this pull request Jan 16, 2023
Build currently doesn't set the right name for target ARM
architecture through switches in CGO_CFLAGS and CGO_CXXFLAGS
when doing cross-compilation. This was previously fixed in moby#43474

Also removes the toolchain configuration. Following changes for
cross-compilation in moby#44546,
we forgot to remove the toolchain configuration that is
not used anymore as xx already sets correct cc/cxx envs already.

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
crazy-max added a commit to crazy-max/moby that referenced this pull request Jan 17, 2023
Build currently doesn't set the right name for target ARM
architecture through switches in CGO_CFLAGS and CGO_CXXFLAGS
when doing cross-compilation. This was previously fixed in moby#43474

Also removes the toolchain configuration. Following changes for
cross-compilation in moby#44546,
we forgot to remove the toolchain configuration that is
not used anymore as xx already sets correct cc/cxx envs already.

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
(cherry picked from commit 9457042)
thaJeztah pushed a commit to thaJeztah/docker that referenced this pull request Jan 17, 2023
Build currently doesn't set the right name for target ARM
architecture through switches in CGO_CFLAGS and CGO_CXXFLAGS
when doing cross-compilation. This was previously fixed in moby#43474

Also removes the toolchain configuration. Following changes for
cross-compilation in moby#44546,
we forgot to remove the toolchain configuration that is
not used anymore as xx already sets correct cc/cxx envs already.

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
(cherry picked from commit 9457042)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
crazy-max added a commit to crazy-max/moby that referenced this pull request Jan 18, 2023
This var was used for the cross target but it has been removed
in moby#44546 so not necessary anymore

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
thaJeztah pushed a commit to thaJeztah/buildkit that referenced this pull request Feb 4, 2023
related to moby/moby#44546

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
(cherry picked from commit 489c823)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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.

make win doesn't work Building Windows version fails How to build docker arm version?
4 participants