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

[27.x backport] scripts/build/plugins: don't override CGO_ENABLED set by .variables #5395

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 60.99%. Comparing base (1a0b6a7) to head (de2b49b).
Report is 8 commits behind head on 27.x.

Additional details and impacted files
@@           Coverage Diff           @@
##             27.x    #5395   +/-   ##
=======================================
  Coverage   60.99%   60.99%           
=======================================
  Files         304      304           
  Lines       21357    21357           
=======================================
  Hits        13026    13026           
  Misses       7402     7402           
  Partials      929      929           

@thaJeztah thaJeztah merged commit 3a15d5a into docker:27.x Sep 3, 2024
102 checks passed
@thaJeztah thaJeztah deleted the 27.x_backport_fix_plugins_CGO_ENABLED branch September 3, 2024 11:25
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.

3 participants