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
This relies on the query flag `--consistent_labels`, which is available
in Bazel 6.4.0.
  • Loading branch information
fmeum committed Sep 19, 2023
1 parent 09a206c commit 5ebf1b3
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 5ebf1b3

Please sign in to comment.