From 32b5803f974d05bd7f4b18dd2f1d9d15fb9e93f8 Mon Sep 17 00:00:00 2001 From: Xiaoyang Tan Date: Thu, 12 May 2022 22:12:16 -0700 Subject: [PATCH] clean up --- .bazelci/presubmit.yml | 1 + go/tools/builders/BUILD.bazel | 2 + go/tools/builders/stdliblist.go | 12 ++-- go/tools/builders/stdliblist_test.go | 92 +++++++++++++++------------- 4 files changed, 57 insertions(+), 50 deletions(-) diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index 2ab856edeb..2c62debc75 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -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" diff --git a/go/tools/builders/BUILD.bazel b/go/tools/builders/BUILD.bazel index 17f3fe8222..192c67993c 100644 --- a/go/tools/builders/BUILD.bazel +++ b/go/tools/builders/BUILD.bazel @@ -34,6 +34,8 @@ go_test( "stdliblist_test.go", ], data = ["@go_sdk//:files"], + rundir = ".", + deps = ["//go/tools/bazel"], ) filegroup( diff --git a/go/tools/builders/stdliblist.go b/go/tools/builders/stdliblist.go index e7cfa3dca8..76fc69e6e7 100644 --- a/go/tools/builders/stdliblist.go +++ b/go/tools/builders/stdliblist.go @@ -18,6 +18,7 @@ import ( "bytes" "encoding/json" "flag" + "fmt" "go/build" "os" "path/filepath" @@ -165,9 +166,6 @@ func flatPackageForStd(cloneBase string, pkg *goListPackage) *flatPackage { // under. func cloneGoRoot(execRoot, relativeGoroot, cloneBase string) (newGoRoot string, err error) { goroot := filepath.Join(execRoot, relativeGoroot) - if err != nil { - return "", err - } newGoRoot = filepath.Join(cloneBase, relativeGoroot) if err := os.MkdirAll(newGoRoot, 01755); err != nil { return "", err @@ -195,15 +193,13 @@ func stdliblist(args []string) error { cloneBase, cleanup, err := goenv.workDir() if err != nil { - return err + return fmt.Errorf("creating temp %v", err) } - defer func() { - cleanup() - }() + defer func() { cleanup() }() cloneGoRoot, err := cloneGoRoot(goenv.wd, goenv.sdk, cloneBase) if err != nil { - return err + return fmt.Errorf("failed to clone new go root %v", err) } // Ensure paths are absolute. diff --git a/go/tools/builders/stdliblist_test.go b/go/tools/builders/stdliblist_test.go index 0a2955bd04..189a0762b3 100644 --- a/go/tools/builders/stdliblist_test.go +++ b/go/tools/builders/stdliblist_test.go @@ -1,50 +1,58 @@ package main import ( - "bufio" - "encoding/json" - "fmt" - "io/ioutil" - "os" - "strings" - "testing" + "bufio" + "encoding/json" + "fmt" + "io/ioutil" + "path/filepath" + "strings" + "testing" + + "github.com/bazelbuild/rules_go/go/tools/bazel" ) func Test_stdliblist(t *testing.T) { - testDir := t.TempDir() - f, _ := ioutil.TempFile(testDir, "*") + testDir := t.TempDir() + f, _ := ioutil.TempFile(testDir, "*") + + // test files are at run file directory, but this test is run at + // {runfile directory}/bazel.TestWorkspace() + // since -sdk is assumed to be a relative path to execRoot + // (go.sdk.root_file.dirname), thus setting wd to + // {runfile directory} so that go_sdk is discoverable + // {runfile directory} is the parent directory of bazel.RunfilesPath() + runFilesPath, err := bazel.RunfilesPath() + if err != nil { + t.Error("failed to find runfiles path") + } + test_args := []string{ + fmt.Sprintf("-out=%s", f.Name()), + fmt.Sprintf("-sdk=%s", "go_sdk"), + fmt.Sprintf("-wd=%s", filepath.Dir(filepath.Clean(runFilesPath))), + } - // test files are at TEST_SRCDIR, but this test is run at - // TEST_SRCDIR/TEST_WORKSPACE/... - // since -sdk is assumed to be a relative path to execRoot - // (go.sdk.root_file.dirname), thus setting wd to - // TEST_SRCDIR so that go_sdk is discoverable - test_args := []string{ - fmt.Sprintf("-out=%s", f.Name()), - fmt.Sprintf("-sdk=%s", "go_sdk"), - fmt.Sprintf("-wd=%s", os.Getenv("TEST_SRCDIR")), - } - err := stdliblist(test_args) - if err != nil { - t.Errorf("calling stdliblist got err: %v", err) - } - 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) - } - 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__/go_sdk") { - t.Errorf("All go files should be prefixed with __BAZEL_OUTPUT_BASE__/go_sdk :%s", jsonLineStr) - } - } - } + err = stdliblist(test_args) + if err != nil { + t.Errorf("calling stdliblist got err: %v", err) + } + 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) + } + 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__/go_sdk") { + t.Errorf("All go files should be prefixed with __BAZEL_OUTPUT_BASE__/go_sdk :%s", jsonLineStr) + } + } + } }