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/go, cmd/link: don't handle GOROOT_FINAL in linker #36118

Closed
jayconrod opened this issue Dec 12, 2019 · 6 comments
Closed

cmd/go, cmd/link: don't handle GOROOT_FINAL in linker #36118

jayconrod opened this issue Dec 12, 2019 · 6 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@jayconrod
Copy link
Contributor

GOROOT_FINAL is used to set the final location of GOROOT when building the toolchain and standard library.

When the compiler or assembler parse a file, they store file names and columns on AST nodes. If the file name is in GOROOT, the GOROOT path is replaced with the literal $GOROOT. This is done in cmd/internal/objabi.AbsFile. These file names eventually end up in debug data.

The linker expands these strings, replacing $GOROOT with the actual GOROOT or GOROOT_FINAL if that's set. This is done in cmd/link/internal/ld.expandGoroot.

We should avoid special handling for GOROOT_FINAL in the linker. Instead we should more unified support for -trimpath.

  • If -trimpath is given to the go command, all source paths in output files (including compiled .a files and intermediate generated cgo files) should be derived from package paths the same way.
  • The compiler, assembler, and cgo should support -trimpath flags with the same format and meaning. They should not have special handling for GOROOT paths.
  • If GOROOT_FINAL is set when the go command builds a package in GOROOT, an appropriate -trimpath flag should be given to the compiler, assembler, and cgo.
  • The linker should not need to transform paths.
@jayconrod jayconrod added the NeedsFix The path to resolution is known, but the work has not been done. label Dec 12, 2019
@jayconrod jayconrod added this to the Backlog milestone Dec 12, 2019
@gopherbot
Copy link

Change https://golang.org/cl/210943 mentions this issue: cmd/go: trim source paths in generated cgo and C source files

@ianlancetaylor
Copy link
Contributor

I don't quite see how we can replace GOROOT_FINAL, which could be in a completely different location, with -trimpath, which can only trim path prefixes.

@bcmills
Copy link
Contributor

bcmills commented Dec 13, 2019

-trimpath cannot replace all of the functionality of GOROOT_FINAL, but #18678 seems to make GOROOT_FINAL unnecessary on most platforms.

Are there still any platforms for which GOROOT_FINAL is still either necessary or widely used?

@jayconrod
Copy link
Contributor Author

When I opened this issue, I was just thinking about how the linker expands $GOROOT in file paths within pcln.go. It seems very strange to me. I think this means that users can set GOROOT_FINAL when linking any binary (not just something in cmd), and that affects stack traces for packages in std, even if those packages are installed elsewhere. Before I looked at this, I expected all the path manipulation to be done at compile time.

I don't quite see how we can replace GOROOT_FINAL, which could be in a completely different location, with -trimpath, which can only trim path prefixes.

-trimpath can replace a list of prefixes with alternate prefixes. So I think -trimpath /home/gopher/goroot=>/usr/local/goroot should work for compile-time path substitution at least. Agreed that it can't replace everything GOROOT_FINAL does today though.

-trimpath cannot replace all of the functionality of GOROOT_FINAL, but #18678 seems to make GOROOT_FINAL unnecessary on most platforms.
Are there still any platforms for which GOROOT_FINAL is still either necessary or widely used?

For the purpose of setting the default GOROOT, I wonder if we can accomplish that with an -X flag passed by cmd/go?

@ianlancetaylor
Copy link
Contributor

I don't know if anybody really uses GOROOT_FINAL but it's defined in make.bash and https://golang.org/cmd/go and https://golang.org/doc/install/source so I don't think we should break it without some sort of deprecation period.

@iwdgo
Copy link
Contributor

iwdgo commented Mar 5, 2024

GOROOT_FINAL has been removed following #62047 and the code mentioned now uses the GOROOT as configured.
Nothing seems left to do to resolve this issue.

@bcmills bcmills closed this as completed Mar 5, 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. NeedsFix The path to resolution is known, but the work has not been done.
Projects
Development

No branches or pull requests

5 participants