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

Fix gopackagesdriver for Go 1.18 by replicating stdlib and add test for stdliblist.go #3157

Merged
merged 18 commits into from
May 19, 2022
1 change: 1 addition & 0 deletions .bazelci/presubmit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ tasks:
- "-@org_golang_x_text//language:language_test"
- "-@org_golang_x_tools//cmd/splitdwarf/internal/macho:macho_test"
- "-@test_chdir_remote//sub:go_default_test"
- "-//go/tools/builders:stdliblist_test" # fails on Windows due to #2491
- "-//tests:buildifier_test" # requires bash
- "-//tests/core/cgo:dylib_client"
- "-//tests/core/cgo:dylib_test"
Expand Down
1 change: 1 addition & 0 deletions go/private/actions/stdlib.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ def _should_use_sdk_stdlib(go):
def _build_stdlib_list_json(go):
out = go.declare_file(go, "stdlib.pkg.json")
args = go.builder_args(go, "stdliblist")
args.add("-sdk", go.sdk.root_file.dirname)
args.add("-out", out)
go.actions.run(
inputs = go.sdk_files,
Expand Down
14 changes: 14 additions & 0 deletions go/tools/builders/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,20 @@ go_test(
],
)

go_test(
name = "stdliblist_test",
size = "small",
srcs = [
"env.go",
"flags.go",
"replicate.go",
"stdliblist.go",
"stdliblist_test.go",
],
data = ["@go_sdk//:files"],
deps = ["//go/tools/bazel"],
)

filegroup(
name = "builder_srcs",
srcs = [
Expand Down
3 changes: 2 additions & 1 deletion go/tools/builders/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ type env struct {
// configured with those flags.
func envFlags(flags *flag.FlagSet) *env {
env := &env{}
flags.StringVar(&env.sdk, "sdk", "", "Path to the Go SDK.")
flags.StringVar(&env.sdk, "sdk", "", "Relative path to the Go SDK.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This set of flags are reused in other places, which may not require it to be relative path. Let's instead check it in stdliblist to verify it's a relative path

Copy link
Contributor Author

@xytan0056 xytan0056 May 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

if filepath.Abs(goenv.sdk) {
   return error
}

flags.Var(&tagFlag{}, "tags", "List of build tags considered true.")
flags.StringVar(&env.installSuffix, "installsuffix", "", "Standard library under GOROOT/pkg")
flags.BoolVar(&env.verbose, "v", false, "Whether subprocess command lines should be printed")
Expand Down Expand Up @@ -121,6 +121,7 @@ func (e *env) goTool(tool string, args ...string) []string {
// and additional arguments.
func (e *env) goCmd(cmd string, args ...string) []string {
exe := filepath.Join(e.sdk, "bin", "go")

if runtime.GOOS == "windows" {
exe += ".exe"
}
Expand Down
64 changes: 52 additions & 12 deletions go/tools/builders/stdliblist.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"bytes"
"encoding/json"
"flag"
"fmt"
"go/build"
"os"
"path/filepath"
Expand Down Expand Up @@ -109,29 +110,32 @@ func stdlibPackageID(importPath string) string {
return "@io_bazel_rules_go//stdlib:" + importPath
}

func execRootPath(execRoot, p string) string {
dir, _ := filepath.Rel(execRoot, p)
// labelledPath replace the cloneBase with output base label
func labelledPath(cloneBase, p string) string {
xytan0056 marked this conversation as resolved.
Show resolved Hide resolved
dir, _ := filepath.Rel(cloneBase, p)
return filepath.Join("__BAZEL_OUTPUT_BASE__", dir)
}

func absoluteSourcesPaths(execRoot, pkgDir string, srcs []string) []string {
// absoluteSourcesPaths replace cloneBase of the absolution
// paths with the label for all source files in a package
func absoluteSourcesPaths(cloneBase, pkgDir string, srcs []string) []string {
ret := make([]string, 0, len(srcs))
pkgDir = execRootPath(execRoot, pkgDir)
pkgDir = labelledPath(cloneBase, pkgDir)
for _, src := range srcs {
ret = append(ret, filepath.Join(pkgDir, src))
}
return ret
}

func flatPackageForStd(execRoot string, pkg *goListPackage) *flatPackage {
func flatPackageForStd(cloneBase string, pkg *goListPackage) *flatPackage {
// Don't use generated files from the stdlib
goFiles := absoluteSourcesPaths(execRoot, pkg.Dir, pkg.GoFiles)
goFiles := absoluteSourcesPaths(cloneBase, pkg.Dir, pkg.GoFiles)

newPkg := &flatPackage{
ID: stdlibPackageID(pkg.ImportPath),
Name: pkg.Name,
PkgPath: pkg.ImportPath,
ExportFile: execRootPath(execRoot, pkg.Target),
ExportFile: labelledPath(cloneBase, pkg.Target),
Imports: map[string]string{},
Standard: pkg.Standard,
GoFiles: goFiles,
Expand All @@ -145,6 +149,34 @@ 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, cloneGoRoot creates a copy of a subset of external/go_sdk
// that is sufficient to call "go list" into a new cloneBase directory, e.g.
// "go list" needs to call "compile", which needs "pkg/tool".
// We also need to retain the same relative path to the root directory, e.g.
// "$OUTPUT_BASE/external/go_sdk" becomes
// {cloneBase}/external/go_sdk", which will be set at GOROOT later. This ensures
// that file paths in the generated JSON are still valid.
//
// cloneGoRoot replicate goRoot to newGoRoot and returns an error if any.
func cloneGoRoot(goRoot, newGoRoot string) error {
if err := os.MkdirAll(newGoRoot, 01755); err != nil {
xytan0056 marked this conversation as resolved.
Show resolved Hide resolved
return err
}

if err := replicate(goRoot, newGoRoot, replicatePaths("src", "pkg/tool", "pkg/include")); err != nil {
return err
}

return nil
}
xytan0056 marked this conversation as resolved.
Show resolved Hide resolved

// stdliblist runs `go list -json` on the standard library and saves it to a file.
func stdliblist(args []string) error {
// process the args
Expand All @@ -158,19 +190,28 @@ func stdliblist(args []string) error {
return err
}

cloneBase, cleanup, err := goenv.workDir()
if err != nil {
return err
}
defer func() { cleanup() }()

newGoRoot := filepath.Join(cloneBase, "external/go_sdk")
xytan0056 marked this conversation as resolved.
Show resolved Hide resolved
err = cloneGoRoot(abs(goenv.sdk), abs(newGoRoot))
if err != nil {
return fmt.Errorf("failed to clone new go root %v", 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", newGoRoot)
// 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)
Expand All @@ -193,15 +234,14 @@ func stdliblist(args []string) error {
if err := goenv.runCommandToFile(jsonData, listArgs); err != nil {
return err
}

encoder := json.NewEncoder(jsonFile)
decoder := json.NewDecoder(jsonData)
for decoder.More() {
var pkg *goListPackage
if err := decoder.Decode(&pkg); err != nil {
return err
}
if err := encoder.Encode(flatPackageForStd(execRoot, pkg)); err != nil {
if err := encoder.Encode(flatPackageForStd(cloneBase, pkg)); err != nil {
return err
}
}
Expand Down
55 changes: 55 additions & 0 deletions go/tools/builders/stdliblist_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package main

import (
"bufio"
"encoding/json"
"fmt"
"github.com/bazelbuild/rules_go/go/tools/bazel"
"os"
"path/filepath"
"strings"
"testing"
)

func Test_stdliblist(t *testing.T) {
testDir := t.TempDir()
outJSON := filepath.Join(testDir, "out.json")

runFilePath, err := bazel.RunfilesPath()
if err != nil {
t.Errorf("cannot file runfile path %v", err)
}
test_args := []string{
fmt.Sprintf("-out=%s", outJSON),
fmt.Sprintf("-sdk=%s", abs(filepath.Join(filepath.Dir(runFilePath), "go_sdk"))),
}

err = stdliblist(test_args)
if err != nil {
t.Errorf("calling stdliblist got err: %v", err)
}
f, err := os.Open(outJSON)
if err != nil {
t.Errorf("cannot open output json: %v", err)
}
defer func() { _ = f.Close() }()
scanner := bufio.NewScanner(f)
for scanner.Scan() {
xytan0056 marked this conversation as resolved.
Show resolved Hide resolved
var result flatPackage
jsonLineStr := scanner.Text()
if err := json.Unmarshal([]byte(jsonLineStr), &result); err != nil {
t.Errorf("cannot parse result line %s \n to goListPackage{}: %v\n", err)
}
if !strings.HasPrefix(result.ID, "@io_bazel_rules_go//stdlib") {
t.Errorf("ID should be prefixed with @io_bazel_rules_go//stdlib :%s", jsonLineStr)
}
if !strings.HasPrefix(result.ExportFile, "__BAZEL_OUTPUT_BASE__") {
t.Errorf("export file should be prefixed with __BAZEL_OUTPUT_BASE__ :%s", jsonLineStr)
}
for _, gofile := range result.GoFiles {
if !strings.HasPrefix(gofile, "__BAZEL_OUTPUT_BASE__/external/go_sdk") {
t.Errorf("All go files should be prefixed with __BAZEL_OUTPUT_BASE__/go_sdk :%s", jsonLineStr)
xytan0056 marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
}
5 changes: 3 additions & 2 deletions tests/core/stdlib/stdlib_files.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,11 @@ pure_transition = transition(
def _stdlib_files_impl(ctx):
# When a transition is used, ctx.attr._stdlib is a list of Target instead
# of a Target. Possibly a bug?
libs = ctx.attr._stdlib[0][GoStdLib].libs
stdlib = ctx.attr._stdlib[0][GoStdLib]
libs = stdlib.libs
runfiles = ctx.runfiles(files = libs)
return [DefaultInfo(
files = depset(libs),
files = depset(libs + [stdlib._list_json]),
runfiles = runfiles,
)]

Expand Down