Skip to content

Commit a8cb4b7

Browse files
authored
bzltestutil: set importmap to fix run_dir (#3679)
Go 1.21 clarified package initialization order and changed behavior: packages with lexicographically lower paths are now initialized earlier. We need bzltestutil to initialize before user packages so it can change to the correct directory. To accomplish this, we set an importmap prefix that starts with '+', the lowest allowed character. Fixes #3675
1 parent 85f2440 commit a8cb4b7

File tree

5 files changed

+47
-10
lines changed

5 files changed

+47
-10
lines changed

go/private/rules/test.bzl

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,11 @@ def _go_test_impl(ctx):
128128
# Disable symbol table and DWARF generation for test binaries.
129129
test_gc_linkopts.extend(["-s", "-w"])
130130

131-
# Link in the run_dir global for bzltestutil
132-
test_gc_linkopts.extend(["-X", "github.com/bazelbuild/rules_go/go/tools/bzltestutil.RunDir=" + run_dir])
131+
# Link in the run_dir global for bzltestutil.
132+
# We add "+initfirst/" to the package path so the package is initialized
133+
# before user code. See comment above the init function
134+
# in bzltestutil/init.go.
135+
test_gc_linkopts.extend(["-X", "+initfirst/github.com/bazelbuild/rules_go/go/tools/bzltestutil.RunDir=" + run_dir])
133136

134137
# Now compile the test binary itself
135138
test_library = GoLibrary(

go/tools/builders/generate_test_main.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,10 @@ func (c *Cases) Version(v string) bool {
8787
const testMainTpl = `
8888
package main
8989
90-
// This package must be initialized before packages being tested.
91-
// NOTE: this relies on the order of package initialization, which is the spec
92-
// is somewhat unclear about-- it only clearly guarantees that imported packages
93-
// are initialized before their importers, though in practice (and implied) it
94-
// also respects declaration order, which we're relying on here.
90+
// bzltestutil may change the current directory in its init function to emulate
91+
// 'go test' behavior. It must be initialized before user packages.
92+
// In Go 1.20 and earlier, this import declaration must appear before
93+
// imports of user packages. See comment in bzltestutil/init.go.
9594
import "github.com/bazelbuild/rules_go/go/tools/bzltestutil"
9695
9796
import (

go/tools/bzltestutil/BUILD.bazel

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ go_tool_library(
99
"wrap.go",
1010
"xml.go",
1111
],
12+
# We add "+initfirst/" to the package path so this package is initialized
13+
# before user code. See comment above the init function in init.go.
14+
importmap = "+initfirst/github.com/bazelbuild/rules_go/go/tools/bzltestutil",
1215
importpath = "github.com/bazelbuild/rules_go/go/tools/bzltestutil",
1316
visibility = ["//visibility:public"],
1417
)

go/tools/bzltestutil/init.go

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,35 @@ var (
3030
testExecDir string
3131
)
3232

33-
// Before Go 1.21, this initializer runs before any user's package.
33+
// This function sets the current working directory to RunDir when the test
34+
// executable is started by Bazel (when TEST_SRCDIR and TEST_WORKSPACE are set).
3435
//
35-
// Since Go 1.21, the order of this initializer calls is not guarenteed to be first.
36-
// See https://go.dev/doc/go1.21#language for more details.
36+
// This hides a difference between Bazel and 'go test': 'go test' starts test
37+
// executables in the package source directory, while Bazel starts test
38+
// executables in a directory made to look like the repository root directory.
39+
// Tests frequently refer to testdata files using paths relative to their
40+
// package directory, so open source tests frequently break unless they're
41+
// written with Bazel specifically in mind (using go/runfiles).
42+
//
43+
// For this init function to work, it must be called before init functions
44+
// in all user packages.
45+
//
46+
// In Go 1.20 and earlier, the package initialization order was underspecified,
47+
// other than a requirement that each package is initialized after all its
48+
// transitively imported packages. We relied on the linker initializing
49+
// packages in the order their imports appeared in source, so we imported
50+
// bzltestutil from the generated test main before other packages.
51+
//
52+
// In Go 1.21, the package initialization order was clarified, and the
53+
// linker implementation was changed. See https://go.dev/doc/go1.21#language.
54+
// The order is now affected by import path: packages with lexicographically
55+
// lower import paths go first.
56+
//
57+
// To ensure this package is initialized before user code, we add the prefix
58+
// '+initfirst/' to this package's path with the 'importmap' directive.
59+
// '+' is the first allowed character that sorts higher than letters.
60+
// Because we're using 'importmap' and not 'importpath', this hack does not
61+
// affect .go source files.
3762
func init() {
3863
var err error
3964
testExecDir, err = os.Getwd()

tests/legacy/test_chdir/data_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,13 @@ import (
88

99
const file = "data.txt"
1010

11+
func init() {
12+
_, err := os.Stat(file)
13+
if err != nil {
14+
log.Fatalf("in init(), could not stat %s: %v", file, err)
15+
}
16+
}
17+
1118
func TestMain(m *testing.M) {
1219
_, err := os.Stat(file)
1320
if err != nil {

0 commit comments

Comments
 (0)