From d35448f943e3cefb7e1c7ba2da7982f846bc35f5 Mon Sep 17 00:00:00 2001 From: Mike Bland Date: Wed, 27 Nov 2024 09:04:24 -0500 Subject: [PATCH] Replace apparent_repo_name with repo rule wrappers (#1650) This presents the same API, remains compatible with both `WORKSPACE` and Bzlmod, and avoids relying on the canonical repository name format at all. Part of #1482, and supersedes the addition of `apparent_repo_name` from #1621. Also adds the `_SCALA_IMPORT_RULE_LOAD` constant in `scala/scala_maven_import_external.bzl`, which also removes `@io_bazel_rules_scala` from the `load`ed file path. It now generates the correct path with a `Label` instead. --- The goal is to maintain the ability to generate default target names from a repository name under Bzlmod. While not documented on https://bazel.build, it is documented in the Bazel source itself, even under the current 9.0.0-pre.20241105.2 prerelease: - https://github.com/bazelbuild/bazel/blob/9.0.0-pre.20241105.2/src/main/java/com/google/devtools/build/lib/cmdline/LabelParser.java#L99 Under `WORKSPACE`, the `repository_ctx.name` value served this purpose directly, as it would reflect the `name` value as provided by the caller. Under Bzlmod, this value now contains the canonical repo name, which is different than that provided by the `name` parameter. The `apparent_repo_name` utility was my first attempt to resolve this, by parsing the original `name` parameter from the canonicalized `repository_ctx.name`. I also created bazelbuild/bazel-skylib#548 to contribute `apparent_repo_name` to `bazel_skylib`, along with other proposed utilities. I quickly found better solutions to obviate the other utilities, but hung onto `apparent_repo_name`. After that pull request stagnated, I eventually realized a macro wrapper could generate a default target name for a repository_rule by duplicating the `name` attr. This isn't as easy as adding `apparent_repo_name(repository_ctx.name)` to a repo rule's implementation, but it's also not that much more work. See also this thread from the #bzlmod channel in the Bazel Slack workspace about generating default repo names: - https://bazelbuild.slack.com/archives/C014RARENH0/p1730909824656159 --- One small downside of this technique is that the macro can't be imported into a `MODULE.bazel` file directly via `use_repo_rule`. If you did want to use it in `MODULE.bazel`, you'd have to write a module extension to call it, or use `modules.as_extension` from `bazel_skylib`. I suppose that's a small price to pay for squashing a canonical repo name format dependency forever. The other small downside may be that the documentation from the original `repository_rule` doesn't automatically convey to the repo wrapper. In this case, `_alias_repository` is already internal, and the original `jvm_import_external` didn't have docstrings to begin with. --- scala/private/macros/bzlmod.bzl | 21 ------------------ scala/scala_maven_import_external.bzl | 27 ++++++++++++++++------- third_party/repositories/repositories.bzl | 11 ++++++--- 3 files changed, 27 insertions(+), 32 deletions(-) delete mode 100644 scala/private/macros/bzlmod.bzl diff --git a/scala/private/macros/bzlmod.bzl b/scala/private/macros/bzlmod.bzl deleted file mode 100644 index db8e8385f..000000000 --- a/scala/private/macros/bzlmod.bzl +++ /dev/null @@ -1,21 +0,0 @@ -"""Utilities for working with Bazel modules""" - -def apparent_repo_name(repository_ctx): - """Generates a repository's apparent name from a repository_ctx object. - - Args: - repository_ctx: a repository_ctx object - - Returns: - An apparent repo name derived from repository_ctx.name - """ - repo_name = repository_ctx.name - - # Based 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/scala_maven_import_external.bzl b/scala/scala_maven_import_external.bzl index 1a7bf8145..4acfbd195 100644 --- a/scala/scala_maven_import_external.bzl +++ b/scala/scala_maven_import_external.bzl @@ -35,9 +35,12 @@ 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") +_SCALA_IMPORT_RULE_LOAD = ( + "load(\"%s\", \"scala_import\")" % Label("//scala:scala_import.bzl") +) + # https://github.com/bazelbuild/bazel/issues/13709#issuecomment-1336699672 def _get_auth(ctx, urls): """Given the list of URLs obtain the correct auth dict.""" @@ -60,12 +63,12 @@ _PASS_PROPS = ( _FETCH_SOURCES_ENV_VAR_NAME = "BAZEL_JVM_FETCH_SOURCES" -def _jvm_import_external(repository_ctx): +def _jvm_import_external_impl(repository_ctx): """Implementation of `java_import_external` rule.""" if (repository_ctx.attr.generated_linkable_rule_name and not repository_ctx.attr.neverlink): fail("Only use generated_linkable_rule_name if neverlink is set") - repo_name = apparent_repo_name(repository_ctx) + repo_name = repository_ctx.name name = repository_ctx.attr.generated_rule_name or repo_name urls = repository_ctx.attr.jar_urls if repository_ctx.attr.jar_sha256: @@ -138,7 +141,7 @@ def _jvm_import_external(repository_ctx): "", "alias(", " name = \"jar\",", - " actual = \"//:%s\"," % repo_name, + " actual = \"//:%s\"," % name, ")", "", ])) @@ -224,8 +227,8 @@ def _serialize_given_rule_import( lines.append("") return lines -jvm_import_external = repository_rule( - implementation = _jvm_import_external, +_jvm_import_external = repository_rule( + implementation = _jvm_import_external_impl, attrs = { "rule_name": attr.string(mandatory = True), "licenses": attr.string_list(mandatory = True, allow_empty = False), @@ -254,10 +257,18 @@ jvm_import_external = repository_rule( environ = [_FETCH_SOURCES_ENV_VAR_NAME], ) +def jvm_import_external(**kwargs): + """Wraps `_jvm_import_external` to pass `name` as `generated_target_name`. + + If `generated_rule_name` is specified already, this is a noop. + """ + generated_rule_name = kwargs.pop("generated_rule_name", kwargs.get("name")) + _jvm_import_external(generated_rule_name = generated_rule_name, **kwargs) + def scala_maven_import_external( artifact, server_urls, - rule_load = "load(\"@io_bazel_rules_scala//scala:scala_import.bzl\", \"scala_import\")", + rule_load = _SCALA_IMPORT_RULE_LOAD, fetch_sources = False, **kwargs): jvm_maven_import_external( @@ -299,7 +310,7 @@ def jvm_maven_import_external( jvm_import_external(jar_urls = jar_urls, srcjar_urls = srcjar_urls, coordinates = artifact, **kwargs) def scala_import_external( - rule_load = "load(\"@io_bazel_rules_scala//scala:scala_import.bzl\", \"scala_import\")", + rule_load = _SCALA_IMPORT_RULE_LOAD, **kwargs): jvm_import_external( rule_name = "scala_import", diff --git a/third_party/repositories/repositories.bzl b/third_party/repositories/repositories.bzl index 1e0529c1e..6af90077e 100644 --- a/third_party/repositories/repositories.bzl +++ b/third_party/repositories/repositories.bzl @@ -1,4 +1,3 @@ -load("//scala/private:macros/bzlmod.bzl", "apparent_repo_name") load( "//third_party/repositories:scala_2_11.bzl", _artifacts_2_11 = "artifacts", @@ -129,13 +128,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 = artifact_repo_name) + _alias_repository_wrapper(name = id, target = artifact_repo_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": apparent_repo_name(rctx), + "name": rctx.attr.default_target_name, "target": rctx.attr.target, } rctx.file("BUILD", """alias( @@ -154,6 +153,12 @@ def _alias_repository_impl(rctx): _alias_repository = repository_rule( implementation = _alias_repository_impl, attrs = { + "default_target_name": attr.string(mandatory = True), "target": attr.string(mandatory = True), }, ) + +def _alias_repository_wrapper(**kwargs): + """Wraps `_alias_repository` to pass `name` as `default_target_name`.""" + default_target_name = kwargs.pop("default_target_name", kwargs.get("name")) + _alias_repository(default_target_name = default_target_name, **kwargs)