search

golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
122.92k stars 17.52k forks source link

all: Alpine Linux ships modified Go binaries #62053

Open rsc opened 1 year ago

rsc commented 1 year ago

One of the goals of Go 1.21's reproducible builds effort was to have the binaries we post on go.dev/dl match the binaries that other distributions post after building from source. Of course, to make that the case, the other distributions need to not patch the source.

For Go 1.21, we incorporated the various minor fixes that Ubuntu had accumulated, with the effect that Ubuntu's debs, while not exactly the same binaries, are easily reproduced. Specifically, the recipe is to build with GO386=softfloat and then run 'strip' on the binaries. For Go 1.22 it would be nice to make go build -ldflags=-s have the same effect as strip on ELF systems, so that GO386=softfloat GO_LDFLAGS=-s ./make.bash -distpack would reproduce the Ubuntu binaries, but even today, we have a trivial reproduction.

For Go 1.21, we also arranged that binaries built on Alpine would automatically use musl as the dynamic linker, and we arranged for the Go toolchain itself to not use dynamic binaries, so that the posted binaries would work unmodified on Alpine. Unfortunately, Alpine is still patching the source code of the Go toolchain, and so their binaries do not match the official ones. Specifically, they make two changes that affect the binaries:

0001-cmd-link-prefer-musl-s-over-glibc-s-ld.so-during-dyn.patch changes the preference list for what happens when running on a system with both the glibc and the musl dynamic linker installed. In this case the Go linker has a choice, and the default is to choose glibc, but Alpine wants Go to choose musl.

@nmeum, is there some standard config file on the system, perhaps some file in etc, that indicates that musl should be preferred over glibc? How does the clang or gcc toolchain on Alpine know? If there is some standard file, the Go linker could look at it too in Go 1.22. That would allow removing this patch.

0003-Prevent-Go-from-downloading-other-toolchain-versions.patch is hard-coding pathOnly = true, overriding the user's own environment settings. This is unnecessary and limits Go's functionality on Alpine. The commentary says:

// On Alpine, we don't allow downloading other toolchain since
// these are linked dynamically against glibc (i.e. do not work).

but this is not true: starting with Go 1.21.0, the toolchain binaries are statically linked pure Go programs, with no references to glibc, and so they should work fine on Alpine. (If this is not true I'd like to fix that.) It's true that downloading older toolchains won't work, but the way toolchain downloads work, automatic downloads only ever fetch newer toolchains. The incompatibility would only arise with explicit versions set in the environment like

GOTOOLCHAIN=go1.20.4 go test

It doesn't make sense to disable the entire feature just to avoid problems with this relatively uncommon use case. (Again, the most common use case by far will be downloading a newer toolchain, and those work.)

I understand that Alpine would prefer to set a system default, and we created the go.env for that purpose, and in fact https://git.alpinelinux.org/aports/tree/community/go/0004-go.env-Don-t-switch-Go-toolchain-version-as-directed.patch does exactly that, by changing the default from GOTOOLCHAIN=auto to GOTOOLCHAIN=local. That seems perfectly reasonable as a policy choice, and go.env is the place to set policy. Making the change there allows users to override the policy decision when they have different choices, either by setting an explicit environment variable or using go env -w.

@nmeum, given the existence of the 0004 patch, can the 0003 patch be removed?

gopherbot commented 1 year ago

Change https://go.dev/cl/520057 mentions this issue: runtime/testdata/testprog: use testenv.SyscallIsNotSupported to check syscall.Unshare

gopherbot commented 1 year ago

Change https://go.dev/cl/520056 mentions this issue: cmd/go: skip gccgo_link_c when cross-compiling

gopherbot commented 1 year ago

Change https://go.dev/cl/520055 mentions this issue: os: skip Chown tests for auxiliary groups that fail due to permission errors

bcmills commented 1 year ago

There are also several patches in the Alpine port working around test failures that, to my knowledge, were never filed as issues in the Go project. 😞

I would really appreciate it if folks would at least report these kinds of issues here so that we can fix them for everybody, rather than adding a one-off patch specific to one distribution.

bcmills commented 1 year ago

Hmm. On a closer look, I did find a few related issues:

bcmills commented 1 year ago

For completeness, one of the patches does have a corresponding issue explicitly mentioning the impact on Alpine:

(But it appears that patch was deleted as of Go 1.20?)

nmeum commented 1 year ago

Generally speaking, we do need to patch Go occasionally as Go releases break every now and then on musl architectures which are AFAIK not covered by your upstream CI (musl-s390x, musl-i386, musl-riscv64, musl-ppc64le, …). See for example #51787, #58385, #52336, #52337, et cetera. Maybe it makes sense to add support for more musl targets to the Golang CI?

I will elaborate further on specific questions below.

0001-cmd-link-prefer-musl-s-over-glibc-s-ld.so-during-dyn.patch changes the preference list for what happens when running on a system with both the glibc and the musl dynamic linker installed. In this case the Go linker has a choice, and the default is to choose glibc, but Alpine wants Go to choose musl.

@nmeum, is there some standard config file on the system, perhaps some file in etc, that indicates that musl should be preferred over glibc? How does the clang or gcc toolchain on Alpine know? If there is some standard file, the Go linker could look at it too in Go 1.22. That would allow removing this patch.

Unfortunately, there is not really a portable way to detect this AFAIK. For clang/gcc the preferred linker is IIRC determined when clang/gcc itself is compiled so this is not an issue (CC: @ncopa). Prior to 3315066f46d5dce3e4474bdcde0997d688c79436 and #54197 Go used to do this as well I think. From the top of my head, I can't think of an upstreamable solution right now which is also why I never raised this issue upstream.

0003-Prevent-Go-from-downloading-other-toolchain-versions.patch is hard-coding pathOnly = true, overriding the user's own environment settings. This is unnecessary and limits Go's functionality on Alpine. The commentary says:

// On Alpine, we don't allow downloading other toolchain since
// these are linked dynamically against glibc (i.e. do not work).

but this is not true: starting with Go 1.21.0, the toolchain binaries are statically linked pure Go programs, with no references to glibc, and so they should work fine on Alpine. (If this is not true I'd like to fix that.) It's true that downloading older toolchains won't work, but the way toolchain downloads work, automatic downloads only ever fetch newer toolchains.

I wasn't aware that Go ≥1.21.0 binaries are fully statically linked. I only tested this feature with older Go versions (i.e. Go 1.20) and for those, it doesn't work, as you rightly pointed out. However, Alpine is generally wary of software that downloads or relies on pre-built binaries, so this might warrant further discussion on the Alpine side. See also: a prior related discussion regarding GOPROXY/GOSUMDB as well as a similar discussion in the Debian bug tracker.

Nonetheless, thank you for bringing this to my attention; I will follow up on this!

nmeum commented 1 year ago

Regarding the general comment made by @bcmills:

There are also several patches in the Alpine port working around test failures that, to my knowledge, were never filed as issues in the Go project. 😞

I would really appreciate it if folks would at least report these kinds of issues here so that we can fix them for everybody, rather than adding a one-off patch specific to one distribution.

I always tried to open Go upstream issues for our patches whenever I was of the opinion that the issue could/should be fixed upstream. As such, there are several musl-compatibility issues in the Go bug tracker that have been reported by me, and in the same spirit, I also upstreamed our entire gccgo patchset earlier this year. In fact, I try to do this for every package I maintain downstream. It's simply not true that we are not opening upstream issues for test failures. Also keep in mind that this is a lot of work that downstream packagers are usually not paid for and hence perform in their free time. Furthermore, my experience with Go upstream regarding musl compatibility issues hasn't necessarily been positive. Last year, I was basically told that musl is not a supported platform, which is not exactly encouraging.

rsc commented 1 year ago

@nmeum in response to https://github.com/golang/go/issues/51787#issuecomment-1072397501, I don't remember the order of events so I can't say whether that comment was true at the time, but it's not true now - we do have a linux-amd64-alpine builder. You can see it on https://build.golang.org/

rsc commented 1 year ago

@nmeum what do you think about the Go linker defaulting on Linux to the dynamic linker of /bin/sh (assuming it is dynamically linked)? That seems like a reasonable way to answer "what is the dynamic linker on this system?" and it would correctly work on Alpine and Ubuntu.

rsc commented 1 year ago

@nmeum regarding defaults for downloading new toolchains, we completely understand Alpine wanting to set different defaults for GOPROXY, GOSUMDB, and GOTOOLCHAIN, which is why we moved them into go.env: that way they can be changed in "source" instead of in "binary": anyone verifying the Alpine Go builds will see the changed go.env and ideally see unchanged binaries.

The 0004 patch takes care of setting GOTOOLCHAIN=local in go.env to set the default, and we do not object to that at all. That's Alpine's prerogative, full stop. (You may want to consider GOTOOLCHAIN=path instead, but I completely understand wanting a default that's not GOTOOLCHAIN=auto.)

The 0003 patch, on the other hand, disables downloads even when the user has tried to enable them by explicitly setting GOTOOLCHAIN=auto. That seems very unfortunate. If a user explicitly wants to opt back in, they should be able to do that without having to install a standard Go toolchain first.

If the 0003 patch didn't mean to target GOTOOLCHAIN=auto and only wanted to target GOTOOLCHAIN=go1.20 (which does not work on Alpine because Go 1.20 and earlier didn't run on Alpine out of the box), then I would be willing to have the toolchain download check for /etc/alpine-release to detect being on Alpine (or maybe ldd /bin/sh to detect musl) and print a nice error instead of downloading and failing to run those earlier toolchains.

nmeum commented 1 year ago

I will follow up on this!

The patch which forcefully sets pathOnly = true has been removed after some discussion.

See: https://git.alpinelinux.org/aports/commit/?id=f21344fec95c820d9d1945e6bee27678268472f8

nmeum commented 1 year ago

@nmeum what do you think about the Go linker defaulting on Linux to the dynamic linker of /bin/sh (assuming it is dynamically linked)? That seems like a reasonable way to answer "what is the dynamic linker on this system?" and it would correctly work on Alpine and Ubuntu.

I suppose that could work on most systems, though there are some edge cases where it doesn't (e.g. statically linked /bin/sh). An alternative/additional heuristic would be consulting the configuration of the C toolchain (if installed, which I assume it will often be when Go attempts to link against the libc?). For example, gcc -dumpmachine and clang -dumpmachine will print the triplet which includes musl on Alpine (the full triplet is x86_64-alpine-linux-musl).

The 0003 patch, on the other hand, disables downloads even when the user has tried to enable them by explicitly setting GOTOOLCHAIN=auto. That seems very unfortunate. If a user explicitly wants to opt back in, they should be able to do that without having to install a standard Go toolchain first.

Originally, it was explicitly intended to prevent users from setting GOTOOLCHAIN=auto. We package a lot of Go software in Alpine and some of that software invokes the go command indirectly (e.g. through Makefiles or Shell scripts). As an example, see the build instructions for Docker. Without the patch, such packages can theoretically set GOTOOLCHAIN=auto which we want to prevent as we want to build all software in our package repository using our packaged toolchain. For this purpose, it would be ideal if GOTOOLCHAIN could only be modified by editing a system-wide configuration file and not via environment variables.

I don't remember the order of events so I can't say whether that comment was true at the time, but it's not true now - we do have a linux-amd64-alpine builder.

I am aware that this is the case nowadays, keep in mind though that Alpine supports architectures that are not covered by your CI, see the first paragraph of my previous comment in https://github.com/golang/go/issues/62053#issuecomment-1686849350.

nmeum commented 1 year ago

One more thing that came to mind regarding the general goal of having “the binaries we post on go.dev/dl match the binaries that other distributions post after building from source”: Alpine builds Go itself with GO_LDFLAGS=-buildmode=pie I assume this is not the case for the binaries posted on go.dev/dl.

zikaeroh commented 1 year ago

It's likely that these deserve other issues depending on what the overall goal is, but I'll note that Alpine/Ubuntu are not the only major distributions to be shipping binaries that aren't going to match those from go.dev/dl; e.g.:

Apologizes if these are already known, of course. This isn't exhaustive, just what was easily searchable (mostly via https://pkgs.org/).

(Unrelated, but I have also noticed quite a few distros which are still doing things that are no longer useful on modern versions of Go, e.g. prebuilding std and so on; IIRC that stuff all goes into the cache. Also a lot of CC/CXX setting to preset those, which I also think doesn't do anything anymore?)

tie commented 1 year ago

NixOS has a number of patches, mostly relating to the differing locations of things like localization/tzdata/etc

This has been a main blocker (at least for me) in using Go packaged in Nixpkgs since compiled binaries (not Go itself but user programs) cannot be reproduced outside of the Nixpkgs environment due to the patches applied to the standard library. See also https://github.com/NixOS/nixpkgs/issues/125198

Perhaps we can add variables that can be set via -ldflags=-X=… so that distributions like NixOS can build packaged Go programs with system paths they need without patching Go toolchain? E.g. -ldflags=-X=time.tzdata=/nix/store/… that is recorded in the binary and can be inspected with go version -m.