Get gazelle_test and generationtest to pass with Bzlmod (#2009)

Also clean up `.bazelrc` and presubmit configs.

.bazelci/presubmit.yml

@@ -12,9 +12,9 @@ tasks:
     name: Ubuntu 22.04 with WORKSPACE
     platform: ubuntu2204
     build_flags:
-    - "--noexperimental_enable_bzlmod"
+    - "--noenable_bzlmod"
     test_flags:
-    - "--noexperimental_enable_bzlmod"
+    - "--noenable_bzlmod"
     build_targets:
     - "..."
     run_targets:
@@ -91,14 +91,13 @@ tasks:
     test_targets:
     - "..."
     - "-//internal:bazel_test"
-    - "-//cmd/gazelle:gazelle_test"
   macos_arm64:
     name: Mac OS Arm 64 with WORKSPACE
     platform: macos_arm64
     build_flags:
-    - "--noexperimental_enable_bzlmod"
+    - "--noenable_bzlmod"
     test_flags:
-    - "--noexperimental_enable_bzlmod"
+    - "--noenable_bzlmod"
     build_targets:
     - "..."
     test_targets:
@@ -114,7 +113,6 @@ tasks:
     test_targets:
     - "..."
     - "-//internal:bazel_test"
-    - "-//cmd/gazelle:gazelle_test"
   macos:
     name: Mac OS with WORKSPACE
     platform: macos
@@ -136,7 +134,6 @@ tasks:
     test_targets:
     - "--"
     - "..."
-    - "-//cmd/gazelle:gazelle_test"
     # autogazelle is only supported on UNIX-like platforms.
     # It requires UNIX domain sockets.
     - "-//cmd/autogazelle/..."
@@ -179,14 +176,14 @@ tasks:
     name: Ubuntu 22.04 with WORKSPACE and --config=incompatible
     platform: ubuntu2204
     build_flags:
-      - "--noexperimental_enable_bzlmod"
+      - "--noenable_bzlmod"
       - "--config=incompatible"
     build_targets:
       - "..."
     run_targets:
       - "//:gazelle_ci"
     test_flags:
-      - "--noexperimental_enable_bzlmod"
+      - "--noenable_bzlmod"
       - "--config=incompatible"
     test_targets:
       - "..."

.bazelrc

@@ -4,19 +4,19 @@
 # TODO: Make all tests work with Bzlmod.
 common --noenable_bzlmod
 
-build:ci --verbose_failures
-build:ci --sandbox_debug
-build:ci --spawn_strategy=standalone
-build:ci --genrule_strategy=standalone
-test:ci --test_strategy=standalone
+common:ci --verbose_failures
+common:ci --sandbox_debug
+common:ci --spawn_strategy=standalone
+common:ci --genrule_strategy=standalone
+common:ci --test_strategy=standalone
 
 common --lockfile_mode=update
-test --test_output=errors
+common --test_output=errors
 
-build:incompatible --incompatible_load_proto_rules_from_bzl
-build:incompatible --incompatible_config_setting_private_default_visibility
-build:incompatible --incompatible_enforce_config_setting_visibility
-build:incompatible --incompatible_disallow_empty_glob
-build:incompatible --incompatible_disable_starlark_host_transitions
-build:incompatible --nolegacy_external_runfiles
-test:incompatible --test_env=GO_BAZEL_TEST_BAZELFLAGS='--incompatible_load_proto_rules_from_bzl --incompatible_config_setting_private_default_visibility --incompatible_enforce_config_setting_visibility --incompatible_disallow_empty_glob --incompatible_disable_starlark_host_transitions --nolegacy_external_runfiles'
+common:incompatible --incompatible_load_proto_rules_from_bzl
+common:incompatible --incompatible_config_setting_private_default_visibility
+common:incompatible --incompatible_enforce_config_setting_visibility
+common:incompatible --incompatible_disallow_empty_glob
+common:incompatible --incompatible_disable_starlark_host_transitions
+common:incompatible --nolegacy_external_runfiles
+common:incompatible --test_env=GO_BAZEL_TEST_BAZELFLAGS='--incompatible_load_proto_rules_from_bzl --incompatible_config_setting_private_default_visibility --incompatible_enforce_config_setting_visibility --incompatible_disallow_empty_glob --incompatible_disable_starlark_host_transitions --nolegacy_external_runfiles'

cmd/gazelle/BUILD.bazel

@@ -51,15 +51,18 @@ go_test(
         "langs.go",  # keep
         "profiler_test.go",
     ],
-    args = ["-go_sdk=go_sdk"],
-    data = ["@go_sdk//:files"],
+    data = [
+        "@go_sdk//:ROOT",
+        "@go_sdk//:files",
+    ],
     embed = [":gazelle_lib"],
+    x_defs = {"goRootFile": "$(rlocationpath @go_sdk//:ROOT)"},
     deps = [
         "//config",
         "//internal/wspace",
         "//testtools",
         "@com_github_google_go_cmp//cmp",
-        "@io_bazel_rules_go//go/tools/bazel:go_default_library",
+        "@io_bazel_rules_go//go/runfiles:go_default_library",
     ],
 )
 

cmd/gazelle/fix_test.go

@@ -20,15 +20,15 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"runtime"
 	"strings"
 	"testing"
 
 	"github.com/bazelbuild/bazel-gazelle/testtools"
-	"github.com/bazelbuild/rules_go/go/tools/bazel"
+	"github.com/bazelbuild/rules_go/go/runfiles"
 )
 
-var goSdk = flag.String("go_sdk", "", "name of the go_sdk repository when invoked by Bazel")
+// Set via x_defs.
+var goRootFile = ""
 
 func TestMain(m *testing.M) {
 	status := 1
@@ -61,32 +61,12 @@ func TestMain(m *testing.M) {
 		os.RemoveAll(tmpDir)
 	}()
 
-	if *goSdk != "" {
-		// This flag is only set when the test is run by Bazel. Figure out where
-		// the Go binary is and set GOROOT appropriately.
-		entries, err := bazel.ListRunfiles()
-		if err != nil {
-			fmt.Fprintln(os.Stderr, err)
-			return
-		}
-
-		var goToolPath string
-		ext := ""
-		if runtime.GOOS == "windows" {
-			ext = ".exe"
-		}
-		for _, entry := range entries {
-			if entry.Workspace == *goSdk && entry.ShortPath == "bin/go"+ext {
-				goToolPath = entry.Path
-				break
-			}
-		}
-		if goToolPath == "" {
-			fmt.Fprintln(os.Stderr, "could not locate go tool")
-			return
-		}
-		os.Setenv("GOROOT", filepath.Dir(filepath.Dir(goToolPath)))
+	goRootFilePath, err := runfiles.Rlocation(goRootFile)
+	if err != nil {
+		fmt.Fprintln(os.Stderr, "could not locate GOROOT file:", err)
+		return
 	}
+	os.Setenv("GOROOT", filepath.Dir(goRootFilePath))
 	os.Setenv("GOCACHE", filepath.Join(tmpDir, "gocache"))
 	os.Setenv("GOPATH", filepath.Join(tmpDir, "gopath"))
 

internal/generationtest/BUILD.bazel

@@ -27,6 +27,6 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//testtools",
-        "@io_bazel_rules_go//go/tools/bazel:go_default_library",
+        "@io_bazel_rules_go//go/runfiles",
     ],
 )

internal/generationtest/generation_test.go

@@ -17,16 +17,18 @@ package generationtest
 
 import (
 	"flag"
+	"io/fs"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
 	"github.com/bazelbuild/bazel-gazelle/testtools"
-	"github.com/bazelbuild/rules_go/go/tools/bazel"
+	"github.com/bazelbuild/rules_go/go/runfiles"
 )
 
 var (
-	gazelleBinaryPath = flag.String("gazelle_binary_path", "", "Path to the gazelle binary to test.")
+	gazelleBinaryPath = flag.String("gazelle_binary_path", "", "rlocationpath to the gazelle binary to test.")
 	buildInSuffix     = flag.String("build_in_suffix", ".in", "The suffix on the test input BUILD.bazel files. Defaults to .in. "+
 		" By default, will use files named BUILD.in as the BUILD files before running gazelle.")
 	buildOutSuffix = flag.String("build_out_suffix", ".out", "The suffix on the expected BUILD.bazel files after running gazelle. Defaults to .out. "+
@@ -38,47 +40,59 @@ var (
 // workspaces and confirms that the generated BUILD files match expectation.
 func TestFullGeneration(t *testing.T) {
 	tests := []*testtools.TestGazelleGenerationArgs{}
-	runfiles, err := bazel.ListRunfiles()
+	relativeGazelleBinary, err := runfiles.Rlocation(*gazelleBinaryPath)
 	if err != nil {
-		t.Fatalf("bazel.ListRunfiles() error: %v", err)
+		t.Fatalf("Failed to find gazelle binary %s. Error: %v", *gazelleBinaryPath, err)
 	}
-	// Convert workspace relative path for gazelle binary into an absolute path.
-	// E.g. path/to/gazelle_binary -> /absolute/path/to/workspace/path/to/gazelle/binary.
-	absoluteGazelleBinary, err := bazel.Runfile(*gazelleBinaryPath)
+	absoluteGazelleBinary, err := filepath.Abs(relativeGazelleBinary)
 	if err != nil {
-		t.Fatalf("Could not convert gazelle binary path %s to absolute path. Error: %v", *gazelleBinaryPath, err)
+		t.Fatalf("Could not convert gazelle binary path %s to absolute path. Error: %v", relativeGazelleBinary, err)
 	}
 	testNames := map[string]struct{}{}
-	for _, f := range runfiles {
-		// Look through runfiles for WORKSPACE or MODULE.bazel files. Each such file specifies a test case.
-		if filepath.Base(f.Path) == "WORKSPACE" || filepath.Base(f.Path) == "MODULE.bazel" {
-			// absolutePathToTestDirectory is the absolute
-			// path to the test case directory. For example, /home/<user>/wksp/path/to/test_data/my_test_case
-			absolutePathToTestDirectory := filepath.Dir(f.Path)
-			// relativePathToTestDirectory is the workspace relative path
-			// to this test case directory. For example, path/to/test_data/my_test_case
-			relativePathToTestDirectory := filepath.Dir(f.ShortPath)
+	runfilesFS, err := runfiles.New()
+	if err != nil {
+		t.Fatalf("Failed to create runfiles filesystem. Error: %v", err)
+	}
+	err = fs.WalkDir(runfilesFS, ".", func(path string, d fs.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+		if d.IsDir() {
+			return nil
+		}
+		if d.Name() == "WORKSPACE" || d.Name() == "MODULE.bazel" {
+			actualFilePath, err := runfiles.Rlocation(path)
+			if err != nil {
+				t.Fatalf("Failed to find runfile %s. Error: %v", path, err)
+			}
+			absolutePathToTestDirectory := filepath.Dir(actualFilePath)
 			// name is the name of the test directory. For example, my_test_case.
 			// The name of the directory doubles as the name of the test.
 			name := filepath.Base(absolutePathToTestDirectory)
 
 			// Don't add a test if it was already added. That could be the case if a directory has
-			// both a WORKSPACE and a MODULE.bazel file in it.
-			if _, exists := testNames[name]; exists {
-				continue
-			}
-			testNames[name] = struct{}{}
+			// both a WORKSPACE and a MODULE.bazel file in it, or due to multiple apparent names
+			// mapping to the same canonical name.
+			if _, exists := testNames[name]; !exists {
+				testNames[name] = struct{}{}
 
-			tests = append(tests, &testtools.TestGazelleGenerationArgs{
-				Name:                 name,
-				TestDataPathAbsolute: absolutePathToTestDirectory,
-				TestDataPathRelative: relativePathToTestDirectory,
-				GazelleBinaryPath:    absoluteGazelleBinary,
-				BuildInSuffix:        *buildInSuffix,
-				BuildOutSuffix:       *buildOutSuffix,
-				Timeout:              *timeout,
-			})
+				tests = append(tests, &testtools.TestGazelleGenerationArgs{
+					Name:                 name,
+					TestDataPathAbsolute: absolutePathToTestDirectory,
+					// Drop the repository name from the rlocationpath.
+					TestDataPathRelative: path[strings.IndexRune(path, '/')+1:],
+					GazelleBinaryPath:    absoluteGazelleBinary,
+					BuildInSuffix:        *buildInSuffix,
+					BuildOutSuffix:       *buildOutSuffix,
+					Timeout:              *timeout,
+				})
+			}
+			return fs.SkipDir
 		}
+		return nil
+	})
+	if err != nil {
+		t.Fatalf("Failed to walk runfiles. Error: %v", err)
 	}
 	if len(tests) == 0 {
 		t.Fatal("no tests found")

internal/generationtest/generationtest.bzl

@@ -58,7 +58,7 @@ def gazelle_generation_test(name, gazelle_binary, test_data, build_in_suffix = "
         name = name,
         embed = [Label(":generationtest_test")],
         args = [
-            "-gazelle_binary_path=$(rootpath %s)" % gazelle_binary,
+            "-gazelle_binary_path=$(rlocationpath %s)" % gazelle_binary,
             "-build_in_suffix=%s" % build_in_suffix,
             "-build_out_suffix=%s" % build_out_suffix,
             "-timeout=%ds" % gazelle_timeout_seconds,