From c4757e50bb1fbba0bd63d42541800b04345523a0 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Thu, 3 Aug 2023 16:14:44 +0200 Subject: [PATCH] Add support for isolated `go_deps` usages (#1584) Since isolated extension usages have their own sets of tags and repos, overrides can be allowed for such usages even in non-root modules. These usages also have to use their own dedicated repo config file. --- .bazelci/presubmit.yml | 1 + .bcr/presubmit.yml | 3 ++ MODULE.bazel | 2 +- internal/bzlmod/go_deps.bzl | 14 +++++-- internal/go_repository.bzl | 16 +++++++- tests/bcr/.bazelignore | 1 + tests/bcr/.bazelrc | 1 + tests/bcr/.bazelversion | 2 +- tests/bcr/MODULE.bazel | 6 +++ tests/bcr/test_dep/BUILD.bazel | 8 ++++ tests/bcr/test_dep/MODULE.bazel | 50 ++++++++++++++++++++++++ tests/bcr/test_dep/WORKSPACE | 0 tests/bcr/test_dep/patches/BUILD.bazel | 0 tests/bcr/test_dep/patches/testify.patch | 13 ++++++ tests/bcr/test_dep/test.go | 13 ++++++ 15 files changed, 123 insertions(+), 7 deletions(-) create mode 100644 tests/bcr/.bazelignore create mode 100644 tests/bcr/test_dep/BUILD.bazel create mode 100644 tests/bcr/test_dep/MODULE.bazel create mode 100644 tests/bcr/test_dep/WORKSPACE create mode 100644 tests/bcr/test_dep/patches/BUILD.bazel create mode 100644 tests/bcr/test_dep/patches/testify.patch create mode 100644 tests/bcr/test_dep/test.go diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index 78472ca20..04bfdbd7b 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -37,6 +37,7 @@ tasks: # Specify this target explicitly to verify that Gazelle generated it correctly. - "//pkg:pkg_test" - "//..." + - "@test_dep//..." macos_arm64: platform: macos_arm64 build_targets: diff --git a/.bcr/presubmit.yml b/.bcr/presubmit.yml index 4a6c7690d..2c80bda3e 100644 --- a/.bcr/presubmit.yml +++ b/.bcr/presubmit.yml @@ -11,6 +11,8 @@ bcr_test_module: run_test_module: name: Run test module platform: ${{ platform }} + build_flags: + - --experimental_isolated_extension_usages shell_commands: # Regenerate the BUILD file for the test module using Gazelle. - rm pkg/BUILD.bazel @@ -22,3 +24,4 @@ bcr_test_module: # Specify this target explicitly to verify that Gazelle generated it correctly. - "//pkg:pkg_test" - "//..." + - "@test_dep//..." diff --git a/MODULE.bazel b/MODULE.bazel index 275b1750c..c22533a3d 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -36,7 +36,7 @@ use_repo( "org_golang_x_mod", "org_golang_x_sync", "org_golang_x_tools", - # Read by go_repository rules. + # Referenced by `gazelle_binary`. "bazel_gazelle_go_repository_config", ) diff --git a/internal/bzlmod/go_deps.bzl b/internal/bzlmod/go_deps.bzl index 1006d3897..7b9289bb6 100644 --- a/internal/bzlmod/go_deps.bzl +++ b/internal/bzlmod/go_deps.bzl @@ -29,10 +29,16 @@ the required directives to the "default_gazelle_overrides.bzl" file at \ https://github.com/bazelbuild/bazel-gazelle/tree/master/internal/bzlmod/default_gazelle_overrides.bzl. """ -def _fail_on_non_root_overrides(module, tag_class): +def _fail_on_non_root_overrides(module_ctx, module, tag_class): if module.is_root: return + # Isolated module extension usages only contain tags from a single module, so we can allow + # overrides. This is a new feature in Bazel 6.3.0, earlier versions do not allow module usages + # to be isolated. + if getattr(module_ctx, "is_isolated", False): + return + if getattr(module.tags, tag_class): fail(_FORBIDDEN_OVERRIDE_TAG.format( tag_class = tag_class, @@ -151,7 +157,7 @@ def _go_deps_impl(module_ctx): elif check_direct_deps == "error": outdated_direct_dep_printer = fail - _fail_on_non_root_overrides(module, "gazelle_override") + _fail_on_non_root_overrides(module_ctx, module, "gazelle_override") for gazelle_override_tag in module.tags.gazelle_override: if gazelle_override_tag.path in gazelle_overrides: fail("Multiple overrides defined for Go module path \"{}\" in module \"{}\".".format(gazelle_override_tag.path, module.name)) @@ -163,7 +169,7 @@ def _go_deps_impl(module_ctx): build_file_generation = gazelle_override_tag.build_file_generation, ) - _fail_on_non_root_overrides(module, "module_override") + _fail_on_non_root_overrides(module_ctx, module, "module_override") for module_override_tag in module.tags.module_override: if module_override_tag.path in module_overrides: fail("Multiple overrides defined for Go module path \"{}\" in module \"{}\".".format(module_override_tag.path, module.name)) @@ -188,7 +194,7 @@ def _go_deps_impl(module_ctx): for tag in module_tags_from_go_mod ] - if module.is_root: + if module.is_root or getattr(module_ctx, "is_isolated", False): replace_map.update(go_mod_replace_map) else: # Register this Bazel module as providing the specified Go module. It participates diff --git a/internal/go_repository.bzl b/internal/go_repository.bzl index 565e75937..cb0ac7405 100644 --- a/internal/go_repository.bzl +++ b/internal/go_repository.bzl @@ -280,6 +280,20 @@ def _go_repository_impl(ctx): # Run Gazelle if gazelle_path == None: gazelle_path = ctx.path(Label(_gazelle)) + + # ctx.attr.name is the canonical name of this repository, which contains a '~' if and only + # if this repository is generated by a module extension rather than an invocation in + # WORKSPACE. + if "~" in ctx.attr.name: + # TODO: In Bazel 6.3.0 and earlier, there is no way to obtain a label referencing a repo + # generated by an extension from within that extension. We thus have to manually + # construct such a label pointing to the sibling `_go_repository_config` repo created by + # the `go_deps` extension. All extension-generated repos have names of the form + # `~`. + extension_repo_prefix = ctx.attr.name.rpartition("~")[0] + "~" + repo_config = ctx.path(Label("@@" + extension_repo_prefix + "bazel_gazelle_go_repository_config//:WORKSPACE")) + else: + repo_config = ctx.path(ctx.attr.build_config) cmd = [ gazelle_path, "-go_repository_mode", @@ -290,7 +304,7 @@ def _go_repository_impl(ctx): "-repo_root", ctx.path(""), "-repo_config", - ctx.path(ctx.attr.build_config), + repo_config, ] if ctx.attr.version: cmd.append("-go_repository_module_mode") diff --git a/tests/bcr/.bazelignore b/tests/bcr/.bazelignore new file mode 100644 index 000000000..777d432da --- /dev/null +++ b/tests/bcr/.bazelignore @@ -0,0 +1 @@ +test_dep diff --git a/tests/bcr/.bazelrc b/tests/bcr/.bazelrc index 3ce91d272..bf48b0e6c 100644 --- a/tests/bcr/.bazelrc +++ b/tests/bcr/.bazelrc @@ -1 +1,2 @@ common --enable_bzlmod +common --experimental_isolated_extension_usages diff --git a/tests/bcr/.bazelversion b/tests/bcr/.bazelversion index 6abaeb2f9..dc0208aba 100644 --- a/tests/bcr/.bazelversion +++ b/tests/bcr/.bazelversion @@ -1 +1 @@ -6.2.0 +6.3.1 diff --git a/tests/bcr/MODULE.bazel b/tests/bcr/MODULE.bazel index 72b77e83b..52faae47a 100644 --- a/tests/bcr/MODULE.bazel +++ b/tests/bcr/MODULE.bazel @@ -8,6 +8,12 @@ local_path_override( path = "../..", ) +bazel_dep(name = "test_dep", version = "1.0.0") +local_path_override( + module_name = "test_dep", + path = "test_dep", +) + bazel_dep(name = "rules_go", version = "0.39.1", repo_name = "my_rules_go") # This bazel_dep provides the Go dependency github.com/cloudflare/circl, which requires custom diff --git a/tests/bcr/test_dep/BUILD.bazel b/tests/bcr/test_dep/BUILD.bazel new file mode 100644 index 000000000..a4ab157c1 --- /dev/null +++ b/tests/bcr/test_dep/BUILD.bazel @@ -0,0 +1,8 @@ +load("@rules_go//go:def.bzl", "go_test") + +go_test( + name = "test", + srcs = ["test.go"], + visibility = ["//visibility:public"], + deps = ["@com_github_stretchr_testify//require"], +) diff --git a/tests/bcr/test_dep/MODULE.bazel b/tests/bcr/test_dep/MODULE.bazel new file mode 100644 index 000000000..c2278edf2 --- /dev/null +++ b/tests/bcr/test_dep/MODULE.bazel @@ -0,0 +1,50 @@ +module( + name = "test_dep", + version = "1.0.0", +) + +bazel_dep(name = "rules_go", version = "0.41.0") +bazel_dep(name = "gazelle", version = "0.31.0") + +# An isolated extension usage does not share any tags with any other usage and results in a +# completely separate set of repos. We explicitly use different directives and patches here to +# verify that they do not interfere with those of the root test module. +go_deps = use_extension("@gazelle//:extensions.bzl", "go_deps", isolate = True) +go_deps.module( + path = "github.com/stretchr/testify", + sum = "h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk=", + version = "v1.8.0", +) +go_deps.gazelle_override( + directives = [ + "gazelle:go_naming_convention import", + ], + path = "github.com/stretchr/testify", +) +go_deps.module_override( + patch_strip = 1, + patches = [ + "//patches:testify.patch", + ], + path = "github.com/stretchr/testify", +) + +go_deps.module( + indirect = True, + path = "gopkg.in/yaml.v3", + sum = "h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=", + version = "v3.0.1", +) +go_deps.gazelle_override( + path = "gopkg.in/yaml.v3", + directives = [ + "gazelle:go_naming_convention import", + ], +) +go_deps.module( + indirect = True, + path = "github.com/davecgh/go-spew", + sum = "h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=", + version = "v1.1.1", +) +use_repo(go_deps, "com_github_stretchr_testify") diff --git a/tests/bcr/test_dep/WORKSPACE b/tests/bcr/test_dep/WORKSPACE new file mode 100644 index 000000000..e69de29bb diff --git a/tests/bcr/test_dep/patches/BUILD.bazel b/tests/bcr/test_dep/patches/BUILD.bazel new file mode 100644 index 000000000..e69de29bb diff --git a/tests/bcr/test_dep/patches/testify.patch b/tests/bcr/test_dep/patches/testify.patch new file mode 100644 index 000000000..4c8db505e --- /dev/null +++ b/tests/bcr/test_dep/patches/testify.patch @@ -0,0 +1,13 @@ +diff --git a/require/require.go b/require/require.go +index a6e1b7c..bc21879 100644 +--- a/require/require.go ++++ b/require/require.go +@@ -12,6 +12,8 @@ import ( + time "time" + ) + ++const Hello = "not_hello" ++ + // Condition uses a Comparison to assert a complex condition. + func Condition(t TestingT, comp assert.Comparison, msgAndArgs ...interface{}) { + if h, ok := t.(tHelper); ok { diff --git a/tests/bcr/test_dep/test.go b/tests/bcr/test_dep/test.go new file mode 100644 index 000000000..586b5deac --- /dev/null +++ b/tests/bcr/test_dep/test.go @@ -0,0 +1,13 @@ +package main + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestPatch(t *testing.T) { + // A patch is used to add this constant with a value that differs from that patched into the + // root module's version of this dep. + require.Equal(t, "not_hello", require.Hello) +}