From 4feb7b8fe2a047e23c6327659896a2637cd0ca38 Mon Sep 17 00:00:00 2001
From: Marat Reymers <16486128+maratori@users.noreply.github.com>
Date: Sun, 18 Aug 2024 02:41:46 +0200
Subject: [PATCH 1/7] gomock: support uber's `-typed` flag (#4017)
**What type of PR is this?**
Feature
**What does this PR do? Why is it needed?**
The PR adds support for `-typed` flag in Uber's
[gomock](https://github.com/uber-go/mock?tab=readme-ov-file#flags).
**Which issues(s) does this PR fix?**
Fixes #4012
**Other notes for review**
I can add tests, but it requires adding a new dependency.
---
docs/go/extras/extras.md | 3 ++-
extras/gomock.bzl | 17 +++++++++++++++--
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/docs/go/extras/extras.md b/docs/go/extras/extras.md
index b3eb5be1df..a0a76c823d 100644
--- a/docs/go/extras/extras.md
+++ b/docs/go/extras/extras.md
@@ -34,7 +34,7 @@ This rule has moved. See [gazelle rule] in the Gazelle repository.
gomock(name, out, library, source_importpath, source, interfaces, package, self_package, aux_files,
- mockgen_tool, imports, copyright_file, mock_names, kwargs)
+ mockgen_tool, imports, copyright_file, mock_names, typed, kwargs)
Calls [mockgen](https://github.com/golang/mock) to generates a Go file containing mocks from the given library.
@@ -60,6 +60,7 @@ If `source` is given, the mocks are generated in source mode; otherwise in refle
| imports | dictionary of name-path pairs of explicit imports to use. See [mockgen's -imports](https://github.com/golang/mock#flags) for more information. | {} |
| copyright_file | optional file containing copyright to prepend to the generated contents. See [mockgen's -copyright_file](https://github.com/golang/mock#flags) for more information. | None |
| mock_names | dictionary of interface name to mock name pairs to change the output names of the mock objects. Mock names default to 'Mock' prepended to the name of the interface. See [mockgen's -mock_names](https://github.com/golang/mock#flags) for more information. | {} |
+| typed | generate type-safe 'Return', 'Do', 'DoAndReturn' functions. See [mockgen's -typed](https://github.com/uber-go/mock#flags) for more information. | False |
| kwargs | -
| none |
diff --git a/extras/gomock.bzl b/extras/gomock.bzl
index 2711ccb479..54e962a180 100644
--- a/extras/gomock.bzl
+++ b/extras/gomock.bzl
@@ -152,6 +152,10 @@ _gomock_source = rule(
allow_single_file = True,
mandatory = False,
),
+ "typed": attr.bool(
+ doc = "Generate type-safe 'Return', 'Do', 'DoAndReturn' functions.",
+ mandatory = False,
+ ),
"mockgen_tool": attr.label(
doc = "The mockgen tool to run",
default = _MOCKGEN_TOOL,
@@ -167,7 +171,7 @@ _gomock_source = rule(
toolchains = [GO_TOOLCHAIN],
)
-def gomock(name, out, library = None, source_importpath = "", source = None, interfaces = [], package = "", self_package = "", aux_files = {}, mockgen_tool = _MOCKGEN_TOOL, imports = {}, copyright_file = None, mock_names = {}, **kwargs):
+def gomock(name, out, library = None, source_importpath = "", source = None, interfaces = [], package = "", self_package = "", aux_files = {}, mockgen_tool = _MOCKGEN_TOOL, imports = {}, copyright_file = None, mock_names = {}, typed = False, **kwargs):
"""Calls [mockgen](https://github.com/golang/mock) to generates a Go file containing mocks from the given library.
If `source` is given, the mocks are generated in source mode; otherwise in reflective mode.
@@ -186,7 +190,8 @@ def gomock(name, out, library = None, source_importpath = "", source = None, int
imports: dictionary of name-path pairs of explicit imports to use. See [mockgen's -imports](https://github.com/golang/mock#flags) for more information.
copyright_file: optional file containing copyright to prepend to the generated contents. See [mockgen's -copyright_file](https://github.com/golang/mock#flags) for more information.
mock_names: dictionary of interface name to mock name pairs to change the output names of the mock objects. Mock names default to 'Mock' prepended to the name of the interface. See [mockgen's -mock_names](https://github.com/golang/mock#flags) for more information.
- kwargs: common attributes](https://bazel.build/reference/be/common-definitions#common-attributes) to all Bazel rules.
+ typed: generate type-safe 'Return', 'Do', 'DoAndReturn' functions. See [mockgen's -typed](https://github.com/uber-go/mock#flags) for more information.
+ kwargs: [common attributes](https://bazel.build/reference/be/common-definitions#common-attributes) to all Bazel rules.
"""
if source:
_gomock_source(
@@ -202,6 +207,7 @@ def gomock(name, out, library = None, source_importpath = "", source = None, int
imports = imports,
copyright_file = copyright_file,
mock_names = mock_names,
+ typed = typed,
**kwargs
)
else:
@@ -216,6 +222,7 @@ def gomock(name, out, library = None, source_importpath = "", source = None, int
imports = imports,
copyright_file = copyright_file,
mock_names = mock_names,
+ typed = typed,
**kwargs
)
@@ -360,6 +367,10 @@ _gomock_prog_exec = rule(
allow_single_file = True,
mandatory = False,
),
+ "typed": attr.bool(
+ doc = "Generate type-safe 'Return', 'Do', 'DoAndReturn' functions.",
+ mandatory = False,
+ ),
"prog_bin": attr.label(
doc = "The program binary generated by mockgen's -prog_only and compiled by bazel.",
allow_single_file = True,
@@ -398,5 +409,7 @@ def _handle_shared_args(ctx, args):
if len(ctx.attr.mock_names) > 0:
mock_names = ",".join(["{0}={1}".format(name, pkg) for name, pkg in ctx.attr.mock_names.items()])
args += ["-mock_names", mock_names]
+ if ctx.attr.typed:
+ args += ["-typed"]
return args, needed_files
From 552592ddf495cece93530b1f1e79b4beed675ecc Mon Sep 17 00:00:00 2001
From: Walter Cacau
Date: Sun, 18 Aug 2024 08:23:07 -0700
Subject: [PATCH 2/7] Making gopackagesdriver correctly handle relative queries
made from a subdirectory. (#4002)
**What type of PR is this?**
Bug fix
**What does this PR do? Why is it needed?**
It allows gopackagesdriver to be called within a subdirectory of a
workspace and properly resolve queries from gopls from IDEs like VSCode
when they include relative references. For example, if you have the
following repo/workspace structure:
```
WORKSPACE
project1/BUILD.bazel
project1/main.go
project2/BUILD.bazel
project2/main.go
...
```
If you run gopackagesdriver using bazel run from project1 directory and
receive the following queries:
```
file=./main.go
./...
```
The existing code will try to resolve those queries against the root of
the workspace, when instead we would expect it to resolve relative to
the project1 directory.
**Which issues(s) does this PR fix?**
Fixes #4001
**Other notes for review**
---
go/tools/gopackagesdriver/bazel.go | 24 ++--
.../gopackagesdriver/bazel_json_builder.go | 37 +++---
.../gopackagesdriver/gopackagesdriver_test.go | 107 +++++++++++++++++-
go/tools/gopackagesdriver/main.go | 3 +-
4 files changed, 140 insertions(+), 31 deletions(-)
diff --git a/go/tools/gopackagesdriver/bazel.go b/go/tools/gopackagesdriver/bazel.go
index 554c2af596..a88a60dcea 100644
--- a/go/tools/gopackagesdriver/bazel.go
+++ b/go/tools/gopackagesdriver/bazel.go
@@ -35,11 +35,12 @@ const (
)
type Bazel struct {
- bazelBin string
- workspaceRoot string
- bazelStartupFlags []string
- info map[string]string
- version bazelVersion
+ bazelBin string
+ workspaceRoot string
+ buildWorkingDirectory string
+ bazelStartupFlags []string
+ info map[string]string
+ version bazelVersion
}
// Minimal BEP structs to access the build outputs
@@ -52,11 +53,12 @@ type BEPNamedSet struct {
} `json:"namedSetOfFiles"`
}
-func NewBazel(ctx context.Context, bazelBin, workspaceRoot string, bazelStartupFlags []string) (*Bazel, error) {
+func NewBazel(ctx context.Context, bazelBin, workspaceRoot string, buildWorkingDirectory string, bazelStartupFlags []string) (*Bazel, error) {
b := &Bazel{
- bazelBin: bazelBin,
- workspaceRoot: workspaceRoot,
- bazelStartupFlags: bazelStartupFlags,
+ bazelBin: bazelBin,
+ workspaceRoot: workspaceRoot,
+ buildWorkingDirectory: buildWorkingDirectory,
+ bazelStartupFlags: bazelStartupFlags,
}
if err := b.fillInfo(ctx); err != nil {
return nil, fmt.Errorf("unable to query bazel info: %w", err)
@@ -163,6 +165,10 @@ func (b *Bazel) WorkspaceRoot() string {
return b.workspaceRoot
}
+func (b *Bazel) BuildWorkingDirectory() string {
+ return b.buildWorkingDirectory
+}
+
func (b *Bazel) ExecutionRoot() string {
return b.info["execution_root"]
}
diff --git a/go/tools/gopackagesdriver/bazel_json_builder.go b/go/tools/gopackagesdriver/bazel_json_builder.go
index f36108ce4b..a86c396011 100644
--- a/go/tools/gopackagesdriver/bazel_json_builder.go
+++ b/go/tools/gopackagesdriver/bazel_json_builder.go
@@ -21,7 +21,6 @@ import (
"fmt"
"io/ioutil"
"os"
- "path"
"path/filepath"
"regexp"
"runtime"
@@ -39,14 +38,9 @@ var _defaultKinds = []string{"go_library", "go_test", "go_binary"}
var externalRe = regexp.MustCompile(".*\\/external\\/([^\\/]+)(\\/(.*))?\\/([^\\/]+.go)")
-func (b *BazelJSONBuilder) fileQuery(filename string) string {
- label := filename
-
- if filepath.IsAbs(filename) {
- label, _ = filepath.Rel(b.bazel.WorkspaceRoot(), filename)
- } else if strings.HasPrefix(filename, "./") {
- label = strings.TrimPrefix(filename, "./")
- }
+func (b *BazelJSONBuilder) fileQuery(label string) string {
+ label = b.adjustToRelativePathIfPossible(label)
+ filename := filepath.FromSlash(label)
if matches := externalRe.FindStringSubmatch(filename); len(matches) == 5 {
// if filepath is for a third party lib, we need to know, what external
@@ -56,7 +50,7 @@ func (b *BazelJSONBuilder) fileQuery(filename string) string {
}
relToBin, err := filepath.Rel(b.bazel.info["output_path"], filename)
- if err == nil && !strings.HasPrefix(relToBin, "../") {
+ if err == nil && !strings.HasPrefix(relToBin, filepath.FromSlash("../")) {
parts := strings.SplitN(relToBin, string(filepath.Separator), 3)
relToBin = parts[2]
// We've effectively converted filename from bazel-bin/some/path.go to some/path.go;
@@ -93,12 +87,7 @@ func (b *BazelJSONBuilder) getKind() string {
}
func (b *BazelJSONBuilder) localQuery(request string) string {
- request = path.Clean(request)
- if filepath.IsAbs(request) {
- if relPath, err := filepath.Rel(workspaceRoot, request); err == nil {
- request = relPath
- }
- }
+ request = b.adjustToRelativePathIfPossible(request)
if !strings.HasSuffix(request, "...") {
request = fmt.Sprintf("%s:*", request)
@@ -107,6 +96,22 @@ func (b *BazelJSONBuilder) localQuery(request string) string {
return fmt.Sprintf(`kind("^(%s) rule$", %s)`, b.getKind(), request)
}
+func (b *BazelJSONBuilder) adjustToRelativePathIfPossible(request string) string {
+ // If request is a relative path and gopackagesdriver is ran within a subdirectory of the
+ // workspace, we must first resolve the absolute path for it.
+ // Note: Using FromSlash/ToSlash for handling windows
+ absRequest := filepath.FromSlash(request)
+ if !filepath.IsAbs(absRequest) {
+ absRequest = filepath.Join(b.bazel.BuildWorkingDirectory(), absRequest)
+ }
+ if relPath, err := filepath.Rel(workspaceRoot, absRequest); err == nil {
+ request = filepath.ToSlash(relPath)
+ } else {
+ fmt.Fprintf(os.Stderr, "error adjusting path to be relative to the workspace root from request %s: %v\n", request, err)
+ }
+ return request
+}
+
func (b *BazelJSONBuilder) packageQuery(importPath string) string {
if strings.HasSuffix(importPath, "/...") {
importPath = fmt.Sprintf(`^%s(/.+)?$`, strings.TrimSuffix(importPath, "/..."))
diff --git a/go/tools/gopackagesdriver/gopackagesdriver_test.go b/go/tools/gopackagesdriver/gopackagesdriver_test.go
index 3d61ab96ae..47d2ce6ce7 100644
--- a/go/tools/gopackagesdriver/gopackagesdriver_test.go
+++ b/go/tools/gopackagesdriver/gopackagesdriver_test.go
@@ -6,6 +6,7 @@ import (
"encoding/json"
"os"
"path"
+ "path/filepath"
"strings"
"testing"
@@ -61,6 +62,25 @@ package hello_test
import "testing"
func TestHelloExternal(t *testing.T) {}
+
+-- subhello/BUILD.bazel --
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+
+go_library(
+ name = "subhello",
+ srcs = ["subhello.go"],
+ importpath = "example.com/hello/subhello",
+ visibility = ["//visibility:public"],
+)
+
+-- subhello/subhello.go --
+package subhello
+
+import "os"
+
+func main() {
+ fmt.Fprintln(os.Stderr, "Subdirectory Hello World!")
+}
`,
})
}
@@ -71,7 +91,7 @@ const (
)
func TestBaseFileLookup(t *testing.T) {
- resp := runForTest(t, "file=hello.go")
+ resp := runForTest(t, ".", "file=hello.go")
t.Run("roots", func(t *testing.T) {
if len(resp.Roots) != 1 {
@@ -140,8 +160,80 @@ func TestBaseFileLookup(t *testing.T) {
})
}
+func TestRelativeFileLookup(t *testing.T) {
+ resp := runForTest(t, "subhello", "file=./subhello.go")
+
+ t.Run("roots", func(t *testing.T) {
+ if len(resp.Roots) != 1 {
+ t.Errorf("Expected 1 package root: %+v", resp.Roots)
+ return
+ }
+
+ if !strings.HasSuffix(resp.Roots[0], "//subhello:subhello") {
+ t.Errorf("Unexpected package id: %q", resp.Roots[0])
+ return
+ }
+ })
+
+ t.Run("package", func(t *testing.T) {
+ var pkg *FlatPackage
+ for _, p := range resp.Packages {
+ if p.ID == resp.Roots[0] {
+ pkg = p
+ }
+ }
+
+ if pkg == nil {
+ t.Errorf("Expected to find %q in resp.Packages", resp.Roots[0])
+ return
+ }
+
+ if len(pkg.CompiledGoFiles) != 1 || len(pkg.GoFiles) != 1 ||
+ path.Base(pkg.GoFiles[0]) != "subhello.go" || path.Base(pkg.CompiledGoFiles[0]) != "subhello.go" {
+ t.Errorf("Expected to find 1 file (subhello.go) in (Compiled)GoFiles:\n%+v", pkg)
+ return
+ }
+ })
+}
+
+func TestRelativePatternWildcardLookup(t *testing.T) {
+ resp := runForTest(t, "subhello", "./...")
+
+ t.Run("roots", func(t *testing.T) {
+ if len(resp.Roots) != 1 {
+ t.Errorf("Expected 1 package root: %+v", resp.Roots)
+ return
+ }
+
+ if !strings.HasSuffix(resp.Roots[0], "//subhello:subhello") {
+ t.Errorf("Unexpected package id: %q", resp.Roots[0])
+ return
+ }
+ })
+
+ t.Run("package", func(t *testing.T) {
+ var pkg *FlatPackage
+ for _, p := range resp.Packages {
+ if p.ID == resp.Roots[0] {
+ pkg = p
+ }
+ }
+
+ if pkg == nil {
+ t.Errorf("Expected to find %q in resp.Packages", resp.Roots[0])
+ return
+ }
+
+ if len(pkg.CompiledGoFiles) != 1 || len(pkg.GoFiles) != 1 ||
+ path.Base(pkg.GoFiles[0]) != "subhello.go" || path.Base(pkg.CompiledGoFiles[0]) != "subhello.go" {
+ t.Errorf("Expected to find 1 file (subhello.go) in (Compiled)GoFiles:\n%+v", pkg)
+ return
+ }
+ })
+}
+
func TestExternalTests(t *testing.T) {
- resp := runForTest(t, "file=hello_external_test.go")
+ resp := runForTest(t, ".", "file=hello_external_test.go")
if len(resp.Roots) != 2 {
t.Errorf("Expected exactly two roots for package: %+v", resp.Roots)
}
@@ -167,7 +259,7 @@ func TestExternalTests(t *testing.T) {
}
}
-func runForTest(t *testing.T, args ...string) driverResponse {
+func runForTest(t *testing.T, relativeWorkingDir string, args ...string) driverResponse {
t.Helper()
// Remove most environment variables, other than those on an allowlist.
@@ -210,7 +302,7 @@ func runForTest(t *testing.T, args ...string) driverResponse {
}
}()
- // Set workspaceRoot global variable.
+ // Set workspaceRoot and buildWorkingDirectory global variable.
// It's initialized to the BUILD_WORKSPACE_DIRECTORY environment variable
// before this point.
wd, err := os.Getwd()
@@ -218,8 +310,13 @@ func runForTest(t *testing.T, args ...string) driverResponse {
t.Fatal(err)
}
oldWorkspaceRoot := workspaceRoot
+ oldBuildWorkingDirectory := buildWorkingDirectory
workspaceRoot = wd
- defer func() { workspaceRoot = oldWorkspaceRoot }()
+ buildWorkingDirectory = filepath.Join(wd, relativeWorkingDir)
+ defer func() {
+ workspaceRoot = oldWorkspaceRoot
+ buildWorkingDirectory = oldBuildWorkingDirectory
+ }()
in := strings.NewReader("{}")
out := &bytes.Buffer{}
diff --git a/go/tools/gopackagesdriver/main.go b/go/tools/gopackagesdriver/main.go
index 0613a3be79..ea591f6f7c 100644
--- a/go/tools/gopackagesdriver/main.go
+++ b/go/tools/gopackagesdriver/main.go
@@ -62,6 +62,7 @@ var (
bazelQueryScope = getenvDefault("GOPACKAGESDRIVER_BAZEL_QUERY_SCOPE", "")
bazelBuildFlags = strings.Fields(os.Getenv("GOPACKAGESDRIVER_BAZEL_BUILD_FLAGS"))
workspaceRoot = os.Getenv("BUILD_WORKSPACE_DIRECTORY")
+ buildWorkingDirectory = os.Getenv("BUILD_WORKING_DIRECTORY")
additionalAspects = strings.Fields(os.Getenv("GOPACKAGESDRIVER_BAZEL_ADDTL_ASPECTS"))
additionalKinds = strings.Fields(os.Getenv("GOPACKAGESDRIVER_BAZEL_KINDS"))
emptyResponse = &driverResponse{
@@ -81,7 +82,7 @@ func run(ctx context.Context, in io.Reader, out io.Writer, args []string) error
return fmt.Errorf("unable to read request: %w", err)
}
- bazel, err := NewBazel(ctx, bazelBin, workspaceRoot, bazelStartupFlags)
+ bazel, err := NewBazel(ctx, bazelBin, workspaceRoot, buildWorkingDirectory, bazelStartupFlags)
if err != nil {
return fmt.Errorf("unable to create bazel instance: %w", err)
}
From 8e662663a818e35da17cb8d47d3da1ad558fb51c Mon Sep 17 00:00:00 2001
From: David Zbarsky
Date: Sun, 18 Aug 2024 16:31:27 -0400
Subject: [PATCH 3/7] Cleanup GoArchiveData usage of as_tuple (#4032)
**What type of PR is this?**
Starlark cleanup
**What does this PR do? Why is it needed?**
`as_tuple` was showing up in profiles, we can just call `tuple`
directly. This exposed that we were flattening a depset which isn't
necessary.
**Which issues(s) does this PR fix?**
Fixes #
**Other notes for review**
---
go/private/actions/archive.bzl | 22 +++++++++-------------
go/private/common.bzl | 10 ----------
go/private/rules/test.bzl | 2 +-
3 files changed, 10 insertions(+), 24 deletions(-)
diff --git a/go/private/actions/archive.bzl b/go/private/actions/archive.bzl
index 40e9d5d87e..8ee3e84392 100644
--- a/go/private/actions/archive.bzl
+++ b/go/private/actions/archive.bzl
@@ -12,10 +12,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-load(
- "//go/private:common.bzl",
- "as_tuple",
-)
load(
"//go/private:context.bzl",
"get_nogo",
@@ -170,17 +166,17 @@ def emit_archive(go, source = None, _recompile_suffix = "", recompile_internal_d
pathtype = source.library.pathtype,
# GoSource fields
- srcs = as_tuple(source.srcs),
+ srcs = tuple(source.srcs),
_cover = source.cover,
- _embedsrcs = as_tuple(source.embedsrcs),
+ _embedsrcs = tuple(source.embedsrcs),
_x_defs = tuple(source.x_defs.items()),
- _gc_goopts = as_tuple(source.gc_goopts),
+ _gc_goopts = tuple(source.gc_goopts),
_cgo = source.cgo,
- _cdeps = as_tuple(source.cdeps),
- _cppopts = as_tuple(source.cppopts),
- _copts = as_tuple(source.copts),
- _cxxopts = as_tuple(source.cxxopts),
- _clinkopts = as_tuple(source.clinkopts),
+ _cdeps = tuple(source.cdeps),
+ _cppopts = tuple(source.cppopts),
+ _copts = tuple(source.copts),
+ _cxxopts = tuple(source.cxxopts),
+ _clinkopts = tuple(source.clinkopts),
# Information on dependencies
_dep_labels = tuple([d.data.label for d in direct]),
@@ -192,7 +188,7 @@ def emit_archive(go, source = None, _recompile_suffix = "", recompile_internal_d
facts_file = out_facts,
runfiles = source.runfiles,
_validation_output = out_nogo_validation,
- _cgo_deps = as_tuple(cgo_deps),
+ _cgo_deps = cgo_deps,
)
x_defs = dict(source.x_defs)
for a in direct:
diff --git a/go/private/common.bzl b/go/private/common.bzl
index 2d5d016b4f..27378b375b 100644
--- a/go/private/common.bzl
+++ b/go/private/common.bzl
@@ -162,16 +162,6 @@ def as_iterable(v):
return v.to_list()
fail("as_iterator failed on {}".format(v))
-def as_tuple(v):
- """Returns a list, tuple, or depset as a tuple."""
- if type(v) == "tuple":
- return v
- if type(v) == "list":
- return tuple(v)
- if type(v) == "depset":
- return tuple(v.to_list())
- fail("as_tuple failed on {}".format(v))
-
def count_group_matches(v, prefix, suffix):
"""Counts reluctant substring matches between prefix and suffix.
diff --git a/go/private/rules/test.bzl b/go/private/rules/test.bzl
index 6b29bb922b..6b970f4ef3 100644
--- a/go/private/rules/test.bzl
+++ b/go/private/rules/test.bzl
@@ -699,7 +699,7 @@ def _recompile_external_deps(go, external_source, internal_archive, library_labe
libs = depset(direct = [arc_data.file], transitive = [a.libs for a in deps]),
transitive = depset(direct = [arc_data], transitive = [a.transitive for a in deps]),
x_defs = source.x_defs,
- cgo_deps = depset(direct = arc_data._cgo_deps, transitive = [a.cgo_deps for a in deps]),
+ cgo_deps = depset(transitive = [arc_data._cgo_deps] + [a.cgo_deps for a in deps]),
cgo_exports = depset(transitive = [a.cgo_exports for a in deps]),
runfiles = source.runfiles,
mode = go.mode,
From 5418bcb89cc501f2d7f3e968c50f4ef178c88902 Mon Sep 17 00:00:00 2001
From: David Zbarsky
Date: Sun, 18 Aug 2024 16:56:24 -0400
Subject: [PATCH 4/7] Cleanup get_mode (#4055)
**What type of PR is this?**
Starlark cleanup
**What does this PR do? Why is it needed?**
This PR simplifies and optimizes `get_mode`. It's now around 2-3x
faster, which is a 100ms win on the buildbuddy repo.
**Which issues(s) does this PR fix?**
Fixes #
**Other notes for review**
---
go/private/context.bzl | 10 +++-
go/private/mode.bzl | 116 +++++++++++++++++++----------------------
2 files changed, 63 insertions(+), 63 deletions(-)
diff --git a/go/private/context.bzl b/go/private/context.bzl
index dcb75128de..ba4e26f220 100644
--- a/go/private/context.bzl
+++ b/go/private/context.bzl
@@ -855,6 +855,14 @@ cgo_context_data_proxy = rule(
)
def _go_config_impl(ctx):
+ pgo_profiles = ctx.attr.pgoprofile.files.to_list()
+ if len(pgo_profiles) > 2:
+ fail("providing more than one pprof file to pgoprofile is not supported")
+ if len(pgo_profiles) == 1:
+ pgoprofile = pgo_profiles[0]
+ else:
+ pgoprofile = None
+
return [GoConfigInfo(
static = ctx.attr.static[BuildSettingInfo].value,
race = ctx.attr.race[BuildSettingInfo].value,
@@ -870,7 +878,7 @@ def _go_config_impl(ctx):
gc_goopts = ctx.attr.gc_goopts[BuildSettingInfo].value,
amd64 = ctx.attr.amd64,
arm = ctx.attr.arm,
- pgoprofile = ctx.attr.pgoprofile,
+ pgoprofile = pgoprofile,
)]
go_config = rule(
diff --git a/go/private/mode.bzl b/go/private/mode.bzl
index 0f6291d17f..173934aaa0 100644
--- a/go/private/mode.bzl
+++ b/go/private/mode.bzl
@@ -12,6 +12,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+load(":providers.bzl", "GoConfigInfo")
+
# Modes are documented in go/modes.rst#compilation-modes
LINKMODE_NORMAL = "normal"
@@ -58,89 +60,79 @@ def mode_string(mode):
result.extend(mode.gc_goopts)
return "_".join(result)
-def _ternary(*values):
- for v in values:
- if v == None:
- continue
- if type(v) == "bool":
- return v
- if type(v) != "string":
- fail("Invalid value type {}".format(type(v)))
- v = v.lower()
- if v == "on":
- return True
- if v == "off":
- return False
- if v == "auto":
- continue
- fail("Invalid value {}".format(v))
- fail("_ternary failed to produce a final result from {}".format(values))
+default_go_config_info = GoConfigInfo(
+ static = False,
+ race = False,
+ msan = False,
+ pure = False,
+ strip = False,
+ debug = False,
+ linkmode = LINKMODE_NORMAL,
+ gc_linkopts = [],
+ tags = [],
+ stamp = False,
+ cover_format = None,
+ gc_goopts = [],
+ amd64 = None,
+ arm = None,
+ pgoprofile = None,
+)
def get_mode(ctx, go_toolchain, cgo_context_info, go_config_info):
- static = _ternary(go_config_info.static if go_config_info else "off")
- if getattr(ctx.attr, "pure", None) == "off" and not cgo_context_info:
- fail("{} has pure explicitly set to off, but no C++ toolchain could be found for its platform".format(ctx.label))
- pure = _ternary(
- "on" if not cgo_context_info else "auto",
- go_config_info.pure if go_config_info else "off",
- )
- race = _ternary(go_config_info.race if go_config_info else "off")
- msan = _ternary(go_config_info.msan if go_config_info else "off")
- strip = go_config_info.strip if go_config_info else False
- stamp = go_config_info.stamp if go_config_info else False
- debug = go_config_info.debug if go_config_info else False
- linkmode = go_config_info.linkmode if go_config_info else LINKMODE_NORMAL
- cover_format = go_config_info and go_config_info.cover_format
- amd64 = go_config_info.amd64 if go_config_info else None
- arm = go_config_info.arm if go_config_info else None
+ if go_config_info == None:
+ go_config_info = default_go_config_info
+
+ if not cgo_context_info:
+ if getattr(ctx.attr, "pure", None) == "off":
+ fail("{} has pure explicitly set to off, but no C++ toolchain could be found for its platform".format(ctx.label))
+ pure = True
+ else:
+ pure = go_config_info.pure
+
+ race = go_config_info.race
+ msan = go_config_info.msan
+ linkmode = go_config_info.linkmode
goos = go_toolchain.default_goos if getattr(ctx.attr, "goos", "auto") == "auto" else ctx.attr.goos
goarch = go_toolchain.default_goarch if getattr(ctx.attr, "goarch", "auto") == "auto" else ctx.attr.goarch
- gc_goopts = go_config_info.gc_goopts if go_config_info else []
- pgoprofile = None
- if go_config_info:
- if len(go_config_info.pgoprofile.files.to_list()) > 2:
- fail("providing more than one pprof file to pgoprofile is not supported")
- elif len(go_config_info.pgoprofile.files.to_list()) == 1:
- pgoprofile = go_config_info.pgoprofile.files.to_list()[0]
# TODO(jayconrod): check for more invalid and contradictory settings.
- if pure and race:
- fail("race instrumentation can't be enabled when cgo is disabled. Check that pure is not set to \"off\" and a C/C++ toolchain is configured.")
- if pure and msan:
- fail("msan instrumentation can't be enabled when cgo is disabled. Check that pure is not set to \"off\" and a C/C++ toolchain is configured.")
- if pure and linkmode in LINKMODES_REQUIRING_EXTERNAL_LINKING:
- fail(("linkmode '{}' can't be used when cgo is disabled. Check that pure is not set to \"off\" and that a C/C++ toolchain is configured for " +
- "your current platform. If you defined a custom platform, make sure that it has the @io_bazel_rules_go//go/toolchain:cgo_on constraint value.").format(linkmode))
-
- gc_linkopts = list(go_config_info.gc_linkopts) if go_config_info else []
- tags = list(go_config_info.tags) if go_config_info else []
+ if pure:
+ if race:
+ fail("race instrumentation can't be enabled when cgo is disabled. Check that pure is not set to \"off\" and a C/C++ toolchain is configured.")
+ if msan:
+ fail("msan instrumentation can't be enabled when cgo is disabled. Check that pure is not set to \"off\" and a C/C++ toolchain is configured.")
+ if linkmode in LINKMODES_REQUIRING_EXTERNAL_LINKING:
+ fail(("linkmode '{}' can't be used when cgo is disabled. Check that pure is not set to \"off\" and that a C/C++ toolchain is configured for " +
+ "your current platform. If you defined a custom platform, make sure that it has the @io_bazel_rules_go//go/toolchain:cgo_on constraint value.").format(linkmode))
+
+ tags = list(go_config_info.tags)
if "gotags" in ctx.var:
- tags.extend(ctx.var["gotags"].split(","))
+ tags += ctx.var["gotags"].split(",")
if cgo_context_info:
- tags.extend(cgo_context_info.tags)
+ tags += cgo_context_info.tags
if race:
tags.append("race")
if msan:
tags.append("msan")
return struct(
- static = static,
+ static = go_config_info.static,
race = race,
msan = msan,
pure = pure,
link = linkmode,
- gc_linkopts = gc_linkopts,
- strip = strip,
- stamp = stamp,
- debug = debug,
+ gc_linkopts = go_config_info.gc_linkopts,
+ strip = go_config_info.strip,
+ stamp = go_config_info.stamp,
+ debug = go_config_info.debug,
goos = goos,
goarch = goarch,
tags = tags,
- cover_format = cover_format,
- amd64 = amd64,
- arm = arm,
- gc_goopts = gc_goopts,
- pgoprofile = pgoprofile,
+ cover_format = go_config_info.cover_format,
+ amd64 = go_config_info.amd64,
+ arm = go_config_info.arm,
+ gc_goopts = go_config_info.gc_goopts,
+ pgoprofile = go_config_info.pgoprofile,
)
def installsuffix(mode):
From 2cbd80609e113efed4877b6b8ffd5255960ee8aa Mon Sep 17 00:00:00 2001
From: David Zbarsky
Date: Mon, 19 Aug 2024 03:38:35 -0400
Subject: [PATCH 5/7] Cleanup _merge_embed (#4048)
**What type of PR is this?**
Starlark cleanup
**What does this PR do? Why is it needed?**
Since only one the lib or one of the embeds can specify cgo, we can
simplify this a bit
**Which issues(s) does this PR fix?**
Fixes #
**Other notes for review**
---
go/private/context.bzl | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/go/private/context.bzl b/go/private/context.bzl
index ba4e26f220..ad0270fe35 100644
--- a/go/private/context.bzl
+++ b/go/private/context.bzl
@@ -222,14 +222,16 @@ def _merge_embed(source, embed):
source["x_defs"].update(s.x_defs)
source["gc_goopts"] = source["gc_goopts"] + s.gc_goopts
source["runfiles"] = source["runfiles"].merge(s.runfiles)
- if s.cgo and source["cgo"]:
- fail("multiple libraries with cgo enabled")
- source["cgo"] = source["cgo"] or s.cgo
- source["cdeps"] = source["cdeps"] or s.cdeps
- source["cppopts"] = source["cppopts"] or s.cppopts
- source["copts"] = source["copts"] or s.copts
- source["cxxopts"] = source["cxxopts"] or s.cxxopts
- source["clinkopts"] = source["clinkopts"] or s.clinkopts
+
+ if s.cgo:
+ if source["cgo"]:
+ fail("multiple libraries with cgo enabled")
+ source["cgo"] = s.cgo
+ source["cdeps"] = s.cdeps
+ source["cppopts"] = s.cppopts
+ source["copts"] = s.copts
+ source["cxxopts"] = s.cxxopts
+ source["clinkopts"] = s.clinkopts
def _dedup_archives(archives):
"""Returns a list of archives without duplicate import paths.
@@ -258,9 +260,7 @@ def _library_to_source(go, attr, library, coverage_instrumented):
srcs = attr_srcs + generated_srcs
embedsrcs = [f for t in getattr(attr, "embedsrcs", []) for f in as_iterable(t.files)]
attr_deps = getattr(attr, "deps", [])
- generated_deps = getattr(library, "deps", [])
- deps = attr_deps + generated_deps
- deps = [get_archive(dep) for dep in deps]
+ deps = [get_archive(dep) for dep in attr_deps]
source = {
"library": library,
"mode": go.mode,
From 756d39ca69a3bf902227907c9ff8818818369408 Mon Sep 17 00:00:00 2001
From: David Zbarsky
Date: Mon, 19 Aug 2024 18:15:56 -0400
Subject: [PATCH 6/7] Remove mode_tags_equivalent (#4057)
**What type of PR is this?**
starlark cleanup
**What does this PR do? Why is it needed?**
It's unused
**Which issues(s) does this PR fix?**
Fixes #
**Other notes for review**
---
go/private/mode.bzl | 8 --------
1 file changed, 8 deletions(-)
diff --git a/go/private/mode.bzl b/go/private/mode.bzl
index 173934aaa0..66cef3a5bc 100644
--- a/go/private/mode.bzl
+++ b/go/private/mode.bzl
@@ -143,14 +143,6 @@ def installsuffix(mode):
s += "_msan"
return s
-def mode_tags_equivalent(l, r):
- # Returns whether two modes are equivalent for Go build tags. For example,
- # goos and goarch must match, but static doesn't matter.
- return (l.goos == r.goos and
- l.goarch == r.goarch and
- l.race == r.race and
- l.msan == r.msan)
-
# Ported from https://github.com/golang/go/blob/master/src/cmd/go/internal/work/init.go#L76
_LINK_C_ARCHIVE_PLATFORMS = {
"darwin/arm64": None,
From f3029e2a89432e06f9e4c8d304f114f54b5436eb Mon Sep 17 00:00:00 2001
From: David Zbarsky
Date: Tue, 20 Aug 2024 03:13:43 -0400
Subject: [PATCH 7/7] Remove more hasattr/getattr calls in go_context (#4054)
**What type of PR is this?**
starlark cleanup
**What does this PR do? Why is it needed?**
The current logic to grab all the providers is confusing; it runs the
fallback logics even in the case when `go_context_data` is found. Easier
to follow when we pass the data in explicitly, and a bit faster as well.
I also considered making a `go_context_v2` api that doesn't have all the
fallbacks and using that to power `go_context`, but didn't do it for
now.
**Which issues(s) does this PR fix?**
Fixes #
**Other notes for review**
---
go/private/context.bzl | 42 +++++++++++++++++++++---------------
go/private/rules/binary.bzl | 8 ++++++-
go/private/rules/library.bzl | 19 +++++++++++++++-
go/private/rules/test.bzl | 8 ++++++-
4 files changed, 57 insertions(+), 20 deletions(-)
diff --git a/go/private/context.bzl b/go/private/context.bzl
index ad0270fe35..03fe09887e 100644
--- a/go/private/context.bzl
+++ b/go/private/context.bzl
@@ -381,7 +381,8 @@ def go_context(
importpath = None,
importmap = None,
embed = None,
- importpath_aliases = None):
+ importpath_aliases = None,
+ go_context_data = None):
"""Returns an API used to build Go code.
See /go/toolchains.rst#go-context
@@ -390,26 +391,33 @@ def go_context(
attr = ctx.attr
toolchain = ctx.toolchains[GO_TOOLCHAIN]
cgo_context_info = None
+ go_context_info = None
go_config_info = None
stdlib = None
- coverdata = None
- nogo = None
- if hasattr(attr, "_go_context_data"):
- go_context_data = _flatten_possibly_transitioned_attr(attr._go_context_data)
+
+ if go_context_data == None:
+ if hasattr(attr, "_go_context_data"):
+ go_context_data = _flatten_possibly_transitioned_attr(attr._go_context_data)
+ if CgoContextInfo in go_context_data:
+ cgo_context_info = go_context_data[CgoContextInfo]
+ go_config_info = go_context_data[GoConfigInfo]
+ stdlib = go_context_data[GoStdLib]
+ go_context_info = go_context_data[GoContextInfo]
+ if getattr(attr, "_cgo_context_data", None) and CgoContextInfo in attr._cgo_context_data:
+ cgo_context_info = attr._cgo_context_data[CgoContextInfo]
+ if getattr(attr, "cgo_context_data", None) and CgoContextInfo in attr.cgo_context_data:
+ cgo_context_info = attr.cgo_context_data[CgoContextInfo]
+ if hasattr(attr, "_go_config"):
+ go_config_info = attr._go_config[GoConfigInfo]
+ if hasattr(attr, "_stdlib"):
+ stdlib = _flatten_possibly_transitioned_attr(attr._stdlib)[GoStdLib]
+ else:
+ go_context_data = _flatten_possibly_transitioned_attr(go_context_data)
if CgoContextInfo in go_context_data:
cgo_context_info = go_context_data[CgoContextInfo]
go_config_info = go_context_data[GoConfigInfo]
stdlib = go_context_data[GoStdLib]
- coverdata = go_context_data[GoContextInfo].coverdata
- nogo = go_context_data[GoContextInfo].nogo
- if getattr(attr, "_cgo_context_data", None) and CgoContextInfo in attr._cgo_context_data:
- cgo_context_info = attr._cgo_context_data[CgoContextInfo]
- if getattr(attr, "cgo_context_data", None) and CgoContextInfo in attr.cgo_context_data:
- cgo_context_info = attr.cgo_context_data[CgoContextInfo]
- if hasattr(attr, "_go_config"):
- go_config_info = attr._go_config[GoConfigInfo]
- if hasattr(attr, "_stdlib"):
- stdlib = _flatten_possibly_transitioned_attr(attr._stdlib)[GoStdLib]
+ go_context_info = go_context_data[GoContextInfo]
mode = get_mode(ctx, toolchain, cgo_context_info, go_config_info)
@@ -515,8 +523,8 @@ def go_context(
importpath_aliases = importpath_aliases,
pathtype = pathtype,
cgo_tools = cgo_tools,
- nogo = nogo,
- coverdata = coverdata,
+ nogo = go_context_info.nogo if go_context_info else None,
+ coverdata = go_context_info.coverdata if go_context_info else None,
coverage_enabled = ctx.configuration.coverage_enabled,
coverage_instrumented = ctx.coverage_instrumented(),
env = env,
diff --git a/go/private/rules/binary.bzl b/go/private/rules/binary.bzl
index 193fd3741a..cd863af97f 100644
--- a/go/private/rules/binary.bzl
+++ b/go/private/rules/binary.bzl
@@ -115,7 +115,13 @@ _go_cc_aspect = aspect(
def _go_binary_impl(ctx):
"""go_binary_impl emits actions for compiling and linking a go executable."""
- go = go_context(ctx, include_deprecated_properties = False)
+ go = go_context(
+ ctx,
+ include_deprecated_properties = False,
+ importpath = ctx.attr.importpath,
+ embed = ctx.attr.embed,
+ go_context_data = ctx.attr._go_context_data,
+ )
is_main = go.mode.link not in (LINKMODE_SHARED, LINKMODE_PLUGIN)
library = go.new_library(go, importable = False, is_main = is_main)
diff --git a/go/private/rules/library.bzl b/go/private/rules/library.bzl
index 23476bc56a..d830409e4f 100644
--- a/go/private/rules/library.bzl
+++ b/go/private/rules/library.bzl
@@ -39,8 +39,11 @@ def _go_library_impl(ctx):
include_deprecated_properties = False,
importpath = ctx.attr.importpath,
importmap = ctx.attr.importmap,
+ importpath_aliases = ctx.attr.importpath_aliases,
embed = ctx.attr.embed,
+ go_context_data = ctx.attr._go_context_data,
)
+
library = go.new_library(go)
source = go.library_to_source(go, ctx.attr, library, ctx.coverage_instrumented())
archive = go.archive(go, source)
@@ -204,8 +207,22 @@ go_library = rule(
# See docs/go/core/rules.md#go_library for full documentation.
+def _go_tool_library_impl(ctx):
+ """Implements the go_tool_library() rule."""
+ go = go_context(ctx, include_deprecated_properties = False)
+
+ library = go.new_library(go)
+ source = go.library_to_source(go, ctx.attr, library, ctx.coverage_instrumented())
+ archive = go.archive(go, source)
+
+ return [
+ library,
+ source,
+ archive,
+ ]
+
go_tool_library = rule(
- _go_library_impl,
+ _go_tool_library_impl,
attrs = {
"data": attr.label_list(allow_files = True),
"srcs": attr.label_list(allow_files = True),
diff --git a/go/private/rules/test.bzl b/go/private/rules/test.bzl
index 6b970f4ef3..3c9b4b02d1 100644
--- a/go/private/rules/test.bzl
+++ b/go/private/rules/test.bzl
@@ -56,7 +56,13 @@ def _go_test_impl(ctx):
It emits an action to run the test generator, and then compiles the
test into a binary."""
- go = go_context(ctx, include_deprecated_properties = False)
+ go = go_context(
+ ctx,
+ include_deprecated_properties = False,
+ importpath = ctx.attr.importpath,
+ embed = ctx.attr.embed,
+ go_context_data = ctx.attr._go_context_data,
+ )
# Compile the library to test with internal white box tests
internal_library = go.new_library(go, testfilter = "exclude")