Skip to content

Commit c2406b2

Browse files
authored
Fix matching between Starlark and query labels in gopackagesdriver (#3701)
This relies on the query flag `--consistent_labels`, which is available in Bazel 6.4.0.
1 parent d1da1bb commit c2406b2

File tree

8 files changed

+43
-20
lines changed

8 files changed

+43
-20
lines changed

go/private/BUILD.sdk.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
load("@io_bazel_rules_go//go/private/rules:binary.bzl", "go_tool_binary")
22
load("@io_bazel_rules_go//go/private/rules:sdk.bzl", "package_list")
33
load("@io_bazel_rules_go//go/private/rules:transition.bzl", "non_go_reset_target")
4+
load("@io_bazel_rules_go//go/private:common.bzl", "RULES_GO_STDLIB_PREFIX")
45
load("@io_bazel_rules_go//go/private:go_toolchain.bzl", "declare_go_toolchains")
56
load("@io_bazel_rules_go//go:def.bzl", "go_sdk")
67

@@ -51,6 +52,7 @@ go_sdk(
5152
go_tool_binary(
5253
name = "builder",
5354
srcs = ["@io_bazel_rules_go//go/tools/builders:builder_srcs"],
55+
ldflags = "-X main.rulesGoStdlibPrefix={}".format(RULES_GO_STDLIB_PREFIX),
5456
sdk = ":go_sdk",
5557
)
5658

go/private/common.bzl

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,3 +252,10 @@ COVERAGE_OPTIONS_DENYLIST = {
252252
"-fprofile-instr-generate": None,
253253
"-fcoverage-mapping": None,
254254
}
255+
256+
_RULES_GO_RAW_REPO_NAME = str(Label("//:unused"))[:-len("//:unused")]
257+
258+
# When rules_go is the main repository and Bazel < 6 is used, the repo name does
259+
# not start with a "@", so we need to add it.
260+
RULES_GO_REPO_NAME = _RULES_GO_RAW_REPO_NAME if _RULES_GO_RAW_REPO_NAME.startswith("@") else "@" + _RULES_GO_RAW_REPO_NAME
261+
RULES_GO_STDLIB_PREFIX = RULES_GO_REPO_NAME + "//stdlib:"

go/private/rules/binary.bzl

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -441,12 +441,13 @@ def _go_tool_binary_impl(ctx):
441441
if sdk.goos == "windows":
442442
gopath = ctx.actions.declare_directory("gopath")
443443
gocache = ctx.actions.declare_directory("gocache")
444-
cmd = "@echo off\nset GOMAXPROCS=1\nset GOCACHE=%cd%\\{gocache}\nset GOPATH=%cd%\\{gopath}\n{go} build -o {out} -trimpath {srcs}".format(
444+
cmd = "@echo off\nset GOMAXPROCS=1\nset GOCACHE=%cd%\\{gocache}\nset GOPATH=%cd%\\{gopath}\n{go} build -o {out} -trimpath -ldflags \"{ldflags}\" {srcs}".format(
445445
gopath = gopath.path,
446446
gocache = gocache.path,
447447
go = sdk.go.path.replace("/", "\\"),
448448
out = out.path,
449449
srcs = " ".join([f.path for f in ctx.files.srcs]),
450+
ldflags = ctx.attr.ldflags,
450451
)
451452
bat = ctx.actions.declare_file(name + ".bat")
452453
ctx.actions.write(
@@ -461,10 +462,11 @@ def _go_tool_binary_impl(ctx):
461462
)
462463
else:
463464
# Note: GOPATH is needed for Go 1.16.
464-
cmd = "GOMAXPROCS=1 GOCACHE=$(mktemp -d) GOPATH=$(mktemp -d) {go} build -o {out} -trimpath {srcs}".format(
465+
cmd = "GOMAXPROCS=1 GOCACHE=$(mktemp -d) GOPATH=$(mktemp -d) {go} build -o {out} -trimpath -ldflags '{ldflags}' {srcs}".format(
465466
go = sdk.go.path,
466467
out = out.path,
467468
srcs = " ".join([f.path for f in ctx.files.srcs]),
469+
ldflags = ctx.attr.ldflags,
468470
)
469471
ctx.actions.run_shell(
470472
command = cmd,
@@ -490,6 +492,9 @@ go_tool_binary = rule(
490492
providers = [GoSDK],
491493
doc = "The SDK containing tools and libraries to build this binary",
492494
),
495+
"ldflags": attr.string(
496+
doc = "Raw value to pass to go build via -ldflags without tokenization",
497+
),
493498
},
494499
executable = True,
495500
doc = """Used instead of go_binary for executables used in the toolchain.

go/tools/builders/BUILD.bazel

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
load("//go:def.bzl", "go_binary", "go_source", "go_test")
22
load("//go/private/rules:transition.bzl", "go_reset_target")
3+
load("//go/private:common.bzl", "RULES_GO_STDLIB_PREFIX")
34

45
go_test(
56
name = "filter_test",
@@ -35,6 +36,9 @@ go_test(
3536
],
3637
data = ["@go_sdk//:files"],
3738
rundir = ".",
39+
x_defs = {
40+
"rulesGoStdlibPrefix": RULES_GO_STDLIB_PREFIX,
41+
},
3842
)
3943

4044
go_test(

go/tools/builders/stdliblist.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,16 @@ type goListPackage struct {
106106
DepsErrors []*flatPackagesError // errors loading dependencies
107107
}
108108

109+
var rulesGoStdlibPrefix string
110+
111+
func init() {
112+
if rulesGoStdlibPrefix == "" {
113+
panic("rulesGoStdlibPrefix should have been set via -X")
114+
}
115+
}
116+
109117
func stdlibPackageID(importPath string) string {
110-
return "@io_bazel_rules_go//stdlib:" + importPath
118+
return rulesGoStdlibPrefix + importPath
111119
}
112120

113121
// outputBasePath replace the cloneBase with output base label
@@ -280,7 +288,7 @@ func stdliblist(args []string) error {
280288

281289
encoder := json.NewEncoder(jsonFile)
282290
decoder := json.NewDecoder(jsonData)
283-
pathReplaceFn := func (s string) string {
291+
pathReplaceFn := func(s string) string {
284292
if strings.HasPrefix(s, absCachePath) {
285293
return strings.Replace(s, absCachePath, filepath.Join("__BAZEL_EXECROOT__", *cachePath), 1)
286294
}

go/tools/builders/stdliblist_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ func Test_stdliblist(t *testing.T) {
3333
t.Errorf("unable to decode output json: %v\n", err)
3434
}
3535

36-
if !strings.HasPrefix(result.ID, "@io_bazel_rules_go//stdlib") {
37-
t.Errorf("ID should be prefixed with @io_bazel_rules_go//stdlib :%v", result)
36+
if !strings.HasPrefix(result.ID, "@//stdlib:") {
37+
t.Errorf("ID should be prefixed with @//stdlib: :%v", result)
3838
}
3939
if !strings.HasPrefix(result.ExportFile, "__BAZEL_OUTPUT_BASE__") {
4040
t.Errorf("export file should be prefixed with __BAZEL_OUTPUT_BASE__ :%v", result)

go/tools/gopackagesdriver/BUILD.bazel

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
load("//go:def.bzl", "go_binary", "go_library")
22
load(":aspect.bzl", "bazel_supports_canonical_label_literals")
3+
load("//go/private:common.bzl", "RULES_GO_REPO_NAME")
34

45
go_library(
56
name = "gopackagesdriver_lib",
@@ -21,19 +22,10 @@ go_library(
2122
go_binary(
2223
name = "gopackagesdriver",
2324
embed = [":gopackagesdriver_lib"],
25+
visibility = ["//visibility:public"],
2426
x_defs = {
25-
# Determine the name of the rules_go repository as we need to specify it when invoking the
26-
# aspect.
27-
# If canonical label literals are supported, we can use a canonical label literal (starting
28-
# with @@) to pass the repository_name() through repo mapping unchanged.
29-
# If canonical label literals are not supported, then bzlmod is certainly not enabled and
30-
# we can assume that the repository name is not affected by repo mappings.
31-
# If run in the rules_go repo itself, repository_name() returns "@", which is equivalent to
32-
# "@io_bazel_rules_go" since Bazel 6:
33-
# https://github.com/bazelbuild/bazel/commit/7694cf75e6366b92e3905c2ad60234cda57627ee
34-
# TODO: Once we drop support for Bazel 5, we can remove the feature detection logic and
35-
# use "@" + repository_name().
36-
"rulesGoRepositoryName": "@" + repository_name() if bazel_supports_canonical_label_literals() else repository_name(),
27+
# Determine the repository part of labels pointing into the rules_go repo. This is needed
28+
# both to specify the aspect and to match labels in query output.
29+
"rulesGoRepositoryName": RULES_GO_REPO_NAME,
3730
},
38-
visibility = ["//visibility:public"],
3931
)

go/tools/gopackagesdriver/bazel_json_builder.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,12 @@ func (b *BazelJSONBuilder) outputGroupsForMode(mode LoadMode) string {
159159
}
160160

161161
func (b *BazelJSONBuilder) query(ctx context.Context, query string) ([]string, error) {
162-
queryArgs := concatStringsArrays(bazelQueryFlags, []string{
162+
var bzlmodQueryFlags []string
163+
if strings.HasPrefix(rulesGoRepositoryName, "@@") {
164+
// Requires Bazel 6.4.0.
165+
bzlmodQueryFlags = []string{"--consistent_labels"}
166+
}
167+
queryArgs := concatStringsArrays(bazelQueryFlags, bzlmodQueryFlags, []string{
163168
"--ui_event_filters=-info,-stderr",
164169
"--noshow_progress",
165170
"--order_output=no",

0 commit comments

Comments
 (0)