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

cmd/dist: tools built with GOROOT_FINAL cannot locate it when copied to another location #61921

Closed
g4s8 opened this issue Aug 10, 2023 · 15 comments
Assignees
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Unfortunate
Milestone

Comments

@g4s8
Copy link
Contributor

g4s8 commented Aug 10, 2023

What version of Go are you using (go version)?

$ go version
go version go1.21.0 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ /usr/lib/go/bin/go env

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/g4s8/.cache/go-build'
GOENV='/home/g4s8/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/g4s8/go/pkg/mod'
GOOS='linux'
GOPATH='/home/g4s8/go'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/lib/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/lib/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.21.0'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -fmessage-length=0 -ffile-prefix-map=/tmp/go-build4273986275=/tmp/go-build -gno-record-gcc-switches'

What did you do?

I'm trying to build go 1.21.0 from sources for Void Linux go package: https://github.com/void-linux/void-packages

The steps to build it (inside go repo directory):

unset GCC CC CXX LD CFLAGS
unset CGO_CXXFLAGS CGO_ENABLED

export GOROOT_FINAL=/usr/lib/go
export GOROOT=$PWD
export GOARCH=amd64

cd src
bash make.bash -v

sudo rm -f /usr/bin/go
sudo rm -fr /usr/lib/go/
sudo cp  ../bin/* -t /usr/bin
sudo cp -r .. /usr/lib/go

What did you expect to see?

When running /usr/bin/go version (or any other go command, like go build, etc) to see

go version go1.21.0 linux/amd64

What did you see instead?

When running go command or /usr/bin/go:

$ go version
go: cannot find GOROOT directory: 'go' binary is trimmed and GOROOT is not set

$ go build
go: cannot find GOROOT directory: 'go' binary is trimmed and GOROOT is not set

If I run /usr/lib/go/bin/go, then all commands works fine:

$ /usr/lib/go/bin/go version
go version go1.21.0 linux/amd64

If I run exactly the same build commands for go version 1.20.7, it's working fine:

$ go version
go version go1.20.7 linux/amd64

or

$ /usr/bin/go version
go version go1.20.7 linux/amd64
@mknyszek mknyszek changed the title GOROOT_FINAL environment doesn't work for go1.21.0 cmd/go: GOROOT_FINAL doesn't work for go1.21.0 Aug 10, 2023
@mknyszek mknyszek added this to the Backlog milestone Aug 10, 2023
@mknyszek mknyszek added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go labels Aug 10, 2023
@mknyszek
Copy link
Contributor

@bcmills bcmills changed the title cmd/go: GOROOT_FINAL doesn't work for go1.21.0 cmd/dist: GOROOT_FINAL doesn't work for go1.21.0 Aug 10, 2023
@bcmills bcmills self-assigned this Aug 10, 2023
@bcmills bcmills removed the GoCommand cmd/go label Aug 10, 2023
@bcmills bcmills modified the milestones: Backlog, Go1.22 Aug 10, 2023
@bcmills
Copy link
Contributor

bcmills commented Aug 10, 2023

This is almost certainly a consequence of https://go.dev/cl/463740 (for #24904).

When built with -trimpath, the go command uses os.Executable to find its GOROOT, expecting that it is installed at $GOROOT/bin/go, and lots of Go programs (especially tests) expect to be able to find the go command at that location (from a configuration that gives only a GOROOT path).

@g4s8, is there a reason your script is using cp to make copies of the binaries in GOROOT/bin, instead of using ln to make /usr/bin/go a symbolic link? It seems like that would be a simpler solution, and would allow the Go toolchain installed on Void Linux to use the same reproducible binaries that are distributed through the Go project.

@bcmills bcmills changed the title cmd/dist: GOROOT_FINAL doesn't work for go1.21.0 cmd/dist: tools built with GOROOT_FINAL cannot locate it when copied to another location Aug 10, 2023
@bcmills
Copy link
Contributor

bcmills commented Aug 10, 2023

I wonder if we should change cmd/dist to omit the -trimpath flag when GOROOT_FINAL is set.

(But I would still recommend not setting it, and using symlinks instead. 🤷‍♂️)

@bcmills
Copy link
Contributor

bcmills commented Aug 10, 2023

I wonder if we should change cmd/dist to omit the -trimpath flag when GOROOT_FINAL is set.

That's extra tricky because I think the releasebot sets GOROOT_FINAL explicitly. 🤔

@golang/release, how much work would it be to stop setting GOROOT_FINAL in the environment for releases at Go 1.21 and later?

@dmitshur
Copy link
Contributor

dmitshur commented Aug 10, 2023

It wouldn't be much work at all. (The relevant code is in BuildletStep.makeEnv.) If it no longer needs to be set and dropping it is safe, I think we should get rid of it (and I'd be happy to help or do it).

Is #51504 relevant to this issue?

@bcmills
Copy link
Contributor

bcmills commented Aug 11, 2023

Is #51504 relevant to this issue?

I don't think it's particularly closely related. The problem there occurs when running tests before moving the binaries to GOROOT_FINAL, but the problem here is running go commands after copying the binaries to their final locations (and copying the go executable to some other location outside of GOROOT_FINAL).

@rsc
Copy link
Contributor

rsc commented Aug 11, 2023

If /usr/bin/go is a symlink to /usr/lib/go/bin/go, I believe that will work correctly. Can your packaging do that?

It would be ideal if we could remove GOROOT_FINAL entirely. The distribution builds are position-independent and therefore reproducible. That's an important feature, and it seems like a mistake to have a flag to turn that off. However, we also want people to be able to package Go for Linux distributions. :-)

@bcmills bcmills added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Aug 11, 2023
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 11, 2023
@bcmills bcmills added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 11, 2023
@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Aug 11, 2023
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/518837 mentions this issue: cmd/dist: omit -trimpath if GOROOT_FINAL is set

@bcmills bcmills added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Aug 11, 2023
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 11, 2023
@g4s8
Copy link
Contributor Author

g4s8 commented Aug 15, 2023

Linking /usr/bin/go -> /usr/lib/go/bin/go works fine. Maybe this line should be removed (or updated?) from src/make.bash to avoid confusion?

# GOROOT_FINAL: The expected final Go root, baked into binaries.
# The default is the location of the Go tree during the build.

@bcmills
Copy link
Contributor

bcmills commented Aug 15, 2023

Maybe this line should be removed (or updated?) from src/make.bash to avoid confusion?

I think we should remove it from pretty much everything, for which I have filed proposal #62047.

@kat-co
Copy link

kat-co commented Aug 15, 2023

If /usr/bin/go is a symlink to /usr/lib/go/bin/go, I believe that will work correctly. Can your packaging do that?

I have just done something very similar while packaging v1.21.0 for Guix, and it appears to have worked. The only remaining issue I can foresee is if other Guix maintainers have an issue with the new link structure.

@tspearconquest
Copy link

I'm building 1.21 from source on alpine using the 1.20 docker-library dockerfile customized to pull down the 1.21 sources rather than the binary package: https://github.com/docker-library/golang/blob/master/1.20/alpine3.18/Dockerfile but with line 13 changed to 1.21.0

Certain packages like golang.org/x/net are included in the base distribution and when there is a CVE identified, I wanted to be able to patch it just before running make.bash rather than making the dev team have to think about/manage CVEs in the base image they use to build their apps, which is why I'm avoiding the binary packages and building from source. This is for an internal build; not anything published publicly.

When I build 1.21.0, I'm seeing this issue. The dockerfile doesn't appear to set GOROOT or GOROOT_FINAL. It simply sets PATH=/usr/local/go/bin:$PATH early; way before the build happens, and then installs go into /usr/local/go.

Is the right fix for me to just add ENV GOROOT=/usr/local/go near the end of the dockerfile, perhaps at line 112?

@bcmills
Copy link
Contributor

bcmills commented Aug 17, 2023

@tspearconquest, as far as I can tell that Dockerfile is not moving the final binaries, and should be resolving go to /usr/local/go/bin/go, from which it should be able to find GOROOT using os.Executable().

That is: the problem you are reporting is not obviously related to this one.

Could you please file a new issue with more detail on the failure mode you're seeing?

@tspearconquest
Copy link

Sure; I did simplify the customizations a bit. We use a multi-stage docker build; so we're doing COPY /usr/local/go /usr/local/go in the final stage, and as a guess, since make.bash is setting GOROOT_FINAL itself but GOROOT is not being re-set in the final stage, that's probably why go is giving this error. Full context in #62119

Cyanoxygen added a commit to AOSC-Dev/aosc-os-abbs that referenced this issue Sep 3, 2023
- Before this, go and gofmt was installed to /usr/bin, which confuses Go
  as it can not figure out $GOROOT automatically by os.Executable, see
  [1] for details.
  By installing binaries to $GOROOT/bin (GOROOT points to /usr/lib), go
  can now figure out its GOROOT.

- Provide symlinks in /usr/bin/go{,fmt}-google to eusure it is there.

- Update alternatives file to point corresponding executables to the
  one installed in $GOROOT.

[1]: golang/go#61921 (comment)
Cyanoxygen added a commit to AOSC-Dev/aosc-os-abbs that referenced this issue Sep 3, 2023
- Before this, go and gofmt was installed to /usr/bin, which confuses Go
  as it can not figure out $GOROOT automatically by os.Executable, see
  [1] for details.
  By installing binaries to $GOROOT/bin (GOROOT points to /usr/lib/go),
  go can now figure out its GOROOT.

- Provide symlinks in /usr/bin/go{,fmt}-google to ensure it is there.

- Update alternatives file to point corresponding executables to the
  one installed in $GOROOT.

[1]: golang/go#61921 (comment)
Cyanoxygen added a commit to AOSC-Dev/aosc-os-abbs that referenced this issue Sep 13, 2023
- Before this, go and gofmt was installed to /usr/bin, which confuses Go
  as it can not figure out $GOROOT automatically by os.Executable, see
  [1] for details.
  By installing binaries to $GOROOT/bin (GOROOT points to /usr/lib/go),
  go can now figure out its GOROOT.

- Provide symlinks in /usr/bin/go{,fmt}-google to ensure it is there.

- Update alternatives file to point corresponding executables to the
  one installed in $GOROOT.

[1]: golang/go#61921 (comment)
Cyanoxygen added a commit to AOSC-Dev/aosc-os-abbs that referenced this issue Sep 21, 2023
- Before this, go and gofmt was installed to /usr/bin, which confuses Go
  as it can not figure out $GOROOT automatically by os.Executable, see
  [1] for details.
  By installing binaries to $GOROOT/bin (GOROOT points to /usr/lib/go),
  go can now figure out its GOROOT.

- Provide symlinks in /usr/bin/go{,fmt}-google to ensure it is there.

- Update alternatives file to point corresponding executables to the
  one installed in $GOROOT.

[1]: golang/go#61921 (comment)
MingcongBai pushed a commit to AOSC-Dev/aosc-os-abbs that referenced this issue Sep 21, 2023
- Before this, go and gofmt was installed to /usr/bin, which confuses Go
  as it can not figure out $GOROOT automatically by os.Executable, see
  [1] for details.
  By installing binaries to $GOROOT/bin (GOROOT points to /usr/lib/go),
  go can now figure out its GOROOT.

- Provide symlinks in /usr/bin/go{,fmt}-google to ensure it is there.

- Update alternatives file to point corresponding executables to the
  one installed in $GOROOT.

[1]: golang/go#61921 (comment)
Cyanoxygen added a commit to AOSC-Dev/aosc-os-abbs that referenced this issue Sep 26, 2023
- Before this, go and gofmt was installed to /usr/bin, which confuses Go
  as it can not figure out $GOROOT automatically by os.Executable, see
  [1] for details.
  By installing binaries to $GOROOT/bin (GOROOT points to /usr/lib/go),
  go can now figure out its GOROOT.

- Provide symlinks in /usr/bin/go{,fmt}-google to ensure it is there.

- Update alternatives file to point corresponding executables to the
  one installed in $GOROOT.

[1]: golang/go#61921 (comment)
@seankhliao seankhliao removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Oct 7, 2023
@bcmills bcmills modified the milestones: Go1.22, Backlog, Unplanned Feb 1, 2024
@bcmills
Copy link
Contributor

bcmills commented Feb 1, 2024

#62047 is approved, so the resolution here is that we will remove support for GOROOT_FINAL rather than making it work for copied binaries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Unfortunate
Projects
None yet
Development

No branches or pull requests

9 participants