diff --git a/go/private/rules/test.bzl b/go/private/rules/test.bzl index 78f2791817..84687fc54f 100644 --- a/go/private/rules/test.bzl +++ b/go/private/rules/test.bzl @@ -132,7 +132,7 @@ def _go_test_impl(ctx): # We add "+initfirst/" to the package path so the package is initialized # before user code. See comment above the init function # in bzltestutil/init.go. - test_gc_linkopts.extend(["-X", "+initfirst/github.com/bazelbuild/rules_go/go/tools/bzltestutil.RunDir=" + run_dir]) + test_gc_linkopts.extend(["-X", "+initfirst/github.com/bazelbuild/rules_go/go/tools/bzltestutil/chdir.RunDir=" + run_dir]) # Now compile the test binary itself test_library = GoLibrary( diff --git a/go/tools/bzltestutil/BUILD.bazel b/go/tools/bzltestutil/BUILD.bazel index a6cbe690f8..a97a7516b9 100644 --- a/go/tools/bzltestutil/BUILD.bazel +++ b/go/tools/bzltestutil/BUILD.bazel @@ -3,17 +3,14 @@ load("//go:def.bzl", "go_test", "go_tool_library") go_tool_library( name = "bzltestutil", srcs = [ - "init.go", "lcov.go", "test2json.go", "wrap.go", "xml.go", ], - # We add "+initfirst/" to the package path so this package is initialized - # before user code. See comment above the init function in init.go. - importmap = "+initfirst/github.com/bazelbuild/rules_go/go/tools/bzltestutil", importpath = "github.com/bazelbuild/rules_go/go/tools/bzltestutil", visibility = ["//visibility:public"], + deps = ["//go/tools/bzltestutil/chdir"], ) go_test( @@ -34,7 +31,7 @@ go_test( filegroup( name = "all_files", testonly = True, - srcs = glob( + srcs = ["//go/tools/bzltestutil/chdir:all_files"] + glob( ["**"], exclude = ["testdata/*"], ), diff --git a/go/tools/bzltestutil/chdir/BUILD.bazel b/go/tools/bzltestutil/chdir/BUILD.bazel new file mode 100644 index 0000000000..7f88d9df07 --- /dev/null +++ b/go/tools/bzltestutil/chdir/BUILD.bazel @@ -0,0 +1,21 @@ +load("//go:def.bzl", "go_tool_library") + +go_tool_library( + name = "chdir", + srcs = ["init.go"], + # We add "+initfirst/" to the package path so this package is initialized + # before user code. See comment above the init function in init.go. + importmap = "+initfirst/github.com/bazelbuild/rules_go/go/tools/bzltestutil/chdir", + importpath = "github.com/bazelbuild/rules_go/go/tools/bzltestutil/chdir", + visibility = ["//go/tools/bzltestutil:__pkg__"], +) + +filegroup( + name = "all_files", + testonly = True, + srcs = glob( + ["**"], + exclude = ["testdata/*"], + ), + visibility = ["//visibility:public"], +) diff --git a/go/tools/bzltestutil/chdir/init.go b/go/tools/bzltestutil/chdir/init.go new file mode 100644 index 0000000000..3c374dec34 --- /dev/null +++ b/go/tools/bzltestutil/chdir/init.go @@ -0,0 +1,143 @@ +// Copyright 2020 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package chdir provides an init function that changes the current working +// directory to RunDir when the test executable is started by Bazel +// (when TEST_SRCDIR and TEST_WORKSPACE are set). +// +// This hides a difference between Bazel and 'go test': 'go test' starts test +// executables in the package source directory, while Bazel starts test +// executables in a directory made to look like the repository root directory. +// Tests frequently refer to testdata files using paths relative to their +// package directory, so open source tests frequently break unless they're +// written with Bazel specifically in mind (using go/runfiles). +// +// For this init function to work, it must be called before init functions +// in all user packages. +// +// In Go 1.20 and earlier, the package initialization order was underspecified, +// other than a requirement that each package is initialized after all its +// transitively imported packages. We relied on the linker initializing +// packages in the order their imports appeared in source, so we import +// bzltestutil (and transitively, this package) from the generated test main +// before other packages. +// +// In Go 1.21, the package initialization order was clarified, and the +// linker implementation was changed. See +// https://go.dev/ref/spec#Program_initialization or +// https://go.dev/doc/go1.21#language. +// +// > Given the list of all packages, sorted by import path, in each step the +// > first uninitialized package in the list for which all imported packages +// > (if any) are already initialized is initialized. This step is repeated +// > until all packages are initialized. +// +// To ensure this package is initialized before user code without injecting +// edges into the dependency graph, we implement the following hack: +// +// 1. Add the prefix '+initfirst/' to this package's path with the 'importmap' +// attribute. '+' is the first allowed character that sorts higher than +// letters. Because we're using 'importmap' and not 'importpath', this +// package may be imported in .go files without the prefix. +// 2. Put this init function in a separate package that only imports "os". +// Previously, this function was in bzltestutil, but bzltest util imports +// several other std packages may be get initialized later. For example, +// the "sync" package is initialized after a user package named +// "github.com/a/b" that only imports "os", and because bzltestutil imports +// "sync", it would get initialized even later. A user package that imports +// nothing may still be initialized before "os", but we assume "os" +// is needed to observe the current directory. +package chdir + +// This package should import nothing other than "os" +// and packages imported by "os" (run 'go list -deps os'). +import "os" + +var ( + // Initialized by linker. + RunDir string + + // Initial working directory. + TestExecDir string +) + +const isWindows = os.PathSeparator == '\\' + +func init() { + var err error + TestExecDir, err = os.Getwd() + if err != nil { + panic(err) + } + + // Check if we're being run by Bazel and change directories if so. + // TEST_SRCDIR and TEST_WORKSPACE are set by the Bazel test runner, so that makes a decent proxy. + testSrcDir, hasSrcDir := os.LookupEnv("TEST_SRCDIR") + testWorkspace, hasWorkspace := os.LookupEnv("TEST_WORKSPACE") + if hasSrcDir && hasWorkspace && RunDir != "" { + abs := RunDir + if !filepathIsAbs(RunDir) { + abs = filepathJoin(testSrcDir, testWorkspace, RunDir) + } + err := os.Chdir(abs) + // Ignore the Chdir err when on Windows, since it might have have runfiles symlinks. + // https://github.com/bazelbuild/rules_go/pull/1721#issuecomment-422145904 + if err != nil && !isWindows { + panic("could not change to test directory: " + err.Error()) + } + if err == nil { + os.Setenv("PWD", abs) + } + } +} + +// filepathIsAbs is a primitive version of filepath.IsAbs. It handles the +// cases we are likely to encounter but is not specialized at compile time +// and does not support DOS device paths (\\.\UNC\host\share\...) nor +// Plan 9 absolute paths (starting with #). +func filepathIsAbs(path string) bool { + if isWindows { + // Drive-letter path + if len(path) >= 3 && + ('A' <= path[0] && path[0] <= 'Z' || 'a' <= path[0] && path[0] <= 'z') && + path[1] == ':' && + (path[2] == '\\' || path[2] == '/') { + return true + } + + // UNC path + if len(path) >= 2 && path[0] == '\\' && path[1] == '\\' { + return true + } + return false + } + + return len(path) > 0 && path[0] == '/' +} + +// filepathJoin is a primitive version of filepath.Join. It only joins +// its arguments with os.PathSeparator. It does not clean arguments first. +func filepathJoin(base string, parts ...string) string { + n := len(base) + for _, part := range parts { + n += 1 + len(part) + } + buf := make([]byte, 0, n) + buf = append(buf, []byte(base)...) + for _, part := range parts { + buf = append(buf, os.PathSeparator) + buf = append(buf, []byte(part)...) + } + return string(buf) +} diff --git a/go/tools/bzltestutil/init.go b/go/tools/bzltestutil/init.go deleted file mode 100644 index 148299a94b..0000000000 --- a/go/tools/bzltestutil/init.go +++ /dev/null @@ -1,88 +0,0 @@ -// Copyright 2020 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package bzltestutil - -// This package must have no deps beyond Go SDK. -import ( - "fmt" - "os" - "path/filepath" - "runtime" -) - -var ( - // Initialized by linker. - RunDir string - - // Initial working directory. - testExecDir string -) - -// This function sets the current working directory to RunDir when the test -// executable is started by Bazel (when TEST_SRCDIR and TEST_WORKSPACE are set). -// -// This hides a difference between Bazel and 'go test': 'go test' starts test -// executables in the package source directory, while Bazel starts test -// executables in a directory made to look like the repository root directory. -// Tests frequently refer to testdata files using paths relative to their -// package directory, so open source tests frequently break unless they're -// written with Bazel specifically in mind (using go/runfiles). -// -// For this init function to work, it must be called before init functions -// in all user packages. -// -// In Go 1.20 and earlier, the package initialization order was underspecified, -// other than a requirement that each package is initialized after all its -// transitively imported packages. We relied on the linker initializing -// packages in the order their imports appeared in source, so we imported -// bzltestutil from the generated test main before other packages. -// -// In Go 1.21, the package initialization order was clarified, and the -// linker implementation was changed. See https://go.dev/doc/go1.21#language. -// The order is now affected by import path: packages with lexicographically -// lower import paths go first. -// -// To ensure this package is initialized before user code, we add the prefix -// '+initfirst/' to this package's path with the 'importmap' directive. -// '+' is the first allowed character that sorts higher than letters. -// Because we're using 'importmap' and not 'importpath', this hack does not -// affect .go source files. -func init() { - var err error - testExecDir, err = os.Getwd() - if err != nil { - panic(err) - } - - // Check if we're being run by Bazel and change directories if so. - // TEST_SRCDIR and TEST_WORKSPACE are set by the Bazel test runner, so that makes a decent proxy. - testSrcDir, hasSrcDir := os.LookupEnv("TEST_SRCDIR") - testWorkspace, hasWorkspace := os.LookupEnv("TEST_WORKSPACE") - if hasSrcDir && hasWorkspace && RunDir != "" { - abs := RunDir - if !filepath.IsAbs(RunDir) { - abs = filepath.Join(testSrcDir, testWorkspace, RunDir) - } - err := os.Chdir(abs) - // Ignore the Chdir err when on Windows, since it might have have runfiles symlinks. - // https://github.com/bazelbuild/rules_go/pull/1721#issuecomment-422145904 - if err != nil && runtime.GOOS != "windows" { - panic(fmt.Sprintf("could not change to test directory: %v", err)) - } - if err == nil { - os.Setenv("PWD", abs) - } - } -} diff --git a/go/tools/bzltestutil/wrap.go b/go/tools/bzltestutil/wrap.go index c8fb65e0d9..4f325dbe55 100644 --- a/go/tools/bzltestutil/wrap.go +++ b/go/tools/bzltestutil/wrap.go @@ -27,6 +27,8 @@ import ( "strconv" "strings" "sync" + + "github.com/bazelbuild/rules_go/go/tools/bzltestutil/chdir" ) // TestWrapperAbnormalExit is used by Wrap to indicate the child @@ -117,8 +119,8 @@ func Wrap(pkg string) error { args = append([]string{"-test.v"}, args...) } exePath := os.Args[0] - if !filepath.IsAbs(exePath) && strings.ContainsRune(exePath, filepath.Separator) && testExecDir != "" { - exePath = filepath.Join(testExecDir, exePath) + if !filepath.IsAbs(exePath) && strings.ContainsRune(exePath, filepath.Separator) && chdir.TestExecDir != "" { + exePath = filepath.Join(chdir.TestExecDir, exePath) } cmd := exec.Command(exePath, args...) cmd.Env = append(os.Environ(), "GO_TEST_WRAP=0")