From 987c89844ce25e6d65cd94aa7dc71a03f016e6d7 Mon Sep 17 00:00:00 2001 From: Mike Bland Date: Sun, 6 Oct 2024 20:37:52 -0400 Subject: [PATCH] Add apparent_repo_name macro to prepare for Bzlmod MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Part of #1482. Splits the last component off of canonical repo names to produce the expected repo name. Without Bzlmod, it returns the original name. With Bzlmod enabled, it avoids generating output like: scala_import( name = "_main~scala_deps~io_bazel_rules_scala_scala_compiler", jars = ["scala-compiler-2.12.18.jar"], ) resulting in errors like: ``` ERROR: .../_main~_repo_rules~io_bazel_rules_scala/scala/BUILD: no such target '@@_main~scala_deps~io_bazel_rules_scala_scala_compiler//:io_bazel_rules_scala_scala_compiler': target 'io_bazel_rules_scala_scala_compiler' not declared in package '' defined by .../_main~scala_deps~io_bazel_rules_scala_scala_compiler/BUILD and referenced by '@@_main~_repo_rules~io_bazel_rules_scala//scala:default_toolchain_scala_compile_classpath_provider' ``` Also fixes the following error when attaching resources from custom repos to targets under Bzlmod: ```txt $ bazel test //test/src/main/scala/scalarules/test/resources:all 1) Scala library depending on resources from external resource-only jar::allow to load resources(scalarules.test.resources.ScalaLibResourcesFromExternalDepTest) java.lang.NullPointerException at scalarules.test.resources.ScalaLibResourcesFromExternalDepTest.get(ScalaLibResourcesFromExternalDepTest.scala:17) at scalarules.test.resources.ScalaLibResourcesFromExternalDepTest.$anonfun$new$3(ScalaLibResourcesFromExternalDepTest.scala:11) at scalarules.test.resources.ScalaLibResourcesFromExternalDepTest.$anonfun$new$2(ScalaLibResourcesFromExternalDepTest.scala:11) ``` Can be replaced with a future bazel-skylib implementation, if accepted into that repo. --- We can't rely on the specific canonical repository name format: > Repos generated by extensions have canonical names in the form of > `module_repo_canonical_name+extension_name+repo_name`. For extensions > hosted in the root module, the `module_repo_canonical_name` part is > replaced with the string `_main`. Note that the canonical name format is > not an API you should depend on — it's subject to change at any time. > > - https://bazel.build/external/extension#repository_names_and_visibility The change to no longer encode module versions in canonical repo names in Bazel 7.1.0 is a recent example of Bazel maintainers altering the format: - https://github.com/bazelbuild/bazel/pull/21316 And the maintainers recently replaced the `~` delimiter with `+` in the upcoming Bazel 8 release due to build performance issues on Windows: - https://github.com/bazelbuild/bazel/issues/22865 This function assumes the only valid `repo_name` characters are letters, numbers, '_', '-', and '.'. It finds the last character not in this set, and returns the contents of `name` following this character. This is valid so long as this condition holds: - https://github.com/bazelbuild/bazel/blob/7.3.2/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java#L159-L162 --- scala/private/macros/bzlmod.bzl | 23 +++++++++++++++++++++++ scala/private/resources.bzl | 10 +++++++++- scala/scala_maven_import_external.bzl | 3 ++- third_party/repositories/repositories.bzl | 7 +++++-- 4 files changed, 39 insertions(+), 4 deletions(-) create mode 100644 scala/private/macros/bzlmod.bzl diff --git a/scala/private/macros/bzlmod.bzl b/scala/private/macros/bzlmod.bzl new file mode 100644 index 000000000..7527fae28 --- /dev/null +++ b/scala/private/macros/bzlmod.bzl @@ -0,0 +1,23 @@ +"""Utilities for working with Bazel modules""" + +def apparent_repo_name(label_or_name): + """Return a repository's apparent repository name. + + Can be replaced with a future bazel-skylib implementation, if accepted into + that repo. + + 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) + + # 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) - 1, -1, -1): + c = repo_name[i] + if not (c.isalnum() or c in "_-."): + return repo_name[i + 1:] + return repo_name diff --git a/scala/private/resources.bzl b/scala/private/resources.bzl index 1db14a822..086cd5a82 100644 --- a/scala/private/resources.bzl +++ b/scala/private/resources.bzl @@ -1,3 +1,5 @@ +load(":macros/bzlmod.bzl", "apparent_repo_name") + def paths(resources, resource_strip_prefix): """Return a list of path tuples (target, source) where: target - is a path in the archive (with given prefix stripped off) @@ -13,7 +15,13 @@ def paths(resources, resource_strip_prefix): def _target_path(resource, resource_strip_prefix): path = _target_path_by_strip_prefix(resource, resource_strip_prefix) if resource_strip_prefix else _target_path_by_default_prefixes(resource) - return _strip_prefix(path, "/") + return _update_external_target_path(_strip_prefix(path, "/")) + +def _update_external_target_path(target_path): + if not target_path.startswith("external/"): + return target_path + prefix, repo_name, rest = target_path.split("/") + return "/".join([prefix, apparent_repo_name(repo_name), rest]) def _target_path_by_strip_prefix(resource, resource_strip_prefix): # Start from absolute resource path and then strip roots so we get to correct short path diff --git a/scala/scala_maven_import_external.bzl b/scala/scala_maven_import_external.bzl index e69388ced..7e575b406 100644 --- a/scala/scala_maven_import_external.bzl +++ b/scala/scala_maven_import_external.bzl @@ -35,6 +35,7 @@ the following macros are defined below that utilize jvm_import_external: - java_import_external - to demonstrate that the original functionality of `java_import_external` stayed intact. """ +load("//scala/private:macros/bzlmod.bzl", "apparent_repo_name") load("@bazel_tools//tools/build_defs/repo:utils.bzl", "read_netrc", "read_user_netrc", "use_netrc") # https://github.com/bazelbuild/bazel/issues/13709#issuecomment-1336699672 @@ -136,7 +137,7 @@ def _jvm_import_external(repository_ctx): "", "alias(", " name = \"jar\",", - " actual = \"@%s\"," % repository_ctx.name, + " actual = \"@%s\"," % apparent_repo_name(repository_ctx.name), ")", "", ])) diff --git a/third_party/repositories/repositories.bzl b/third_party/repositories/repositories.bzl index c243c8e0b..0e5ce7cae 100644 --- a/third_party/repositories/repositories.bzl +++ b/third_party/repositories/repositories.bzl @@ -1,3 +1,4 @@ +load("//scala/private:macros/bzlmod.bzl", "apparent_repo_name") load( "//third_party/repositories:scala_2_11.bzl", _artifacts_2_11 = "artifacts", @@ -95,8 +96,10 @@ def repositories( default_artifacts = artifacts_by_major_scala_version[major_scala_version] artifacts = dict(default_artifacts.items() + overriden_artifacts.items()) for id in for_artifact_ids: + generated_rule_name = apparent_repo_name(id) + suffix _scala_maven_import_external( name = id + suffix, + generated_rule_name = generated_rule_name, artifact = artifacts[id]["artifact"], artifact_sha256 = artifacts[id]["sha256"], licenses = ["notice"], @@ -111,13 +114,13 @@ def repositories( # See: https://github.com/bazelbuild/rules_scala/pull/1573 # Hopefully we can deprecate and remove it one day. if suffix and scala_version == SCALA_VERSION: - _alias_repository(name = id, target = id + suffix) + _alias_repository(name = id, target = generated_rule_name) def _alias_repository_impl(rctx): """ Builds a repository containing just two aliases to the Scala Maven artifacts in the `target` repository. """ format_kwargs = { - "name": rctx.name, + "name": apparent_repo_name(rctx.name), "target": rctx.attr.target, } rctx.file("BUILD", """alias(