Support --nolegacy_external_runfiles (#3795)

* Test with `--nolegacy_external_runfiles`

* Fix tests with `--nolegacy_external_runfiles`

* Update runfiles.go (#3796)

* Update runfiles.go

* Update runfiles.go

* Update check_runfiles.go

* Update check_runfiles.go

.bazelrc

@@ -32,7 +32,8 @@ 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
 # Also enable all incompatible flags in go_bazel_test by default.
 # TODO: Add --incompatible_disallow_empty_glob once
 # https://github.com/bazelbuild/bazel-gazelle/pull/1405 has been released.
-test:incompatible --test_env=GO_BAZEL_TEST_BAZELFLAGS='--incompatible_load_proto_rules_from_bzl --incompatible_enable_cc_toolchain_resolution --incompatible_config_setting_private_default_visibility --incompatible_enforce_config_setting_visibility --incompatible_disable_starlark_host_transitions'
+test:incompatible --test_env=GO_BAZEL_TEST_BAZELFLAGS='--incompatible_load_proto_rules_from_bzl --incompatible_enable_cc_toolchain_resolution --incompatible_config_setting_private_default_visibility --incompatible_enforce_config_setting_visibility --incompatible_disable_starlark_host_transitions --nolegacy_external_runfiles'

go/tools/bazel/runfiles.go

@@ -395,6 +395,13 @@ func initRunfiles() {
 					entry.ShortPath = entry.ShortPath[i+1:]
 				}
 			}
+			if strings.HasPrefix(entry.ShortPath, "../") {
+				entry.ShortPath = entry.ShortPath[len("../"):]
+				if i := strings.IndexByte(entry.ShortPath, '/'); i >= 0 {
+					entry.Workspace = entry.ShortPath[:i]
+					entry.ShortPath = entry.ShortPath[i+1:]
+				}
+			}
 
 			runfiles.list = append(runfiles.list, entry)
 			runfiles.index.Put(&entry)

go/tools/bazel_testing/bazel_testing.go

@@ -482,16 +482,22 @@ func extractTxtar(dir, txt string) error {
 
 func parseLocationArg(arg string) (workspace, shortPath string, err error) {
 	cleanPath := path.Clean(arg)
-	if !strings.HasPrefix(cleanPath, "external/") {
+	// Support both states of --legacy_external_runfiles.
+	if !strings.HasPrefix(cleanPath, "../") && !strings.HasPrefix(cleanPath, "external/") {
 		return "", cleanPath, nil
 	}
-	i := strings.IndexByte(arg[len("external/"):], '/')
+	var trimmedPath string
+	if strings.HasPrefix(cleanPath, "../") {
+		trimmedPath = cleanPath[len("../"):]
+	} else {
+		trimmedPath = cleanPath[len("external/"):]
+	}
+	i := strings.IndexByte(trimmedPath, '/')
 	if i < 0 {
-		return "", "", fmt.Errorf("unexpected file (missing / after external/): %s", arg)
+		return "", "", fmt.Errorf("unexpected file (missing / after ../): %s", arg)
 	}
-	i += len("external/")
-	workspace = cleanPath[len("external/"):i]
-	shortPath = cleanPath[i+1:]
+	workspace = trimmedPath[:i]
+	shortPath = trimmedPath[i+1:]
 	return workspace, shortPath, nil
 }
 

go/tools/builders/stdliblist_test.go

@@ -15,7 +15,7 @@ func Test_stdliblist(t *testing.T) {
 
 	test_args := []string{
 		fmt.Sprintf("-out=%s", outJSON),
-		"-sdk=external/go_sdk",
+		"-sdk=../go_sdk",
 	}
 
 	if err := stdliblist(test_args); err != nil {
@@ -40,8 +40,9 @@ func Test_stdliblist(t *testing.T) {
 			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__/external/go_sdk :%v", result)
+			// The SDK runfiles are prefixed with __BAZEL_OUTPUT_BASE__/../go_sdk, which is cleaned.
+			if !strings.HasPrefix(gofile, "go_sdk/") {
+				t.Errorf("all go files should be prefixed with go_sdk/ :%v", result)
 			}
 		}
 	}

tests/core/go_test/BUILD.bazel

@@ -237,10 +237,10 @@ go_test(
     srcs = ["env_test.go"],
     data = ["@go_sdk//:lib/time/zoneinfo.zip"],
     env = {
-        "ZONEINFO": "$(execpath @go_sdk//:lib/time/zoneinfo.zip)",
+        "ZONEINFO": "$(rlocationpath @go_sdk//:lib/time/zoneinfo.zip)",
     },
     deps = [
-        "@io_bazel_rules_go//go/tools/bazel",
+        "//go/runfiles",
     ],
 )
 

tests/core/go_test/env_test.go

@@ -15,25 +15,23 @@
 package env_test
 
 import (
+    "github.com/bazelbuild/rules_go/go/runfiles"
     "os"
     "testing"
-
-    "github.com/bazelbuild/rules_go/go/tools/bazel"
 )
 
 func TestEnv(t *testing.T) {
-    v := os.Getenv("ZONEINFO")
-    if v == "" {
-        t.Fatalf("ZONEINFO env var was empty")
-    }
+	v := os.Getenv("ZONEINFO")
+	if v == "" {
+		t.Fatalf("ZONEINFO env var was empty")
+	}
 
-    path, err := bazel.Runfile(v)
-    if err != nil {
-        t.Fatalf("Could not find runfile %v: %v", v, err)
-    }
+	path, err := runfiles.Rlocation(v)
+	if err != nil {
+		t.Fatalf("Could not find runfile %v: %v", v, err)
+	}
 
-    if _, err := os.Stat(path); err != nil {
-        t.Fatalf("Could not find file at env var $ZONEINFO (value: %v) at path %v: %v", v, path, err)
-    }
+	if _, err := os.Stat(path); err != nil {
+		t.Fatalf("Could not find file at env var $ZONEINFO (value: %v) at path %v: %v", v, path, err)
+	}
 }
-

tests/core/runfiles/BUILD.bazel

@@ -6,7 +6,7 @@ test_suite(
     name = "runfiles_tests",
     tests = [
         ":local_test",
-        "@runfiles_remote_test//:remote_test",
+        "@runfiles_remote_test//remote_pkg:remote_test",
     ],
 )
 
@@ -34,9 +34,9 @@ go_library(
         "local_file.txt",
         ":local_bin",
         ":local_group",
-        "@runfiles_remote_test//:remote_bin",
-        "@runfiles_remote_test//:remote_file.txt",
-        "@runfiles_remote_test//:remote_group",
+        "@runfiles_remote_test//remote_pkg:remote_bin",
+        "@runfiles_remote_test//remote_pkg:remote_file.txt",
+        "@runfiles_remote_test//remote_pkg:remote_group",
     ],
     importpath = "github.com/bazelbuild/rules_go/tests/core/runfiles/check",
     deps = ["//go/tools/bazel:go_default_library"],

tests/core/runfiles/check_runfiles.go

@@ -34,9 +34,9 @@ var DefaultTestFiles = []TestFile{
 	{Workspace: "io_bazel_rules_go", Path: "tests/core/runfiles/local_file.txt"},
 	{Workspace: "io_bazel_rules_go", Path: "tests/core/runfiles/local_group.txt"},
 	{Workspace: "io_bazel_rules_go", Path: "tests/core/runfiles/local_bin", Binary: true},
-	{Workspace: "runfiles_remote_test", Path: "remote_file.txt"},
-	{Workspace: "runfiles_remote_test", Path: "remote_group.txt"},
-	{Workspace: "runfiles_remote_test", Path: "remote_bin", Binary: true},
+	{Workspace: "runfiles_remote_test", Path: "remote_pkg/remote_file.txt"},
+	{Workspace: "runfiles_remote_test", Path: "remote_pkg/remote_group.txt"},
+	{Workspace: "runfiles_remote_test", Path: "remote_pkg/remote_bin", Binary: true},
 }
 
 func CheckRunfiles(files []TestFile) error {