address comments

go/tools/builders/env.go

@@ -66,7 +66,7 @@ type env struct {
 // configured with those flags.
 func envFlags(flags *flag.FlagSet) *env {
 	env := &env{}
-	flags.StringVar(&env.sdk, "sdk", "", "Relative path to the Go SDK.")
+	flags.StringVar(&env.sdk, "sdk", "", "Path to the Go SDK.")
 	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")
@@ -121,7 +121,6 @@ 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"
 	}

go/tools/builders/stdliblist.go

@@ -149,34 +149,6 @@ func flatPackageForStd(cloneBase 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 {
-		return err
-	}
-
-	if err := replicate(goRoot, newGoRoot, replicatePaths("src", "pkg/tool", "pkg/include")); err != nil {
-		return err
-	}
-
-	return nil
-}
-
 // stdliblist runs `go list -json` on the standard library and saves it to a file.
 func stdliblist(args []string) error {
 	// process the args
@@ -190,22 +162,37 @@ 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() }()
 
-	relGoSdk, err := filepath.Rel(".", goenv.sdk)
-	if err != nil {
+	newGoRoot := filepath.Join(cloneBase, goenv.sdk)
+	if err := replicate(abs(goenv.sdk), abs(newGoRoot), replicatePaths("src", "pkg/tool", "pkg/include")); err != nil {
 		return err
 	}
 
-	newGoRoot := filepath.Join(cloneBase, relGoSdk)
-	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")) {

go/tools/builders/stdliblist_test.go

@@ -1,7 +1,6 @@
 package main
 
 import (
-	"bufio"
 	"encoding/json"
 	"fmt"
 	"os"
@@ -16,7 +15,7 @@ func Test_stdliblist(t *testing.T) {
 
 	test_args := []string{
 		fmt.Sprintf("-out=%s", outJSON),
-		fmt.Sprintf("-sdk=%s", "external/go_sdk"),
+		"-sdk=external/go_sdk",
 	}
 
 	if err := stdliblist(test_args); err != nil {
@@ -27,22 +26,22 @@ func Test_stdliblist(t *testing.T) {
 		t.Errorf("cannot open output json: %v", err)
 	}
 	defer func() { _ = f.Close() }()
-	scanner := bufio.NewScanner(f)
-	for scanner.Scan() {
-		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)
+	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 :%s", jsonLineStr)
+			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__ :%s", jsonLineStr)
+			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__/go_sdk :%s", jsonLineStr)
+				t.Errorf("all go files should be prefixed with __BAZEL_OUTPUT_BASE__/external/go_sdk :%v", result)
 			}
 		}
 	}