Skip to content

Commit

Permalink
Replace apparent_repo_name with repo rule wrappers (bazelbuild#1650)
Browse files Browse the repository at this point in the history
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 bazelbuild#1482, and supersedes the addition of `apparent_repo_name`
from bazelbuild#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.
  • Loading branch information
mbland authored Nov 27, 2024
1 parent 43146ea commit d35448f
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 32 deletions.
21 changes: 0 additions & 21 deletions scala/private/macros/bzlmod.bzl

This file was deleted.

27 changes: 19 additions & 8 deletions scala/scala_maven_import_external.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand All @@ -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:
Expand Down Expand Up @@ -138,7 +141,7 @@ def _jvm_import_external(repository_ctx):
"",
"alias(",
" name = \"jar\",",
" actual = \"//:%s\"," % repo_name,
" actual = \"//:%s\"," % name,
")",
"",
]))
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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",
Expand Down
11 changes: 8 additions & 3 deletions third_party/repositories/repositories.bzl
Original file line number Diff line number Diff line change
@@ -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",
Expand Down Expand Up @@ -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(
Expand All @@ -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)

0 comments on commit d35448f

Please sign in to comment.