Skip to content

scripts/build/plugins: don't override CGO_ENABLED set by .variables - #5393

Merged
thaJeztah merged 1 commit into
docker:masterfrom
thaJeztah:fix_plugins_CGO_ENABLED
Sep 3, 2024
Merged

scripts/build/plugins: don't override CGO_ENABLED set by .variables#5393
thaJeztah merged 1 commit into
docker:masterfrom
thaJeztah:fix_plugins_CGO_ENABLED

Conversation

@thaJeztah

Copy link
Copy Markdown
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.123 Building static docker-helloworld
#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=ecb650e" -X "github.com/docker/cli/cli/version.BuildTime=2024-09-03T08:56:01Z" -X "github.com/docker/cli/cli/version.Version=pr-5387" -extldflags -static' -buildmode=pie github.com/docker/cli/cli-plugins/examples/helloworld
....

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

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 <github@gone.nl>
@thaJeztah
thaJeztahforce-pushed the fix_plugins_CGO_ENABLED branch from c7ca3cc to 9e29967CompareSeptember 3, 2024 09:49
@codecov-commenter

codecov-commenter commented Sep 3, 2024

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.94%. Comparing base (092b5f0) to head (9e29967).
Report is 266 commits behind head on master.

Additional details and impacted files
@@ Coverage Diff @@## master #5393 +/- ##
=======================================
Coverage 60.94% 60.94% =======================================
Files 304 304 Lines 21346 21346 =======================================
Hits 13010 13010 Misses 7407 7407 Partials 929 929 

@thaJeztahthaJeztah mentioned this pull request Sep 3, 2024
1 task
Sign up for freeto 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

@thaJeztah@codecov-commenter@vvoland