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: drop support for GOROOT_FINAL #62047

Closed
bcmills opened this issue Aug 15, 2023 · 7 comments
Closed

cmd: drop support for GOROOT_FINAL #62047

bcmills opened this issue Aug 15, 2023 · 7 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FixPending Issues that have a fix which has not yet been reviewed or submitted. GoCommand cmd/go Proposal Proposal-Accepted
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Aug 15, 2023

Prior to Go 1.21, setting the GOROOT_FINAL variable during the make scripts changed the GOROOT value embedded in the tools installed in $GOROOT/bin and $GOROOT/pkg/tool. That allowed those tools (primarily cmd/go) to locate GOROOT even if it (or just its binaries) was symlinked or copied to a different install location.

In Go 1.9 (#18678), to reduce confusion and boilerplate from explicit GOROOT settings, cmd/go was changed to use os.Executable to locate its own binary and then derive GOROOT from the fact that cmd/go is normally installed in $GOROOT/bin. cmd/go then passes that path to other tools explicitly as needed. As long as the go command invoked by the user is actually resolved in $GOROOT/bin (possibly via a symlink), it will infer the correct GOROOT regardless of what value may happen to be baked in.

As of Go 1.21, in order to provide reproducible builds of the Go toolchain (#57120), cmd/dist started building releases with the -trimpath flag set, which incidentally caused GOROOT_FINAL to have no effect (#61921). (Development versions of toolchains are still built without -trimpath by default, so GOROOT_FINAL may affect those builds somewhat.)

So GOROOT_FINAL is only really helpful if all of the following occur:

  • The toolchain is being built only for a non-release version,
  • GOROOT is moved after the make script is run,
  • the cmd/go binary is copied or hard-linked (not symlinked!) to some location outside of GOROOT/bin, and
  • the copied cmd/go binary is invoked without setting GOROOT explicitly.

I believe that the intersection of those conditions is “basically never”:


On the other hand, GOROOT_FINAL has long been a source of bugs and complexity in the Go project (see https://github.com/golang/go/issues?q=is%3Aissue+GOROOT_FINAL+in%3Atitle). In particular, tests of the toolchain itself may need to take GOROOT_FINAL into account when they inspect or run artifacts built during the test.

I propose that we:

  • Drop support for custom GOROOT_FINAL settings in cmd/dist, cmd/go, and the make scripts.
  • Either replace the use of GOROOT_FINAL in cmd/link with an explict flag (--trimgoroot?), or change cmd/go to always set GOROOT_FINAL to be equal to either GOROOT (if -trimpath is off) or the literal string "$GOROOT" (if -trimpath is on).
  • Remove public-facing documentation that refers to GOROOT_FINAL.
    • (Note that the documentation for cmd/link already does not mention its use of GOROOT_FINAL.)
@rittneje
Copy link

We use GOROOT_FINAL to prevent our build server's path from being written into the resulting toolchain. It sounds like now make.bash will end up building the toolchain with -trimpath set (by default?), which achieves the same goal. Is that right?

@bcmills
Copy link
Contributor Author

bcmills commented Aug 16, 2023

@rittneje, correct.

The recommended way to prevent build server paths from being written is to use -trimpath, which is already what cmd/dist (and therefore make.bash) does by default for release versions as of Go 1.21.

@rsc
Copy link
Contributor

rsc commented Oct 3, 2023

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Oct 11, 2023

Have all remaining concerns about this proposal been addressed?
We haven't seen or heard any objections from packagers, at least that I'm aware of.

@rsc
Copy link
Contributor

rsc commented Oct 26, 2023

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

Remove all use of GOROOT_FINAL in the tree, since it stopped working in Go 1.21 and no one has complained. Specifically:

  1. Drop support for custom GOROOT_FINAL settings in cmd/dist, cmd/go, and the make scripts.
  2. In cmd/link, use GOROOT as GOROOT_FINAL; if -trimpath is set, keep doing what we already do, namely use the package paths as in math/abs.go or cmd/compile/main.go).
  3. Remove public-facing documentation that refers to GOROOT_FINAL.

@rsc
Copy link
Contributor

rsc commented Nov 2, 2023

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

Remove all use of GOROOT_FINAL in the tree, since it stopped working in Go 1.21 and no one has complained. Specifically:

  1. Drop support for custom GOROOT_FINAL settings in cmd/dist, cmd/go, and the make scripts.
  2. In cmd/link, use GOROOT as GOROOT_FINAL; if -trimpath is set, keep doing what we already do, namely use the package paths as in math/abs.go or cmd/compile/main.go).
  3. Remove public-facing documentation that refers to GOROOT_FINAL.

@rsc rsc changed the title proposal: cmd: drop support for GOROOT_FINAL cmd: drop support for GOROOT_FINAL Nov 2, 2023
@rsc rsc modified the milestones: Proposal, Backlog Nov 2, 2023
@gopherbot
Copy link

Change https://go.dev/cl/539975 mentions this issue: cmd: remove support for GOROOT_FINAL

@mknyszek mknyszek assigned cherrymui and unassigned cherrymui Nov 8, 2023
@dmitshur dmitshur modified the milestones: Backlog, Go1.23 Feb 1, 2024
@dmitshur dmitshur added the FixPending Issues that have a fix which has not yet been reviewed or submitted. label Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FixPending Issues that have a fix which has not yet been reviewed or submitted. GoCommand cmd/go Proposal Proposal-Accepted
Projects
Status: Accepted
Development

No branches or pull requests

6 participants