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

fix cross compilation for arm platforms #43474

Merged
merged 4 commits into from
Apr 14, 2022
Merged

Conversation

crazy-max
Copy link
Member

follow-up docker/docker-ce-packaging#665 (comment)

cross compilation is currently broken against some arm platforms:

#43 0.132 
#43 0.138 ---> Making bundle: cross (in /build/bundles/cross)
#43 0.139 Cross building: /build/bundles/cross/linux/arm/v6
#43 0.182 Building: /build/bundles/cross/linux/arm/v6-daemon/dockerd-0.0.0-20220329083112-174e51c
#43 0.182 GOOS="linux" GOARCH="arm" GOARM="6"
#43 76.80 # github.com/docker/docker/cmd/dockerd
#43 76.80 loadinternal: cannot find runtime/cgo
#43 76.80 /usr/local/go/pkg/tool/linux_amd64/link: running gcc failed: exit status 1
#43 76.80 gcc: error: unrecognized command-line option '-marm'; did you mean '-mabm'?

- What I did

fix cross compilation for arm platforms by setting correct switches for CGO_CFLAGS and CGO_CXXFLAGS flags.

also adds a GitHub Action job to check cross targets work, which is not currently done in Jenkins.

- How to verify it

$ mkdir -p autogen # workaround that will be fixed by https://github.com/moby/moby/pull/43431
$ DOCKER_CROSSPLATFORMS="linux/arm/v7 linux/arm/v6 linux/arm/v5" docker buildx bake cross

- Description for the changelog

fix cross compilation for arm platforms


in a follow-up we should switch to xx.

cc @thaJeztah

@crazy-max crazy-max requested a review from tianon as a code owner April 9, 2022 01:00
@crazy-max crazy-max force-pushed the fix-cross branch 3 times, most recently from 9df27b7 to 566c327 Compare April 9, 2022 01:24
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.

LGTM

Are any of these perhaps also needed to address docker-library/docker#260 ?

if-no-files-found: error
retention-days: 7

cross:
Copy link
Member

Choose a reason for hiding this comment

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

Should we remove the cross stage from Jenkins once this is merged?

remove --build-arg CROSS=true from

sh 'docker build --force-rm --build-arg APT_MIRROR --build-arg CROSS=true -t docker:${GIT_COMMIT} .'

remove the cross stage:

moby/Jenkinsfile

Lines 167 to 179 in c687298

stage("Cross") {
steps {
sh '''
docker run --rm -t --privileged \
-v "$WORKSPACE/bundles:/go/src/github.com/docker/docker/bundles" \
--name docker-pr$BUILD_NUMBER \
-e DOCKER_GITCOMMIT=${GIT_COMMIT} \
-e DOCKER_GRAPHDRIVER \
docker:${GIT_COMMIT} \
hack/make.sh cross
'''
}
}

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we should remove the CROSS and DOCKER_CROSSPLATFORMS args in a follow-up and rely only on --platform like docker/cli, buildx so packaging will be easier and also we could remove the cross logic in the .binary script. But that needs some work to align the dependencies in the dockerfile.

Copy link
Member

Choose a reason for hiding this comment

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

We need to make sure it can be run outside of the Dockerfile / container as well; it's possible there's people using the scripts to build/cross compile directly on the host.

(I guess xx can be used in that situation as well, but may need either extra instructions or a script to set it up)

Copy link
Member

Choose a reason for hiding this comment

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

We do not need a non-Dockerfile "cross-all" type target if we have no use for it ourselves. The script for the single arch would run for any target arch inside Dockerfile(and cross-all is just --platform list). To run the same script locally it is user's responsibility that they have a cross toolchain for the target arch they are setting.

Copy link
Member

Choose a reason for hiding this comment

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

it's possible there's people using the scripts to build/cross compile directly on the host.

I don't know of distros that would use these scripts for cross-compilation, but I do know at least some distros definitely build their packages outside of containers/in their own environments.

Copy link
Member

Choose a reason for hiding this comment

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

Let's continue that discussion when that change is actually made (this PR does not yet make that change)

.github/workflows/ci.yml Show resolved Hide resolved
@thaJeztah
Copy link
Member

@tonistiigi @tianon PTAL

@thaJeztah thaJeztah added this to the 22.04.0 milestone Apr 10, 2022
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
@thaJeztah
Copy link
Member

@tonistiigi @tianon PTAL; this LGTY?

@crazy-max
Copy link
Member Author

crazy-max commented Apr 12, 2022

@thaJeztah

Are any of these perhaps also needed to address docker-library/docker#260 ?

Don't think it's an issue in this repo but in https://github.com/docker-library/docker/blob/master/Dockerfile.template.

My bad I see this Dockerfile downloads our static packages. Are these packages cross compiled or native? If cross then yes that should fix this issue.

@thaJeztah
Copy link
Member

Thx! Let's get this one in

@thaJeztah thaJeztah merged commit 61404de into moby:master Apr 14, 2022
@crazy-max crazy-max deleted the fix-cross branch April 14, 2022 17:46
crazy-max added a commit to crazy-max/moby that referenced this pull request Jan 12, 2023
Sandboxed build invocation with xx currently doesn't set
the right name for target ARM architecture through switches
in CGO_CFLAGS and CGO_CXXFLAGS. This was previously fixed in moby#43474

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
Sandboxed build invocation with xx currently doesn't set
the right name for target ARM architecture through switches
in CGO_CFLAGS and CGO_CXXFLAGS. This was previously fixed in moby#43474

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
Sandboxed build invocation with xx currently doesn't set
the right name for target ARM architecture through switches
in CGO_CFLAGS and CGO_CXXFLAGS. This was previously fixed in moby#43474

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
Builds 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

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>
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.

None yet

5 participants