From f8be6d31b08617c082acddd934148337b84cccce Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Sat, 23 Dec 2023 14:06:51 +0100 Subject: [PATCH 1/5] Test with `--nolegacy_external_runfiles` --- .bazelrc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.bazelrc b/.bazelrc index 54ddff1186..2cc995f10c 100644 --- a/.bazelrc +++ b/.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' From 7dc1bb82e84b73a61c457a65074b8bc62b946624 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Sat, 23 Dec 2023 14:38:56 +0100 Subject: [PATCH 2/5] Fix tests with `--nolegacy_external_runfiles` --- go/tools/bazel_testing/bazel_testing.go | 18 +++++++++++------ go/tools/builders/stdliblist_test.go | 7 ++++--- tests/core/go_test/BUILD.bazel | 4 ++-- tests/core/go_test/env_test.go | 26 ++++++++++++------------- tests/core/runfiles/check_runfiles.go | 2 +- 5 files changed, 31 insertions(+), 26 deletions(-) diff --git a/go/tools/bazel_testing/bazel_testing.go b/go/tools/bazel_testing/bazel_testing.go index 92789cecd6..e79629ff68 100644 --- a/go/tools/bazel_testing/bazel_testing.go +++ b/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 } diff --git a/go/tools/builders/stdliblist_test.go b/go/tools/builders/stdliblist_test.go index f92c227234..19a69a8105 100644 --- a/go/tools/builders/stdliblist_test.go +++ b/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) } } } diff --git a/tests/core/go_test/BUILD.bazel b/tests/core/go_test/BUILD.bazel index 8750e7e978..e44564cca9 100644 --- a/tests/core/go_test/BUILD.bazel +++ b/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", ], ) diff --git a/tests/core/go_test/env_test.go b/tests/core/go_test/env_test.go index 89dae3ca56..7bbef945bc 100644 --- a/tests/core/go_test/env_test.go +++ b/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) + } } - diff --git a/tests/core/runfiles/check_runfiles.go b/tests/core/runfiles/check_runfiles.go index 8858a573c4..e6a74132e3 100644 --- a/tests/core/runfiles/check_runfiles.go +++ b/tests/core/runfiles/check_runfiles.go @@ -36,7 +36,7 @@ var DefaultTestFiles = []TestFile{ {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: "../runfiles_remote_test/remote_bin", Binary: true}, } func CheckRunfiles(files []TestFile) error { From b90c1dc9c27ff4b0177be36d12da096f0163a925 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Sat, 23 Dec 2023 15:20:37 +0100 Subject: [PATCH 3/5] Update runfiles.go (#3796) * Update runfiles.go * Update runfiles.go --- go/tools/bazel/runfiles.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/go/tools/bazel/runfiles.go b/go/tools/bazel/runfiles.go index a2ac11d81c..963247063a 100644 --- a/go/tools/bazel/runfiles.go +++ b/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) From e6b68aea1c6ff7fa9e6a0d7677f03658bacb5468 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Sat, 23 Dec 2023 15:21:18 +0100 Subject: [PATCH 4/5] Update check_runfiles.go --- tests/core/runfiles/check_runfiles.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/runfiles/check_runfiles.go b/tests/core/runfiles/check_runfiles.go index e6a74132e3..8858a573c4 100644 --- a/tests/core/runfiles/check_runfiles.go +++ b/tests/core/runfiles/check_runfiles.go @@ -36,7 +36,7 @@ var DefaultTestFiles = []TestFile{ {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: "../runfiles_remote_test/remote_bin", Binary: true}, + {Workspace: "runfiles_remote_test", Path: "remote_bin", Binary: true}, } func CheckRunfiles(files []TestFile) error { From 98ff92540f23ab889804cd46bc8d9f877129c502 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Sat, 23 Dec 2023 15:21:18 +0100 Subject: [PATCH 5/5] Update check_runfiles.go --- tests/core/runfiles/BUILD.bazel | 8 ++++---- tests/core/runfiles/check_runfiles.go | 6 +++--- .../runfiles_remote_test/{ => remote_pkg}/BUILD.bazel | 0 .../runfiles_remote_test/{ => remote_pkg}/remote_file.txt | 0 .../{ => remote_pkg}/remote_group.txt | 0 5 files changed, 7 insertions(+), 7 deletions(-) rename tests/core/runfiles/runfiles_remote_test/{ => remote_pkg}/BUILD.bazel (100%) rename tests/core/runfiles/runfiles_remote_test/{ => remote_pkg}/remote_file.txt (100%) rename tests/core/runfiles/runfiles_remote_test/{ => remote_pkg}/remote_group.txt (100%) diff --git a/tests/core/runfiles/BUILD.bazel b/tests/core/runfiles/BUILD.bazel index c7db6d3d35..d248cc066c 100644 --- a/tests/core/runfiles/BUILD.bazel +++ b/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"], diff --git a/tests/core/runfiles/check_runfiles.go b/tests/core/runfiles/check_runfiles.go index 8858a573c4..bc0f48d6b3 100644 --- a/tests/core/runfiles/check_runfiles.go +++ b/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 { diff --git a/tests/core/runfiles/runfiles_remote_test/BUILD.bazel b/tests/core/runfiles/runfiles_remote_test/remote_pkg/BUILD.bazel similarity index 100% rename from tests/core/runfiles/runfiles_remote_test/BUILD.bazel rename to tests/core/runfiles/runfiles_remote_test/remote_pkg/BUILD.bazel diff --git a/tests/core/runfiles/runfiles_remote_test/remote_file.txt b/tests/core/runfiles/runfiles_remote_test/remote_pkg/remote_file.txt similarity index 100% rename from tests/core/runfiles/runfiles_remote_test/remote_file.txt rename to tests/core/runfiles/runfiles_remote_test/remote_pkg/remote_file.txt diff --git a/tests/core/runfiles/runfiles_remote_test/remote_group.txt b/tests/core/runfiles/runfiles_remote_test/remote_pkg/remote_group.txt similarity index 100% rename from tests/core/runfiles/runfiles_remote_test/remote_group.txt rename to tests/core/runfiles/runfiles_remote_test/remote_pkg/remote_group.txt