-
-
Notifications
You must be signed in to change notification settings - Fork 676
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
Fix gopackagesdriver for Go 1.18 by replicating stdlib #3083
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ import ( | |
"bytes" | ||
"encoding/json" | ||
"flag" | ||
"fmt" | ||
"go/build" | ||
"os" | ||
"path/filepath" | ||
|
@@ -145,6 +146,42 @@ func flatPackageForStd(execRoot string, pkg *goListPackage) *flatPackage { | |
return newPkg | ||
} | ||
|
||
// In Go 1.18, the standard library started using go:embed directives. | ||
// When Bazel runs this action, it does so inside a sandbox where GOROOT points | ||
// to an external/go_sdk directory that contains a symlink farm of all files in | ||
// the Go SDK. | ||
// If we run "go list" with that GOROOT, this action will fail because those | ||
// go:embed directives will refuse to include the symlinks in the sandbox. | ||
// | ||
// To work around this, cloneRoot creates a copy of external/go_sdk into a new | ||
// directory "root" while retaining its path relative to the root directory. | ||
// So "$OUTPUT_BASE/external/go_sdk" becomes | ||
// "$OUTPUT_BASE/root/external/go_sdk". | ||
// This ensures that file paths in the generated JSON are still valid. | ||
// | ||
// cloneRoot returns the new root directory and the new GOROOT we should run | ||
// under. | ||
func cloneRoot(execRoot, goroot string) (newRoot string, newGoroot string, err error) { | ||
relativeGoroot, err := filepath.Rel(abs(execRoot), abs(goroot)) | ||
if err != nil { | ||
// GOROOT has to be a subdirectory of execRoot. | ||
// Otherwise we're breaking the sandbox. | ||
return "", "", fmt.Errorf("GOROOT %q is not relative to execution root %q: %v", goroot, execRoot) | ||
} | ||
|
||
newRoot = filepath.Join(execRoot, "root") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am a bit nervous about writing things to execRoot. It's supposed to contain a symlink tree intended to be read-only. This may have unexpected result if there happen to be a |
||
newGoroot = filepath.Join(newRoot, relativeGoroot) | ||
if err := os.MkdirAll(newGoroot, 01755); err != nil { | ||
return "", "", err | ||
} | ||
|
||
if err := replicate(goroot, newGoroot, replicatePaths("src", "pkg/tool", "pkg/include")); err != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you need to replicate |
||
return "", "", err | ||
} | ||
|
||
return newRoot, newGoroot, nil | ||
} | ||
|
||
// stdliblist runs `go list -json` on the standard library and saves it to a file. | ||
func stdliblist(args []string) error { | ||
// process the args | ||
|
@@ -158,19 +195,22 @@ func stdliblist(args []string) error { | |
return err | ||
} | ||
|
||
execRoot, goroot, err := cloneRoot(".", os.Getenv("GOROOT")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "execRoot" is a special name in Bazel. It means the "The working directory for all actions" (https://bazel.build/docs/output_directories). Because you copied the symlink tree to to a different place, it's no longer the actual execRoot. Please rename it to something like "newBase" or "cloneBase". There are several variables/arguments in this file called "execRoot". After this pull request, they are no longer the actual execroot. Please also renaming all of them. |
||
if err != nil { | ||
return err | ||
} | ||
|
||
// Ensure paths are absolute. | ||
absPaths := []string{} | ||
for _, path := range filepath.SplitList(os.Getenv("PATH")) { | ||
absPaths = append(absPaths, abs(path)) | ||
} | ||
os.Setenv("PATH", strings.Join(absPaths, string(os.PathListSeparator))) | ||
os.Setenv("GOROOT", abs(os.Getenv("GOROOT"))) | ||
os.Setenv("GOROOT", goroot) | ||
// Make sure we have an absolute path to the C compiler. | ||
// TODO(#1357): also take absolute paths of includes and other paths in flags. | ||
os.Setenv("CC", abs(os.Getenv("CC"))) | ||
|
||
execRoot := abs(".") | ||
|
||
cachePath := abs(*out + ".gocache") | ||
defer os.RemoveAll(cachePath) | ||
os.Setenv("GOCACHE", cachePath) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you add
args.add("-sdk", go.sdk.root_file.dirname)
to here: https://github.com/bazelbuild/rules_go/blob/d606ba2c123826f153bbd037cf46e80b5eb8dc3e/go/private/actions/stdlib.bzl#L56-L57Then you can get a relative path of goroot from execRoot with
goenv.sdk
.