Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support --nolegacy_external_runfiles #3795

Merged
merged 5 commits into from
Dec 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -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'
7 changes: 7 additions & 0 deletions go/tools/bazel/runfiles.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
18 changes: 12 additions & 6 deletions go/tools/bazel_testing/bazel_testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
7 changes: 4 additions & 3 deletions go/tools/builders/stdliblist_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions tests/core/go_test/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
)

Expand Down
26 changes: 12 additions & 14 deletions tests/core/go_test/env_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

8 changes: 4 additions & 4 deletions tests/core/runfiles/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ test_suite(
name = "runfiles_tests",
tests = [
":local_test",
"@runfiles_remote_test//:remote_test",
"@runfiles_remote_test//remote_pkg:remote_test",
],
)

Expand Down Expand Up @@ -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"],
Expand Down
6 changes: 3 additions & 3 deletions tests/core/runfiles/check_runfiles.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading