From 1b94058b5c2103e028793feb99305888d87fe17a Mon Sep 17 00:00:00 2001 From: Mike Bland Date: Wed, 9 Oct 2024 15:09:17 -0400 Subject: [PATCH 1/8] Add apparent repo name utilities to modules.bzl Adds the following macros to work with apparent repo names when running under Bzlmod. - `adjust_main_repo_prefix` - `apparent_repo_label_string` - `apparent_repo_name` Originally developed while updating rules_scala to support Bzlmod as part of bazelbuild/rules_scala#1482. For examples of their use, see bazelbuild/rules_scala#1621. --- docs/modules_doc.md | 74 ++++++++++++++++++++ lib/modules.bzl | 71 +++++++++++++++++++ tests/modules_test.bzl | 153 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 296 insertions(+), 2 deletions(-) diff --git a/docs/modules_doc.md b/docs/modules_doc.md index 18042eed..4a8c7fb1 100755 --- a/docs/modules_doc.md +++ b/docs/modules_doc.md @@ -2,6 +2,80 @@ Skylib module containing utilities for Bazel modules and module extensions. + + +## modules.adjust_main_repo_prefix + +
+modules.adjust_main_repo_prefix(target_pattern)
+
+ +Updates the main repo prefix to match the current Bazel version. + +Used to automatically update strings representing include/exclude target +patterns so that they match actual main repo target Labels correctly. The +main repo prefix will be "@//" for Bazel < 7.1.0, and "@@//" for Bazel >= +7.1.0 under Bzlmod. + + +**PARAMETERS** + + +| Name | Description | Default Value | +| :------------- | :------------- | :------------- | +| target_pattern | a string used to match a BUILD target pattern | none | + +**RETURNS** + +the string with any main repository prefix updated to match the current + Bazel version + + + + +## modules.apparent_repo_label_string + +
+modules.apparent_repo_label_string(label)
+
+ +Return a Label string starting with its apparent repo name. + +**PARAMETERS** + + +| Name | Description | Default Value | +| :------------- | :------------- | :------------- | +| label | a Label instance | none | + +**RETURNS** + +str(label) with its canonical repository name replaced with its apparent + repository name + + + + +## modules.apparent_repo_name + +
+modules.apparent_repo_name(label_or_name)
+
+ +Return a repository's apparent repository name. + +**PARAMETERS** + + +| Name | Description | Default Value | +| :------------- | :------------- | :------------- | +| label_or_name | a Label or repository name string | none | + +**RETURNS** + +The apparent repository name + + ## modules.as_extension diff --git a/lib/modules.bzl b/lib/modules.bzl index 61c8f87f..7882294a 100644 --- a/lib/modules.bzl +++ b/lib/modules.bzl @@ -114,7 +114,78 @@ def _use_all_repos(module_ctx, reproducible = False): **extension_metadata_kwargs ) +def _apparent_repo_name(label_or_name): + """Return a repository's apparent repository name. + + Args: + label_or_name: a Label or repository name string + + Returns: + The apparent repository name + """ + repo_name = getattr(label_or_name, "repo_name", label_or_name).lstrip("@") + delimiter_indices = [] + + # Bazed on this pattern from the Bazel source: + # com.google.devtools.build.lib.cmdline.RepositoryName.VALID_REPO_NAME + for i in range(len(repo_name)): + c = repo_name[i] + if not (c.isalnum() or c in "_-."): + delimiter_indices.append(i) + + if len(delimiter_indices) == 0: + # Already an apparent repo name, apparently. + return repo_name + + if len(delimiter_indices) == 1: + # The name is for a top level module, possibly containing a version ID. + return repo_name[:delimiter_indices[0]] + + return repo_name[delimiter_indices[-1] + 1:] + +def _apparent_repo_label_string(label): + """Return a Label string starting with its apparent repo name. + + Args: + label: a Label instance + + Returns: + str(label) with its canonical repository name replaced with its apparent + repository name + """ + if len(label.repo_name) == 0: + return str(label) + + label_str = "@" + str(label).lstrip("@") + return label_str.replace(label.repo_name, _apparent_repo_name(label)) + +_main_repo_prefix = str(Label("@@//:all")).split(":")[0] + +def _adjust_main_repo_prefix(target_pattern): + """Updates the main repository prefix to match the current Bazel version. + + The main repo prefix will be "@//" for Bazel < 7.1.0, and "@@//" for Bazel + >= 7.1.0 under Bzlmod. This macro automatically updates strings representing + include/exclude target patterns so that they match actual main repository + target Labels correctly. + + Args: + target_pattern: a string used to match a BUILD target pattern + + Returns: + the string with any main repository prefix updated to match the current + Bazel version + """ + if target_pattern.startswith("@//") or target_pattern.startswith("@@//"): + return _main_repo_prefix + target_pattern.lstrip("@/") + + return target_pattern + modules = struct( as_extension = _as_extension, use_all_repos = _use_all_repos, + apparent_repo_name = _apparent_repo_name, + apparent_repo_label_string = _apparent_repo_label_string, + main_repo_prefix = _main_repo_prefix, + adjust_main_repo_prefix = _adjust_main_repo_prefix ) diff --git a/tests/modules_test.bzl b/tests/modules_test.bzl index 0887463c..3a9a6468 100644 --- a/tests/modules_test.bzl +++ b/tests/modules_test.bzl @@ -15,8 +15,11 @@ """Test usage of modules.bzl.""" load("//lib:modules.bzl", "modules") +load("//lib:unittest.bzl", "asserts", "unittest") load("//rules:build_test.bzl", "build_test") +_is_bzlmod_enabled = str(Label("//tests:module_tests.bzl")).startswith("@@") + def _repo_rule_impl(repository_ctx): repository_ctx.file("WORKSPACE") repository_ctx.file("BUILD", """exports_files(["hello"])""") @@ -45,12 +48,158 @@ use_all_repos_test_ext = module_extension( doc = "Only used for testing modules.use_all_repos().", ) +def _apparent_repo_name_test(ctx): + """Unit tests for modules.apparent_repo_name.""" + env = unittest.begin(ctx) + + asserts.equals( + env, "", modules.apparent_repo_name(""), + msg = "Handles the empty string as input", + ) + + asserts.equals( + env, "foo", modules.apparent_repo_name("foo"), + msg = ( + "Return the original name unchanged if it doesn't start with `@`.", + ), + ) + + asserts.equals( + env, "foo", modules.apparent_repo_name("@foo"), + msg = "Return the original name without `@` if already apparent.", + ) + + asserts.equals( + env, "foo", modules.apparent_repo_name(Label("@foo").repo_name), + msg = "Return the apparent name from a canonical name string.", + ) + + asserts.equals( + env, "", modules.apparent_repo_name(Label("@@//:all")), + msg = "Returns the empty string for a main repository Label.", + ) + + asserts.equals( + env, "", modules.apparent_repo_name(Label("@bazel_skylib//:all")), + msg = " ".join([ + "Returns the empty string for a Label containing the main", + "repository's module name.", + ]), + ) + + asserts.equals( + env, "foo", modules.apparent_repo_name(Label("@foo")), + msg = "Return the apparent name from a Label.", + ) + + asserts.equals( + env, "rules_pkg", modules.apparent_repo_name(Label("@rules_pkg")), + msg = " ".join([ + "Top level module repos have the canonical name delimiter at the", + "end. Therefore, this should not return the empty string, but the", + "name without the leading `@` and trailing delimiter.", + ]), + ) + + asserts.equals( + env, + "stardoc" if _is_bzlmod_enabled else "io_bazel_stardoc", + modules.apparent_repo_name(Label("@io_bazel_stardoc")), + msg = " ".join([ + "Label values will already map bazel_dep repo_names to", + "actual repo names under Bzlmod (no-op under WORKSPACE)." + ]) + ) + + asserts.equals( + env, "foo", modules.apparent_repo_name("foo+1.2.3"), + msg = "Ignores version numbers in canonical repo names", + ) + + return unittest.end(env) + +apparent_repo_name_test = unittest.make(_apparent_repo_name_test) + +def _apparent_repo_label_string_test(ctx): + """Unit tests for modules.apparent_repo_label_string.""" + env = unittest.begin(ctx) + + main_repo = str(Label("//:all")) + asserts.equals( + env, main_repo, modules.apparent_repo_label_string(Label("//:all")), + msg = "Returns top level target with leading `@` or `@@`", + ) + + main_module_label = Label("@bazel_skylib//:all") + asserts.equals( + env, main_repo, modules.apparent_repo_label_string(main_module_label), + msg = " ".join([ + "Returns top level target with leading `@` or `@@`", + "for a Label containing the main module's name", + ]), + ) + + rules_pkg = "@rules_pkg//:all" + asserts.equals( + env, rules_pkg, modules.apparent_repo_label_string(Label(rules_pkg)), + msg = "Returns original repo name", + ) + + asserts.equals( + env, + "@%s//:all" % ("stardoc" if _is_bzlmod_enabled else "io_bazel_stardoc"), + modules.apparent_repo_label_string(Label("@io_bazel_stardoc//:all")), + msg = " ".join([ + "Returns the actual module name instead of", + "repo_name from bazel_dep() (no-op under WORKSPACE).", + ]), + ) + + return unittest.end(env) + +apparent_repo_label_string_test = unittest.make( + _apparent_repo_label_string_test +) + +def _adjust_main_repo_prefix_test(ctx): + """Unit tests for modules.apparent_repo_label_string.""" + env = unittest.begin(ctx) + + expected = modules.main_repo_prefix + ":all" + asserts.equals( + env, expected, modules.adjust_main_repo_prefix("@//:all"), + msg = "Normalizes a target pattern starting with `@//`.", + ) + + asserts.equals( + env, expected, modules.adjust_main_repo_prefix("@@//:all"), + msg = "Normalizes a target pattern starting with `@@//`.", + ) + + original = "@not_the_main_repo" + asserts.equals( + env, original, modules.adjust_main_repo_prefix(original), + msg = "Returns non main repo target patterns unchanged.", + ) + + return unittest.end(env) + +adjust_main_repo_prefix_test = unittest.make( + _adjust_main_repo_prefix_test +) + # buildifier: disable=unnamed-macro def modules_test_suite(): """Creates the tests for modules.bzl if Bzlmod is enabled.""" - is_bzlmod_enabled = str(Label("//tests:module_tests.bzl")).startswith("@@") - if not is_bzlmod_enabled: + unittest.suite( + "modules_tests", + apparent_repo_name_test, + apparent_repo_label_string_test, + adjust_main_repo_prefix_test, + ) + + if not _is_bzlmod_enabled: return build_test( From d9216c0ee10be388bc7548fbf822101ff5ac0c6b Mon Sep 17 00:00:00 2001 From: Mike Bland Date: Mon, 14 Oct 2024 17:42:52 -0400 Subject: [PATCH 2/8] Fix buildifier, docs errors from CI on #548 D'oh! --- docs/modules_doc.md | 10 +++---- lib/modules.bzl | 2 +- tests/modules_test.bzl | 68 ++++++++++++++++++++++++++++++------------ 3 files changed, 55 insertions(+), 25 deletions(-) diff --git a/docs/modules_doc.md b/docs/modules_doc.md index 4a8c7fb1..b2dbf206 100755 --- a/docs/modules_doc.md +++ b/docs/modules_doc.md @@ -10,12 +10,12 @@ Skylib module containing utilities for Bazel modules and module extensions. modules.adjust_main_repo_prefix(target_pattern) -Updates the main repo prefix to match the current Bazel version. +Updates the main repository prefix to match the current Bazel version. -Used to automatically update strings representing include/exclude target -patterns so that they match actual main repo target Labels correctly. The -main repo prefix will be "@//" for Bazel < 7.1.0, and "@@//" for Bazel >= -7.1.0 under Bzlmod. +The main repo prefix will be "@//" for Bazel < 7.1.0, and "@@//" for Bazel +>= 7.1.0 under Bzlmod. This macro automatically updates strings representing +include/exclude target patterns so that they match actual main repository +target Labels correctly. **PARAMETERS** diff --git a/lib/modules.bzl b/lib/modules.bzl index 7882294a..76c1e420 100644 --- a/lib/modules.bzl +++ b/lib/modules.bzl @@ -187,5 +187,5 @@ modules = struct( apparent_repo_name = _apparent_repo_name, apparent_repo_label_string = _apparent_repo_label_string, main_repo_prefix = _main_repo_prefix, - adjust_main_repo_prefix = _adjust_main_repo_prefix + adjust_main_repo_prefix = _adjust_main_repo_prefix, ) diff --git a/tests/modules_test.bzl b/tests/modules_test.bzl index 3a9a6468..d0c8b1e0 100644 --- a/tests/modules_test.bzl +++ b/tests/modules_test.bzl @@ -53,34 +53,46 @@ def _apparent_repo_name_test(ctx): env = unittest.begin(ctx) asserts.equals( - env, "", modules.apparent_repo_name(""), + env, + "", + modules.apparent_repo_name(""), msg = "Handles the empty string as input", ) asserts.equals( - env, "foo", modules.apparent_repo_name("foo"), + env, + "foo", + modules.apparent_repo_name("foo"), msg = ( "Return the original name unchanged if it doesn't start with `@`.", ), ) asserts.equals( - env, "foo", modules.apparent_repo_name("@foo"), + env, + "foo", + modules.apparent_repo_name("@foo"), msg = "Return the original name without `@` if already apparent.", ) asserts.equals( - env, "foo", modules.apparent_repo_name(Label("@foo").repo_name), + env, + "foo", + modules.apparent_repo_name(Label("@foo").repo_name), msg = "Return the apparent name from a canonical name string.", ) asserts.equals( - env, "", modules.apparent_repo_name(Label("@@//:all")), + env, + "", + modules.apparent_repo_name(Label("@@//:all")), msg = "Returns the empty string for a main repository Label.", ) asserts.equals( - env, "", modules.apparent_repo_name(Label("@bazel_skylib//:all")), + env, + "", + modules.apparent_repo_name(Label("@bazel_skylib//:all")), msg = " ".join([ "Returns the empty string for a Label containing the main", "repository's module name.", @@ -88,12 +100,16 @@ def _apparent_repo_name_test(ctx): ) asserts.equals( - env, "foo", modules.apparent_repo_name(Label("@foo")), + env, + "foo", + modules.apparent_repo_name(Label("@foo")), msg = "Return the apparent name from a Label.", ) asserts.equals( - env, "rules_pkg", modules.apparent_repo_name(Label("@rules_pkg")), + env, + "rules_pkg", + modules.apparent_repo_name(Label("@rules_pkg")), msg = " ".join([ "Top level module repos have the canonical name delimiter at the", "end. Therefore, this should not return the empty string, but the", @@ -107,12 +123,14 @@ def _apparent_repo_name_test(ctx): modules.apparent_repo_name(Label("@io_bazel_stardoc")), msg = " ".join([ "Label values will already map bazel_dep repo_names to", - "actual repo names under Bzlmod (no-op under WORKSPACE)." - ]) + "actual repo names under Bzlmod (no-op under WORKSPACE).", + ]), ) asserts.equals( - env, "foo", modules.apparent_repo_name("foo+1.2.3"), + env, + "foo", + modules.apparent_repo_name("foo+1.2.3"), msg = "Ignores version numbers in canonical repo names", ) @@ -126,13 +144,17 @@ def _apparent_repo_label_string_test(ctx): main_repo = str(Label("//:all")) asserts.equals( - env, main_repo, modules.apparent_repo_label_string(Label("//:all")), + env, + main_repo, + modules.apparent_repo_label_string(Label("//:all")), msg = "Returns top level target with leading `@` or `@@`", ) main_module_label = Label("@bazel_skylib//:all") asserts.equals( - env, main_repo, modules.apparent_repo_label_string(main_module_label), + env, + main_repo, + modules.apparent_repo_label_string(main_module_label), msg = " ".join([ "Returns top level target with leading `@` or `@@`", "for a Label containing the main module's name", @@ -141,7 +163,9 @@ def _apparent_repo_label_string_test(ctx): rules_pkg = "@rules_pkg//:all" asserts.equals( - env, rules_pkg, modules.apparent_repo_label_string(Label(rules_pkg)), + env, + rules_pkg, + modules.apparent_repo_label_string(Label(rules_pkg)), msg = "Returns original repo name", ) @@ -158,7 +182,7 @@ def _apparent_repo_label_string_test(ctx): return unittest.end(env) apparent_repo_label_string_test = unittest.make( - _apparent_repo_label_string_test + _apparent_repo_label_string_test, ) def _adjust_main_repo_prefix_test(ctx): @@ -167,25 +191,31 @@ def _adjust_main_repo_prefix_test(ctx): expected = modules.main_repo_prefix + ":all" asserts.equals( - env, expected, modules.adjust_main_repo_prefix("@//:all"), + env, + expected, + modules.adjust_main_repo_prefix("@//:all"), msg = "Normalizes a target pattern starting with `@//`.", ) asserts.equals( - env, expected, modules.adjust_main_repo_prefix("@@//:all"), + env, + expected, + modules.adjust_main_repo_prefix("@@//:all"), msg = "Normalizes a target pattern starting with `@@//`.", ) original = "@not_the_main_repo" asserts.equals( - env, original, modules.adjust_main_repo_prefix(original), + env, + original, + modules.adjust_main_repo_prefix(original), msg = "Returns non main repo target patterns unchanged.", ) return unittest.end(env) adjust_main_repo_prefix_test = unittest.make( - _adjust_main_repo_prefix_test + _adjust_main_repo_prefix_test, ) # buildifier: disable=unnamed-macro From d0e2f9561c8922b23baa575f9fe2d34709d913e5 Mon Sep 17 00:00:00 2001 From: Mike Bland Date: Mon, 14 Oct 2024 17:47:40 -0400 Subject: [PATCH 3/8] Make repo name macros compatible with Bazel 5 The Bazel 5 build from #548 broke because: ```txt (21:43:53) ERROR: Traceback (most recent call last): File "/workdir/lib/modules.bzl", line 162, column 30, in _main_repo_prefix = str(Label("@@//:all")).split(":")[0] Error in Label: Illegal absolute label syntax: @@//:all ``` Then things broke locally because `Label.repo_name` isn't available in Bazel 5. Changed the `@@//` prefixes to `//` and added the new `repo_name` utility to adapt to Bazel 5. Confirmed locally that everything now works with Bazel 5.4.1, 6.5.0, and 7.3.2. --- docs/modules_doc.md | 27 +++++++++++++++++++++++++++ lib/modules.bzl | 28 ++++++++++++++++++++++++---- tests/modules_test.bzl | 4 ++-- 3 files changed, 53 insertions(+), 6 deletions(-) diff --git a/docs/modules_doc.md b/docs/modules_doc.md index b2dbf206..3d3b013a 100755 --- a/docs/modules_doc.md +++ b/docs/modules_doc.md @@ -112,6 +112,33 @@ imported via `use_repo`. The extension is marked as reproducible if supported by version of Bazel and thus doesn't result in a lockfile entry. + + +## modules.repo_name + +
+modules.repo_name(label_or_name)
+
+ +Utility to provide Label compatibility with Bazel 5. + +Under Bazel 5, calls `Label.workspace_name`. Otherwise calls +`Label.repo_name`. + + +**PARAMETERS** + + +| Name | Description | Default Value | +| :------------- | :------------- | :------------- | +| label_or_name | a Label or repository name string | none | + +**RETURNS** + +The repository name returned directly from the Label API, or the + original string if not a Label + + ## modules.use_all_repos diff --git a/lib/modules.bzl b/lib/modules.bzl index 76c1e420..6820e457 100644 --- a/lib/modules.bzl +++ b/lib/modules.bzl @@ -114,6 +114,24 @@ def _use_all_repos(module_ctx, reproducible = False): **extension_metadata_kwargs ) +def _repo_name(label_or_name): + """Utility to provide Label compatibility with Bazel 5. + + Under Bazel 5, calls `Label.workspace_name`. Otherwise calls + `Label.repo_name`. + + Args: + label_or_name: a Label or repository name string + + Returns: + The repository name returned directly from the Label API, or the + original string if not a Label + """ + if hasattr(label_or_name, "repo_name"): + return label_or_name.repo_name + + return getattr(label_or_name, "workspace_name", label_or_name) + def _apparent_repo_name(label_or_name): """Return a repository's apparent repository name. @@ -123,7 +141,7 @@ def _apparent_repo_name(label_or_name): Returns: The apparent repository name """ - repo_name = getattr(label_or_name, "repo_name", label_or_name).lstrip("@") + repo_name = _repo_name(label_or_name).lstrip("@") delimiter_indices = [] # Bazed on this pattern from the Bazel source: @@ -153,13 +171,14 @@ def _apparent_repo_label_string(label): str(label) with its canonical repository name replaced with its apparent repository name """ - if len(label.repo_name) == 0: + repo_name = _repo_name(label) + if len(repo_name) == 0: return str(label) label_str = "@" + str(label).lstrip("@") - return label_str.replace(label.repo_name, _apparent_repo_name(label)) + return label_str.replace(repo_name, _apparent_repo_name(label)) -_main_repo_prefix = str(Label("@@//:all")).split(":")[0] +_main_repo_prefix = str(Label("//:all")).split(":")[0] def _adjust_main_repo_prefix(target_pattern): """Updates the main repository prefix to match the current Bazel version. @@ -184,6 +203,7 @@ def _adjust_main_repo_prefix(target_pattern): modules = struct( as_extension = _as_extension, use_all_repos = _use_all_repos, + repo_name = _repo_name, apparent_repo_name = _apparent_repo_name, apparent_repo_label_string = _apparent_repo_label_string, main_repo_prefix = _main_repo_prefix, diff --git a/tests/modules_test.bzl b/tests/modules_test.bzl index d0c8b1e0..d2aef52d 100644 --- a/tests/modules_test.bzl +++ b/tests/modules_test.bzl @@ -78,14 +78,14 @@ def _apparent_repo_name_test(ctx): asserts.equals( env, "foo", - modules.apparent_repo_name(Label("@foo").repo_name), + modules.apparent_repo_name(modules.repo_name(Label("@foo"))), msg = "Return the apparent name from a canonical name string.", ) asserts.equals( env, "", - modules.apparent_repo_name(Label("@@//:all")), + modules.apparent_repo_name(Label("//:all")), msg = "Returns the empty string for a main repository Label.", ) From cffa08e4cc6199a89b5b0b42502d44f2c355087d Mon Sep 17 00:00:00 2001 From: Mike Bland Date: Mon, 14 Oct 2024 20:47:34 -0400 Subject: [PATCH 4/8] Fix main_repo_prefix, add is_bzlmod_enabled After thinking it through, I realized `Label(//:all)` would not produce the main repo prefix when imported into other repos. I also realized taking a variation of the `_is_bzlmod_enabled` test from `tests/modules_test.bzl` would make the correct implementation easier. So I made `is_bzlmod_enabled` part of the `modules` exports, and used it internally to set `_main_repo_prefix`. All the tests continue to pass. --- lib/modules.bzl | 5 ++++- tests/modules_test.bzl | 10 +++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/modules.bzl b/lib/modules.bzl index 6820e457..9516f3b9 100644 --- a/lib/modules.bzl +++ b/lib/modules.bzl @@ -178,7 +178,9 @@ def _apparent_repo_label_string(label): label_str = "@" + str(label).lstrip("@") return label_str.replace(repo_name, _apparent_repo_name(label)) -_main_repo_prefix = str(Label("//:all")).split(":")[0] +_is_bzlmod_enabled = str(Label("//:all")).startswith("@@") + +_main_repo_prefix = "@@//" if _is_bzlmod_enabled else "@//" def _adjust_main_repo_prefix(target_pattern): """Updates the main repository prefix to match the current Bazel version. @@ -206,6 +208,7 @@ modules = struct( repo_name = _repo_name, apparent_repo_name = _apparent_repo_name, apparent_repo_label_string = _apparent_repo_label_string, + is_bzlmod_enabled = _is_bzlmod_enabled, main_repo_prefix = _main_repo_prefix, adjust_main_repo_prefix = _adjust_main_repo_prefix, ) diff --git a/tests/modules_test.bzl b/tests/modules_test.bzl index d2aef52d..ada83aa7 100644 --- a/tests/modules_test.bzl +++ b/tests/modules_test.bzl @@ -18,8 +18,6 @@ load("//lib:modules.bzl", "modules") load("//lib:unittest.bzl", "asserts", "unittest") load("//rules:build_test.bzl", "build_test") -_is_bzlmod_enabled = str(Label("//tests:module_tests.bzl")).startswith("@@") - def _repo_rule_impl(repository_ctx): repository_ctx.file("WORKSPACE") repository_ctx.file("BUILD", """exports_files(["hello"])""") @@ -119,7 +117,7 @@ def _apparent_repo_name_test(ctx): asserts.equals( env, - "stardoc" if _is_bzlmod_enabled else "io_bazel_stardoc", + "stardoc" if modules.is_bzlmod_enabled else "io_bazel_stardoc", modules.apparent_repo_name(Label("@io_bazel_stardoc")), msg = " ".join([ "Label values will already map bazel_dep repo_names to", @@ -171,7 +169,9 @@ def _apparent_repo_label_string_test(ctx): asserts.equals( env, - "@%s//:all" % ("stardoc" if _is_bzlmod_enabled else "io_bazel_stardoc"), + "@%s//:all" % ( + "stardoc" if modules.is_bzlmod_enabled else "io_bazel_stardoc" + ), modules.apparent_repo_label_string(Label("@io_bazel_stardoc//:all")), msg = " ".join([ "Returns the actual module name instead of", @@ -229,7 +229,7 @@ def modules_test_suite(): adjust_main_repo_prefix_test, ) - if not _is_bzlmod_enabled: + if not modules.is_bzlmod_enabled: return build_test( From c8d0f67ba1e362708e02844833149e41dcdea211 Mon Sep 17 00:00:00 2001 From: Mike Bland Date: Tue, 15 Oct 2024 11:14:18 -0400 Subject: [PATCH 5/8] Withdraw modules functions bazed on #548 Based @fmeum's review comments on #548, removed the following functions from `lib/modules.bzl`: - `adjust_main_repo_prefix`: prefer solutions based on `attr.label_list`, `Label`, and `native.package_relative_label` to pass `Label` values in the code - `is_bzlmod_enabled`: available in `@bazel_features`, only added because of `repo_name` - `repo_name`: replaced with a `getattr` + `hasattr` expression, since Bazel 5 is near end of life and it's easier to search and replace this expression --- docs/modules_doc.md | 56 ------------------------------------------ lib/modules.bzl | 52 ++++----------------------------------- tests/modules_test.bzl | 49 +++++++++--------------------------- 3 files changed, 16 insertions(+), 141 deletions(-) diff --git a/docs/modules_doc.md b/docs/modules_doc.md index 3d3b013a..9cf61073 100755 --- a/docs/modules_doc.md +++ b/docs/modules_doc.md @@ -2,35 +2,6 @@ Skylib module containing utilities for Bazel modules and module extensions. - - -## modules.adjust_main_repo_prefix - -
-modules.adjust_main_repo_prefix(target_pattern)
-
- -Updates the main repository prefix to match the current Bazel version. - -The main repo prefix will be "@//" for Bazel < 7.1.0, and "@@//" for Bazel ->= 7.1.0 under Bzlmod. This macro automatically updates strings representing -include/exclude target patterns so that they match actual main repository -target Labels correctly. - - -**PARAMETERS** - - -| Name | Description | Default Value | -| :------------- | :------------- | :------------- | -| target_pattern | a string used to match a BUILD target pattern | none | - -**RETURNS** - -the string with any main repository prefix updated to match the current - Bazel version - - ## modules.apparent_repo_label_string @@ -112,33 +83,6 @@ imported via `use_repo`. The extension is marked as reproducible if supported by version of Bazel and thus doesn't result in a lockfile entry. - - -## modules.repo_name - -
-modules.repo_name(label_or_name)
-
- -Utility to provide Label compatibility with Bazel 5. - -Under Bazel 5, calls `Label.workspace_name`. Otherwise calls -`Label.repo_name`. - - -**PARAMETERS** - - -| Name | Description | Default Value | -| :------------- | :------------- | :------------- | -| label_or_name | a Label or repository name string | none | - -**RETURNS** - -The repository name returned directly from the Label API, or the - original string if not a Label - - ## modules.use_all_repos diff --git a/lib/modules.bzl b/lib/modules.bzl index 9516f3b9..f8b8f146 100644 --- a/lib/modules.bzl +++ b/lib/modules.bzl @@ -114,23 +114,9 @@ def _use_all_repos(module_ctx, reproducible = False): **extension_metadata_kwargs ) -def _repo_name(label_or_name): - """Utility to provide Label compatibility with Bazel 5. - - Under Bazel 5, calls `Label.workspace_name`. Otherwise calls - `Label.repo_name`. - - Args: - label_or_name: a Label or repository name string - - Returns: - The repository name returned directly from the Label API, or the - original string if not a Label - """ - if hasattr(label_or_name, "repo_name"): - return label_or_name.repo_name - - return getattr(label_or_name, "workspace_name", label_or_name) +_repo_attr = ( + "repo_name" if hasattr(Label("//:all"), "repo_name") else "workspace_name" +) def _apparent_repo_name(label_or_name): """Return a repository's apparent repository name. @@ -141,7 +127,7 @@ def _apparent_repo_name(label_or_name): Returns: The apparent repository name """ - repo_name = _repo_name(label_or_name).lstrip("@") + repo_name = getattr(label_or_name, _repo_attr, label_or_name).lstrip("@") delimiter_indices = [] # Bazed on this pattern from the Bazel source: @@ -171,44 +157,16 @@ def _apparent_repo_label_string(label): str(label) with its canonical repository name replaced with its apparent repository name """ - repo_name = _repo_name(label) + repo_name = getattr(label, _repo_attr).lstrip("@") if len(repo_name) == 0: return str(label) label_str = "@" + str(label).lstrip("@") return label_str.replace(repo_name, _apparent_repo_name(label)) -_is_bzlmod_enabled = str(Label("//:all")).startswith("@@") - -_main_repo_prefix = "@@//" if _is_bzlmod_enabled else "@//" - -def _adjust_main_repo_prefix(target_pattern): - """Updates the main repository prefix to match the current Bazel version. - - The main repo prefix will be "@//" for Bazel < 7.1.0, and "@@//" for Bazel - >= 7.1.0 under Bzlmod. This macro automatically updates strings representing - include/exclude target patterns so that they match actual main repository - target Labels correctly. - - Args: - target_pattern: a string used to match a BUILD target pattern - - Returns: - the string with any main repository prefix updated to match the current - Bazel version - """ - if target_pattern.startswith("@//") or target_pattern.startswith("@@//"): - return _main_repo_prefix + target_pattern.lstrip("@/") - - return target_pattern - modules = struct( as_extension = _as_extension, use_all_repos = _use_all_repos, - repo_name = _repo_name, apparent_repo_name = _apparent_repo_name, apparent_repo_label_string = _apparent_repo_label_string, - is_bzlmod_enabled = _is_bzlmod_enabled, - main_repo_prefix = _main_repo_prefix, - adjust_main_repo_prefix = _adjust_main_repo_prefix, ) diff --git a/tests/modules_test.bzl b/tests/modules_test.bzl index ada83aa7..193aa06f 100644 --- a/tests/modules_test.bzl +++ b/tests/modules_test.bzl @@ -18,6 +18,8 @@ load("//lib:modules.bzl", "modules") load("//lib:unittest.bzl", "asserts", "unittest") load("//rules:build_test.bzl", "build_test") +_is_bzlmod_enabled = str(Label("//tests:module_tests.bzl")).startswith("@@") + def _repo_rule_impl(repository_ctx): repository_ctx.file("WORKSPACE") repository_ctx.file("BUILD", """exports_files(["hello"])""") @@ -73,10 +75,15 @@ def _apparent_repo_name_test(ctx): msg = "Return the original name without `@` if already apparent.", ) + foo_label = Label("@foo") + foo_canonical_name = getattr( + foo_label, + "repo_name" if hasattr(foo_label, "repo_name") else "workspace_name", + ) asserts.equals( env, "foo", - modules.apparent_repo_name(modules.repo_name(Label("@foo"))), + modules.apparent_repo_name(foo_canonical_name), msg = "Return the apparent name from a canonical name string.", ) @@ -117,7 +124,7 @@ def _apparent_repo_name_test(ctx): asserts.equals( env, - "stardoc" if modules.is_bzlmod_enabled else "io_bazel_stardoc", + "stardoc" if _is_bzlmod_enabled else "io_bazel_stardoc", modules.apparent_repo_name(Label("@io_bazel_stardoc")), msg = " ".join([ "Label values will already map bazel_dep repo_names to", @@ -170,7 +177,7 @@ def _apparent_repo_label_string_test(ctx): asserts.equals( env, "@%s//:all" % ( - "stardoc" if modules.is_bzlmod_enabled else "io_bazel_stardoc" + "stardoc" if _is_bzlmod_enabled else "io_bazel_stardoc" ), modules.apparent_repo_label_string(Label("@io_bazel_stardoc//:all")), msg = " ".join([ @@ -185,39 +192,6 @@ apparent_repo_label_string_test = unittest.make( _apparent_repo_label_string_test, ) -def _adjust_main_repo_prefix_test(ctx): - """Unit tests for modules.apparent_repo_label_string.""" - env = unittest.begin(ctx) - - expected = modules.main_repo_prefix + ":all" - asserts.equals( - env, - expected, - modules.adjust_main_repo_prefix("@//:all"), - msg = "Normalizes a target pattern starting with `@//`.", - ) - - asserts.equals( - env, - expected, - modules.adjust_main_repo_prefix("@@//:all"), - msg = "Normalizes a target pattern starting with `@@//`.", - ) - - original = "@not_the_main_repo" - asserts.equals( - env, - original, - modules.adjust_main_repo_prefix(original), - msg = "Returns non main repo target patterns unchanged.", - ) - - return unittest.end(env) - -adjust_main_repo_prefix_test = unittest.make( - _adjust_main_repo_prefix_test, -) - # buildifier: disable=unnamed-macro def modules_test_suite(): """Creates the tests for modules.bzl if Bzlmod is enabled.""" @@ -226,10 +200,9 @@ def modules_test_suite(): "modules_tests", apparent_repo_name_test, apparent_repo_label_string_test, - adjust_main_repo_prefix_test, ) - if not modules.is_bzlmod_enabled: + if not _is_bzlmod_enabled: return build_test( From b5f6722ac69ec30f7fbaaa08126d3b4b0e92cf57 Mon Sep 17 00:00:00 2001 From: Mike Bland Date: Tue, 15 Oct 2024 12:18:58 -0400 Subject: [PATCH 6/8] Remove `modules.apparent_repo_label_string` After removing `modules.adjust_main_repo_prefix` in the previous commit, there's no need for `modules.apparent_repo_label_string`. In other words, if the solution to the problem is to replace `string` inputs with `Label`s, there's no need for a special `Label` stringifier. --- docs/modules_doc.md | 23 ------------------- lib/modules.bzl | 18 --------------- tests/modules_test.bzl | 50 ------------------------------------------ 3 files changed, 91 deletions(-) diff --git a/docs/modules_doc.md b/docs/modules_doc.md index 9cf61073..6a995706 100755 --- a/docs/modules_doc.md +++ b/docs/modules_doc.md @@ -2,29 +2,6 @@ Skylib module containing utilities for Bazel modules and module extensions. - - -## modules.apparent_repo_label_string - -
-modules.apparent_repo_label_string(label)
-
- -Return a Label string starting with its apparent repo name. - -**PARAMETERS** - - -| Name | Description | Default Value | -| :------------- | :------------- | :------------- | -| label | a Label instance | none | - -**RETURNS** - -str(label) with its canonical repository name replaced with its apparent - repository name - - ## modules.apparent_repo_name diff --git a/lib/modules.bzl b/lib/modules.bzl index f8b8f146..3ae708bb 100644 --- a/lib/modules.bzl +++ b/lib/modules.bzl @@ -147,26 +147,8 @@ def _apparent_repo_name(label_or_name): return repo_name[delimiter_indices[-1] + 1:] -def _apparent_repo_label_string(label): - """Return a Label string starting with its apparent repo name. - - Args: - label: a Label instance - - Returns: - str(label) with its canonical repository name replaced with its apparent - repository name - """ - repo_name = getattr(label, _repo_attr).lstrip("@") - if len(repo_name) == 0: - return str(label) - - label_str = "@" + str(label).lstrip("@") - return label_str.replace(repo_name, _apparent_repo_name(label)) - modules = struct( as_extension = _as_extension, use_all_repos = _use_all_repos, apparent_repo_name = _apparent_repo_name, - apparent_repo_label_string = _apparent_repo_label_string, ) diff --git a/tests/modules_test.bzl b/tests/modules_test.bzl index 193aa06f..a9ba0797 100644 --- a/tests/modules_test.bzl +++ b/tests/modules_test.bzl @@ -143,55 +143,6 @@ def _apparent_repo_name_test(ctx): apparent_repo_name_test = unittest.make(_apparent_repo_name_test) -def _apparent_repo_label_string_test(ctx): - """Unit tests for modules.apparent_repo_label_string.""" - env = unittest.begin(ctx) - - main_repo = str(Label("//:all")) - asserts.equals( - env, - main_repo, - modules.apparent_repo_label_string(Label("//:all")), - msg = "Returns top level target with leading `@` or `@@`", - ) - - main_module_label = Label("@bazel_skylib//:all") - asserts.equals( - env, - main_repo, - modules.apparent_repo_label_string(main_module_label), - msg = " ".join([ - "Returns top level target with leading `@` or `@@`", - "for a Label containing the main module's name", - ]), - ) - - rules_pkg = "@rules_pkg//:all" - asserts.equals( - env, - rules_pkg, - modules.apparent_repo_label_string(Label(rules_pkg)), - msg = "Returns original repo name", - ) - - asserts.equals( - env, - "@%s//:all" % ( - "stardoc" if _is_bzlmod_enabled else "io_bazel_stardoc" - ), - modules.apparent_repo_label_string(Label("@io_bazel_stardoc//:all")), - msg = " ".join([ - "Returns the actual module name instead of", - "repo_name from bazel_dep() (no-op under WORKSPACE).", - ]), - ) - - return unittest.end(env) - -apparent_repo_label_string_test = unittest.make( - _apparent_repo_label_string_test, -) - # buildifier: disable=unnamed-macro def modules_test_suite(): """Creates the tests for modules.bzl if Bzlmod is enabled.""" @@ -199,7 +150,6 @@ def modules_test_suite(): unittest.suite( "modules_tests", apparent_repo_name_test, - apparent_repo_label_string_test, ) if not _is_bzlmod_enabled: From cc378f01caf62ce7134b70b8d7d8e976ab8ea9b0 Mon Sep 17 00:00:00 2001 From: Mike Bland Date: Wed, 16 Oct 2024 13:08:55 -0400 Subject: [PATCH 7/8] Use `apparent_repo_name` only for `repository_ctx` Repurposed `apparent_repo_name` to only work with `repository_ctx` objects after finding solutions to other repository name related problems in bazelbuild/rules_scala#1621. The one problem I couldn't solve (elegantly) without this function was setting a default target name for a generated repo. Technically, such repos could require an attribute to mirror `name`, but that seems like a terrible user experience. --- MODULE.bazel | 3 + WORKSPACE | 4 ++ docs/modules_doc.md | 28 ++++++-- lib/modules.bzl | 46 +++++++------ tests/extensions/apparent_repo_name.bzl | 22 +++++++ tests/modules_test.bzl | 86 ++----------------------- 6 files changed, 84 insertions(+), 105 deletions(-) create mode 100644 tests/extensions/apparent_repo_name.bzl diff --git a/MODULE.bazel b/MODULE.bazel index b53443c6..e77ad3c2 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -37,3 +37,6 @@ use_repo(as_extension_test_ext, "bar", "foo") use_all_repos_test_ext = use_extension("//tests:modules_test.bzl", "use_all_repos_test_ext", dev_dependency = True) use_repo(use_all_repos_test_ext, "baz", "qux") + +apparent_repo_name_test_ext = use_extension("//tests:extensions/apparent_repo_name.bzl", "apparent_repo_name_test_ext", dev_dependency = True) +use_repo(apparent_repo_name_test_ext, "apparent-repo-name-test") diff --git a/WORKSPACE b/WORKSPACE index 3a4d21f5..9f3a1000 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -81,3 +81,7 @@ load("//tests/directory:external_directory_tests.bzl", "external_directory_tests external_directory_tests(name = "external_directory_tests") register_unittest_toolchains() + +load("//tests:extensions/apparent_repo_name.bzl", "apparent_repo_name_test_macro") + +apparent_repo_name_test_macro() diff --git a/docs/modules_doc.md b/docs/modules_doc.md index 6a995706..17e3088b 100755 --- a/docs/modules_doc.md +++ b/docs/modules_doc.md @@ -7,21 +7,41 @@ Skylib module containing utilities for Bazel modules and module extensions. ## modules.apparent_repo_name 
-modules.apparent_repo_name(label_or_name)
+modules.apparent_repo_name(repository_ctx)
-Return a repository's apparent repository name. +Generates a repository's apparent name from a repository_ctx object. + +Useful when generating the default top level `BUILD` target for the +repository. + +Example: +```starlark +_ALIAS_TARGET_TEMPLATE = """alias( + name = "{name}", + actual = "@{target_repo_name}", + visibility = ["//visibility:public"], +) +""" + +def _alias_repository_impl(repository_ctx): + repository_ctx.file("BUILD", _ALIAS_TARGET_TEMPLATE.format( + name = apparent_repo_name(rctx), + target = rctx.attr.target_repo_name, + )) +``` + **PARAMETERS** | Name | Description | Default Value | | :------------- | :------------- | :------------- | -| label_or_name | a Label or repository name string | none | +| repository_ctx | a repository_ctx object | none | **RETURNS** -The apparent repository name +An apparent repo name derived from repository_ctx.name diff --git a/lib/modules.bzl b/lib/modules.bzl index 3ae708bb..347e7db1 100644 --- a/lib/modules.bzl +++ b/lib/modules.bzl @@ -114,38 +114,44 @@ def _use_all_repos(module_ctx, reproducible = False): **extension_metadata_kwargs ) -_repo_attr = ( - "repo_name" if hasattr(Label("//:all"), "repo_name") else "workspace_name" -) +def _apparent_repo_name(repository_ctx): + """Generates a repository's apparent name from a repository_ctx object. + + Useful when generating the default top level `BUILD` target for the + repository. -def _apparent_repo_name(label_or_name): - """Return a repository's apparent repository name. + Example: + ```starlark + _ALIAS_TARGET_TEMPLATE = \"\"\"alias( + name = "{name}", + actual = "@{target_repo_name}", + visibility = ["//visibility:public"], + ) + \"\"\" + + def _alias_repository_impl(repository_ctx): + repository_ctx.file("BUILD", _ALIAS_TARGET_TEMPLATE.format( + name = apparent_repo_name(rctx), + target = rctx.attr.target_repo_name, + )) + ``` Args: - label_or_name: a Label or repository name string + repository_ctx: a repository_ctx object Returns: - The apparent repository name + An apparent repo name derived from repository_ctx.name """ - repo_name = getattr(label_or_name, _repo_attr, label_or_name).lstrip("@") - delimiter_indices = [] + repo_name = repository_ctx.name # Bazed on this pattern from the Bazel source: # com.google.devtools.build.lib.cmdline.RepositoryName.VALID_REPO_NAME - for i in range(len(repo_name)): + for i in range(len(repo_name) - 1, -1, -1): c = repo_name[i] if not (c.isalnum() or c in "_-."): - delimiter_indices.append(i) - - if len(delimiter_indices) == 0: - # Already an apparent repo name, apparently. - return repo_name - - if len(delimiter_indices) == 1: - # The name is for a top level module, possibly containing a version ID. - return repo_name[:delimiter_indices[0]] + return repo_name[i + 1:] - return repo_name[delimiter_indices[-1] + 1:] + return repo_name modules = struct( as_extension = _as_extension, diff --git a/tests/extensions/apparent_repo_name.bzl b/tests/extensions/apparent_repo_name.bzl new file mode 100644 index 00000000..9370fb29 --- /dev/null +++ b/tests/extensions/apparent_repo_name.bzl @@ -0,0 +1,22 @@ +"""Repo rule and module extension used to test modules.apparent_repo_name""" + +load("//lib:modules.bzl", "modules") + +def _apparent_repo_name_test_repo_impl(repository_ctx): + repo_name = modules.apparent_repo_name(repository_ctx) + test_file = "repo-name.bzl" + repository_ctx.file("WORKSPACE") + repository_ctx.file("BUILD", """exports_files(["%s"])""" % test_file) + repository_ctx.file(test_file, "REPO_NAME = \"%s\"" % repo_name) + +apparent_repo_name_test_repo = repository_rule( + _apparent_repo_name_test_repo_impl, +) + +def apparent_repo_name_test_macro(*args): + apparent_repo_name_test_repo(name = "apparent-repo-name-test") + +apparent_repo_name_test_ext = module_extension( + lambda _: apparent_repo_name_test_macro(), + doc = "Only used for testing modules.apparent_repo_name()", +) diff --git a/tests/modules_test.bzl b/tests/modules_test.bzl index a9ba0797..7e4b8657 100644 --- a/tests/modules_test.bzl +++ b/tests/modules_test.bzl @@ -14,6 +14,7 @@ """Test usage of modules.bzl.""" +load("@apparent-repo-name-test//:repo-name.bzl", "REPO_NAME") load("//lib:modules.bzl", "modules") load("//lib:unittest.bzl", "asserts", "unittest") load("//rules:build_test.bzl", "build_test") @@ -54,91 +55,14 @@ def _apparent_repo_name_test(ctx): asserts.equals( env, - "", - modules.apparent_repo_name(""), - msg = "Handles the empty string as input", - ) - - asserts.equals( - env, - "foo", - modules.apparent_repo_name("foo"), - msg = ( - "Return the original name unchanged if it doesn't start with `@`.", - ), - ) - - asserts.equals( - env, - "foo", - modules.apparent_repo_name("@foo"), - msg = "Return the original name without `@` if already apparent.", - ) - - foo_label = Label("@foo") - foo_canonical_name = getattr( - foo_label, - "repo_name" if hasattr(foo_label, "repo_name") else "workspace_name", - ) - asserts.equals( - env, - "foo", - modules.apparent_repo_name(foo_canonical_name), - msg = "Return the apparent name from a canonical name string.", - ) - - asserts.equals( - env, - "", - modules.apparent_repo_name(Label("//:all")), - msg = "Returns the empty string for a main repository Label.", - ) - - asserts.equals( - env, - "", - modules.apparent_repo_name(Label("@bazel_skylib//:all")), + "apparent-repo-name-test", + REPO_NAME, msg = " ".join([ - "Returns the empty string for a Label containing the main", - "repository's module name.", + "Returns the original name unchanged under WORKSPACE, and", + "the apparent repo name under Bzlmod.", ]), ) - asserts.equals( - env, - "foo", - modules.apparent_repo_name(Label("@foo")), - msg = "Return the apparent name from a Label.", - ) - - asserts.equals( - env, - "rules_pkg", - modules.apparent_repo_name(Label("@rules_pkg")), - msg = " ".join([ - "Top level module repos have the canonical name delimiter at the", - "end. Therefore, this should not return the empty string, but the", - "name without the leading `@` and trailing delimiter.", - ]), - ) - - asserts.equals( - env, - "stardoc" if _is_bzlmod_enabled else "io_bazel_stardoc", - modules.apparent_repo_name(Label("@io_bazel_stardoc")), - msg = " ".join([ - "Label values will already map bazel_dep repo_names to", - "actual repo names under Bzlmod (no-op under WORKSPACE).", - ]), - ) - - asserts.equals( - env, - "foo", - modules.apparent_repo_name("foo+1.2.3"), - msg = "Ignores version numbers in canonical repo names", - ) - return unittest.end(env) apparent_repo_name_test = unittest.make(_apparent_repo_name_test) From 73da0473203035c493f7ae4a8c645f7817383c3e Mon Sep 17 00:00:00 2001 From: Mike Bland Date: Wed, 16 Oct 2024 13:15:51 -0400 Subject: [PATCH 8/8] Fix lint error in `apparent_repo_name_test_macro` --- tests/extensions/apparent_repo_name.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/extensions/apparent_repo_name.bzl b/tests/extensions/apparent_repo_name.bzl index 9370fb29..cdf883e8 100644 --- a/tests/extensions/apparent_repo_name.bzl +++ b/tests/extensions/apparent_repo_name.bzl @@ -13,7 +13,7 @@ apparent_repo_name_test_repo = repository_rule( _apparent_repo_name_test_repo_impl, ) -def apparent_repo_name_test_macro(*args): +def apparent_repo_name_test_macro(): apparent_repo_name_test_repo(name = "apparent-repo-name-test") apparent_repo_name_test_ext = module_extension(