Skip to content

Commit

Permalink
Fix matching between Starlark and query labels in gopackagesdriver (#…
Browse files Browse the repository at this point in the history
…3701)

This relies on the query flag `--consistent_labels`, which is available
in Bazel 6.4.0.
  • Loading branch information
fmeum authored Sep 27, 2023
1 parent d1da1bb commit c2406b2
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 20 deletions.
2 changes: 2 additions & 0 deletions go/private/BUILD.sdk.bazel
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
load("@io_bazel_rules_go//go/private/rules:binary.bzl", "go_tool_binary")
load("@io_bazel_rules_go//go/private/rules:sdk.bzl", "package_list")
load("@io_bazel_rules_go//go/private/rules:transition.bzl", "non_go_reset_target")
load("@io_bazel_rules_go//go/private:common.bzl", "RULES_GO_STDLIB_PREFIX")
load("@io_bazel_rules_go//go/private:go_toolchain.bzl", "declare_go_toolchains")
load("@io_bazel_rules_go//go:def.bzl", "go_sdk")

Expand Down Expand Up @@ -51,6 +52,7 @@ go_sdk(
go_tool_binary(
name = "builder",
srcs = ["@io_bazel_rules_go//go/tools/builders:builder_srcs"],
ldflags = "-X main.rulesGoStdlibPrefix={}".format(RULES_GO_STDLIB_PREFIX),
sdk = ":go_sdk",
)

Expand Down
7 changes: 7 additions & 0 deletions go/private/common.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -252,3 +252,10 @@ COVERAGE_OPTIONS_DENYLIST = {
"-fprofile-instr-generate": None,
"-fcoverage-mapping": None,
}

_RULES_GO_RAW_REPO_NAME = str(Label("//:unused"))[:-len("//:unused")]

# When rules_go is the main repository and Bazel < 6 is used, the repo name does
# not start with a "@", so we need to add it.
RULES_GO_REPO_NAME = _RULES_GO_RAW_REPO_NAME if _RULES_GO_RAW_REPO_NAME.startswith("@") else "@" + _RULES_GO_RAW_REPO_NAME
RULES_GO_STDLIB_PREFIX = RULES_GO_REPO_NAME + "//stdlib:"
9 changes: 7 additions & 2 deletions go/private/rules/binary.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -441,12 +441,13 @@ def _go_tool_binary_impl(ctx):
if sdk.goos == "windows":
gopath = ctx.actions.declare_directory("gopath")
gocache = ctx.actions.declare_directory("gocache")
cmd = "@echo off\nset GOMAXPROCS=1\nset GOCACHE=%cd%\\{gocache}\nset GOPATH=%cd%\\{gopath}\n{go} build -o {out} -trimpath {srcs}".format(
cmd = "@echo off\nset GOMAXPROCS=1\nset GOCACHE=%cd%\\{gocache}\nset GOPATH=%cd%\\{gopath}\n{go} build -o {out} -trimpath -ldflags \"{ldflags}\" {srcs}".format(
gopath = gopath.path,
gocache = gocache.path,
go = sdk.go.path.replace("/", "\\"),
out = out.path,
srcs = " ".join([f.path for f in ctx.files.srcs]),
ldflags = ctx.attr.ldflags,
)
bat = ctx.actions.declare_file(name + ".bat")
ctx.actions.write(
Expand All @@ -461,10 +462,11 @@ def _go_tool_binary_impl(ctx):
)
else:
# Note: GOPATH is needed for Go 1.16.
cmd = "GOMAXPROCS=1 GOCACHE=$(mktemp -d) GOPATH=$(mktemp -d) {go} build -o {out} -trimpath {srcs}".format(
cmd = "GOMAXPROCS=1 GOCACHE=$(mktemp -d) GOPATH=$(mktemp -d) {go} build -o {out} -trimpath -ldflags '{ldflags}' {srcs}".format(
go = sdk.go.path,
out = out.path,
srcs = " ".join([f.path for f in ctx.files.srcs]),
ldflags = ctx.attr.ldflags,
)
ctx.actions.run_shell(
command = cmd,
Expand All @@ -490,6 +492,9 @@ go_tool_binary = rule(
providers = [GoSDK],
doc = "The SDK containing tools and libraries to build this binary",
),
"ldflags": attr.string(
doc = "Raw value to pass to go build via -ldflags without tokenization",
),
},
executable = True,
doc = """Used instead of go_binary for executables used in the toolchain.
Expand Down
4 changes: 4 additions & 0 deletions go/tools/builders/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
load("//go:def.bzl", "go_binary", "go_source", "go_test")
load("//go/private/rules:transition.bzl", "go_reset_target")
load("//go/private:common.bzl", "RULES_GO_STDLIB_PREFIX")

go_test(
name = "filter_test",
Expand Down Expand Up @@ -35,6 +36,9 @@ go_test(
],
data = ["@go_sdk//:files"],
rundir = ".",
x_defs = {
"rulesGoStdlibPrefix": RULES_GO_STDLIB_PREFIX,
},
)

go_test(
Expand Down
12 changes: 10 additions & 2 deletions go/tools/builders/stdliblist.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,16 @@ type goListPackage struct {
DepsErrors []*flatPackagesError // errors loading dependencies
}

var rulesGoStdlibPrefix string

func init() {
if rulesGoStdlibPrefix == "" {
panic("rulesGoStdlibPrefix should have been set via -X")
}
}

func stdlibPackageID(importPath string) string {
return "@io_bazel_rules_go//stdlib:" + importPath
return rulesGoStdlibPrefix + importPath
}

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

encoder := json.NewEncoder(jsonFile)
decoder := json.NewDecoder(jsonData)
pathReplaceFn := func (s string) string {
pathReplaceFn := func(s string) string {
if strings.HasPrefix(s, absCachePath) {
return strings.Replace(s, absCachePath, filepath.Join("__BAZEL_EXECROOT__", *cachePath), 1)
}
Expand Down
4 changes: 2 additions & 2 deletions go/tools/builders/stdliblist_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ func Test_stdliblist(t *testing.T) {
t.Errorf("unable to decode output json: %v\n", err)
}

if !strings.HasPrefix(result.ID, "@io_bazel_rules_go//stdlib") {
t.Errorf("ID should be prefixed with @io_bazel_rules_go//stdlib :%v", result)
if !strings.HasPrefix(result.ID, "@//stdlib:") {
t.Errorf("ID should be prefixed with @//stdlib: :%v", result)
}
if !strings.HasPrefix(result.ExportFile, "__BAZEL_OUTPUT_BASE__") {
t.Errorf("export file should be prefixed with __BAZEL_OUTPUT_BASE__ :%v", result)
Expand Down
18 changes: 5 additions & 13 deletions go/tools/gopackagesdriver/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
load("//go:def.bzl", "go_binary", "go_library")
load(":aspect.bzl", "bazel_supports_canonical_label_literals")
load("//go/private:common.bzl", "RULES_GO_REPO_NAME")

go_library(
name = "gopackagesdriver_lib",
Expand All @@ -21,19 +22,10 @@ go_library(
go_binary(
name = "gopackagesdriver",
embed = [":gopackagesdriver_lib"],
visibility = ["//visibility:public"],
x_defs = {
# Determine the name of the rules_go repository as we need to specify it when invoking the
# aspect.
# If canonical label literals are supported, we can use a canonical label literal (starting
# with @@) to pass the repository_name() through repo mapping unchanged.
# If canonical label literals are not supported, then bzlmod is certainly not enabled and
# we can assume that the repository name is not affected by repo mappings.
# If run in the rules_go repo itself, repository_name() returns "@", which is equivalent to
# "@io_bazel_rules_go" since Bazel 6:
# https://github.com/bazelbuild/bazel/commit/7694cf75e6366b92e3905c2ad60234cda57627ee
# TODO: Once we drop support for Bazel 5, we can remove the feature detection logic and
# use "@" + repository_name().
"rulesGoRepositoryName": "@" + repository_name() if bazel_supports_canonical_label_literals() else repository_name(),
# Determine the repository part of labels pointing into the rules_go repo. This is needed
# both to specify the aspect and to match labels in query output.
"rulesGoRepositoryName": RULES_GO_REPO_NAME,
},
visibility = ["//visibility:public"],
)
7 changes: 6 additions & 1 deletion go/tools/gopackagesdriver/bazel_json_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,12 @@ func (b *BazelJSONBuilder) outputGroupsForMode(mode LoadMode) string {
}

func (b *BazelJSONBuilder) query(ctx context.Context, query string) ([]string, error) {
queryArgs := concatStringsArrays(bazelQueryFlags, []string{
var bzlmodQueryFlags []string
if strings.HasPrefix(rulesGoRepositoryName, "@@") {
// Requires Bazel 6.4.0.
bzlmodQueryFlags = []string{"--consistent_labels"}
}
queryArgs := concatStringsArrays(bazelQueryFlags, bzlmodQueryFlags, []string{
"--ui_event_filters=-info,-stderr",
"--noshow_progress",
"--order_output=no",
Expand Down

0 comments on commit c2406b2

Please sign in to comment.