Skip to content

Commit

Permalink
Fix gopackagesdriver for Go 1.18 by replicating stdlib and add test f…
Browse files Browse the repository at this point in the history
…or stdliblist.go (#3157)

* stdliblist: Fix for Go 1.18 by replicating stdlib

The stdliblist operation that the gopackagesdriver relies on fails on
Go 1.18rc1 with the following error:

```
external/go_sdk/src/crypto/elliptic/p256_asm.go:24:12: pattern p256_asm_table.bin: cannot embed irregular file p256_asm_table.bin
```

We see this failure because Bazel builds a symlink tree of the GOROOT run
`go list` with. However, since [CL 380475][1], the Go standard library uses
`go:embed` to embed a file in `crypto/elliptic`, but `go:embed` does not
support symlinks.

[1]: https://go.dev/cl/380475

Fix this by having stdliblist copy the relevant portions of the GOROOT to
run `go list` with. This matches [what the stdlib action does][2].

[2]: https://github.com/bazelbuild/rules_go/blob/e9a7054ff11a520e3b8aceb76a3ba44bb8da4c94/go/tools/builders/stdlib.go#L54-L57

Resolves #3080

* test/stdlib: Depend on _list_json

Add a dependency on `GoStdLib._list_json` to the stdlib test.
This ensures that we can successfully build the JSON data needed by the
gopackagesdriver.
  • Loading branch information
xytan0056 authored May 19, 2022
1 parent 0df637f commit db94584
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 14 deletions.
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"],
rundir = ".",
)

filegroup(
name = "builder_srcs",
srcs = [
Expand Down
56 changes: 44 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)
// outputBasePath replace the cloneBase with output base label
func outputBasePath(cloneBase, p string) string {
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 = outputBasePath(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: outputBasePath(cloneBase, pkg.Target),
Imports: map[string]string{},
Standard: pkg.Standard,
GoFiles: goFiles,
Expand All @@ -158,19 +162,48 @@ func stdliblist(args []string) error {
return err
}

if filepath.IsAbs(goenv.sdk) {
return fmt.Errorf("-sdk needs to be a relative path, but got %s", goenv.sdk)
}

// 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.
//
// Here we replicate goRoot(absolute path of goenv.sdk) to newGoRoot.
cloneBase, cleanup, err := goenv.workDir()
if err != nil {
return err
}
defer func() { cleanup() }()

newGoRoot := filepath.Join(cloneBase, goenv.sdk)
if err := replicate(abs(goenv.sdk), abs(newGoRoot), replicatePaths("src", "pkg/tool", "pkg/include")); 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", 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 +226,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
48 changes: 48 additions & 0 deletions go/tools/builders/stdliblist_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package main

import (
"encoding/json"
"fmt"
"os"
"path/filepath"
"strings"
"testing"
)

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

test_args := []string{
fmt.Sprintf("-out=%s", outJSON),
"-sdk=external/go_sdk",
}

if err := stdliblist(test_args); 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() }()
decoder := json.NewDecoder(f)
for decoder.More() {
var result *flatPackage
if err := decoder.Decode(&result); err != nil {
t.Errorf("unable to decode output json: %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 :%v", result)
}
if !strings.HasPrefix(result.ExportFile, "__BAZEL_OUTPUT_BASE__") {
t.Errorf("export file should be prefixed with __BAZEL_OUTPUT_BASE__ :%v", result)
}
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__/external/go_sdk :%v", result)
}
}
}
}
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

0 comments on commit db94584

Please sign in to comment.