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

[26.1 backport[] scripts/build/plugins: don't override CGO_ENABLED set by .variables #5396

Merged

Conversation

thaJeztah
Copy link
Member

The .variables sets CGO_ENABLED=1 on arm;

if [ -z "$CGO_ENABLED" ]; then
case "$(go env GOOS)" in
linux)
case "$(go env GOARCH)" in
amd64|arm64|arm|s390x|riscv64)
CGO_ENABLED=1
;;
*)
CGO_ENABLED=0
;;
esac
;;
And if enabled, it sets -buildmode=pie;
if [ "$CGO_ENABLED" = "1" ] && [ "$(go env GOOS)" != "windows" ]; then
case "$(go env GOARCH)" in
mips*|ppc64)
# pie build mode is not supported on mips architectures
;;
*)
GO_BUILDMODE="-buildmode=pie"
;;
esac
fi

But that looks to be conflicting with the hardcoded CGO_ENABLED=0 in this script, which causes the build to fail on go1.22;

> [build-plugins 1/1] RUN --mount=ro --mount=type=cache,target=/root/.cache     xx-go --wrap &&     TARGET=/out ./scripts/build/plugins e2e/cli-plugins/plugins/*:
0.127 Building static docker-helloworld
0.127 + CGO_ENABLED=0
0.127 + GO111MODULE=auto
0.127 + go build -o /out/plugins-linux-arm/docker-helloworld -tags ' osusergo' -ldflags ' -X "github.com/docker/cli/cli/version.GitCommit=5c123b1" -X "github.com/docker/cli/cli/version.BuildTime=2024-09-02T13:52:17Z" -X "github.com/docker/cli/cli/version.Version=pr-5387" -extldflags -static' -buildmode=pie github.com/docker/cli/cli-plugins/examples/helloworld
0.135 -buildmode=pie requires external (cgo) linking, but cgo is not enabled

This patch sets the CGO_ENABLED variable before sourcing .variables, so that other variables which are conditionally set are handled correctly.

Before this PR:

#18 [build-plugins 1/1] RUN --mount=ro --mount=type=cache,target=/root/.cache     xx-go --wrap &&     TARGET=/out ./scripts/build/plugins e2e/cli-plugins/plugins/*
#18 0.123 Building static docker-helloworld
#18 0.124 + CGO_ENABLED=0
#18 0.124 + GO111MODULE=auto
#18 0.124 + go build -o /out/plugins-linux-arm/docker-helloworld -tags ' osusergo' -ldflags ' -X "github.com/docker/cli/cli/version.GitCommit=c8c402e" -X "github.com/docker/cli/cli/version.BuildTime=2024-09-03T08:28:25Z" -X "github.com/docker/cli/cli/version.Version=pr-5381" -extldflags -static' -buildmode=pie github.com/docker/cli/cli-plugins/examples/helloworld
....

With this PR:

#18 [build-plugins 1/1] RUN --mount=ro --mount=type=cache,target=/root/.cache     xx-go --wrap &&     TARGET=/out ./scripts/build/plugins e2e/cli-plugins/plugins/*
#18 0.110 Building static docker-helloworld
#18 0.110 + GO111MODULE=auto
#18 0.110 + go build -o /out/plugins-linux-arm/docker-helloworld -tags '' -ldflags ' -X "github.com/docker/cli/cli/version.GitCommit=050d9d6" -X "github.com/docker/cli/cli/version.BuildTime=2024-09-03T09:19:05Z" -X "github.com/docker/cli/cli/version.Version=pr-5387"' github.com/docker/cli/cli-plugins/examples/helloworld
....

(cherry picked from commit 9e29967)

The `.variables` sets `CGO_ENABLED=1` on arm; https://github.com/docker/cli/blob/b0c41b78d8c80c5f8faa736cf3c74dc31092e443/scripts/build/.variables#L57-L68
And if enabled, it sets `-buildmode=pie`; https://github.com/docker/cli/blob/b0c41b78d8c80c5f8faa736cf3c74dc31092e443/scripts/build/.variables#L79-L88

But that looks to be conflicting with the hardcoded `CGO_ENABLED=0` in
this script, which causes the build to fail on go1.22;

    > [build-plugins 1/1] RUN --mount=ro --mount=type=cache,target=/root/.cache     xx-go --wrap &&     TARGET=/out ./scripts/build/plugins e2e/cli-plugins/plugins/*:
    0.127 Building static docker-helloworld
    0.127 + CGO_ENABLED=0
    0.127 + GO111MODULE=auto
    0.127 + go build -o /out/plugins-linux-arm/docker-helloworld -tags ' osusergo' -ldflags ' -X "github.com/docker/cli/cli/version.GitCommit=5c123b1" -X "github.com/docker/cli/cli/version.BuildTime=2024-09-02T13:52:17Z" -X "github.com/docker/cli/cli/version.Version=pr-5387" -extldflags -static' -buildmode=pie github.com/docker/cli/cli-plugins/examples/helloworld
    0.135 -buildmode=pie requires external (cgo) linking, but cgo is not enabled

This patch sets the CGO_ENABLED variable before sourcing `.variables`,
so that other variables which are conditionally set are handled correctly.

Before this PR:

    docker#18 [build-plugins 1/1] RUN --mount=ro --mount=type=cache,target=/root/.cache     xx-go --wrap &&     TARGET=/out ./scripts/build/plugins e2e/cli-plugins/plugins/*
    docker#18 0.123 Building static docker-helloworld
    docker#18 0.124 + CGO_ENABLED=0
    docker#18 0.124 + GO111MODULE=auto
    docker#18 0.124 + go build -o /out/plugins-linux-arm/docker-helloworld -tags ' osusergo' -ldflags ' -X "github.com/docker/cli/cli/version.GitCommit=c8c402e" -X "github.com/docker/cli/cli/version.BuildTime=2024-09-03T08:28:25Z" -X "github.com/docker/cli/cli/version.Version=pr-5381" -extldflags -static' -buildmode=pie github.com/docker/cli/cli-plugins/examples/helloworld
    ....

With this PR:

    docker#18 [build-plugins 1/1] RUN --mount=ro --mount=type=cache,target=/root/.cache     xx-go --wrap &&     TARGET=/out ./scripts/build/plugins e2e/cli-plugins/plugins/*
    docker#18 0.110 Building static docker-helloworld
    docker#18 0.110 + GO111MODULE=auto
    docker#18 0.110 + go build -o /out/plugins-linux-arm/docker-helloworld -tags '' -ldflags ' -X "github.com/docker/cli/cli/version.GitCommit=050d9d6" -X "github.com/docker/cli/cli/version.BuildTime=2024-09-03T09:19:05Z" -X "github.com/docker/cli/cli/version.Version=pr-5387"' github.com/docker/cli/cli-plugins/examples/helloworld
    ....

Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 9e29967)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Sep 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.36%. Comparing base (dea625c) to head (8b95529).
Report is 2 commits behind head on 26.1.

Additional details and impacted files
@@           Coverage Diff           @@
##             26.1    #5396   +/-   ##
=======================================
  Coverage   61.36%   61.36%           
=======================================
  Files         298      298           
  Lines       20712    20712           
=======================================
  Hits        12709    12709           
  Misses       7102     7102           
  Partials      901      901           

@vvoland vvoland merged commit 4edbb63 into docker:26.1 Sep 5, 2024
101 checks passed
@thaJeztah thaJeztah deleted the 26.1_backport_fix_plugins_CGO_ENABLED branch September 9, 2024 19:27
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