Skip to content

Commit

Permalink
Add repo name macros for Bzlmod compatibility
Browse files Browse the repository at this point in the history
Part of bazelbuild#1482.

These helper macros fix various repository name related errors when
building under Bzlmod, while remaining backwards compatible with
`WORKSPACE`.

Without Bzlmod, these macros return original repo names. With Bzlmod
enabled, they avoid the problems described below.

I've prepared a change for bazelbuild/bazel-skylib containing these
macros with full unit tests. If the maintainers accept that change, we
can bump our bazel_skylib version to use the macros from there, and
remove the `bzlmod.bzl` file.

---

Also includes a couple of other minor touch-ups:

- Updated the `runtime_deps` attribute in `repositories()` to add the
  Scala version suffix, just like `deps`.

- Added a `fail()` message to `repositories()` to make it more clear
  which Scala version dictionary is missing an artifact.

- Removed unnecessary internal uses of the `@io_bazel_rules_scala` repo
  name, applying `Label()` where necessary.

- Updated the construction of `dep_providers` in
  `_default_dep_providers` to remove the repo name, reduce duplication,
  and make the upcoming toolchain update slightly cleaner.

---

Before this change, `repositories()` would originally emit `BUILD`
targets including canonical repo names:

```py
scala_import(
    name = "_main~scala_deps~io_bazel_rules_scala_scala_compiler",
    jars = ["scala-compiler-2.12.18.jar"],
)
```

resulting in errors like:

```txt
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'
```

---

Attaching resources from custom repos to targets under Bzlmod, like in
`scalarules.test.resources.ScalaLibResourcesFromExternalDepTest`, would
break with:

```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)
```

`_update_external_target_path` in `resources.bzl` fixes this problem.

---

Fixes `test_strict_deps_filter_included_target` from
`test/shell/test_strict_dependency.sh` when run under Bzlmod.

The `dependency_tracking_strict_deps_patterns` attribute of
//test_expect_failure/missing_direct_deps/filtering:plus_one_strict_deps_filter_a_impl
contains patterns starting with `@//`. However, in `_phase_dependency()`
from `scala/private/phases/phase_dependency.bzl`, these patterns were
compared against a stringified Label. Under Bazel < 7.1.0, this works
for root target Labels. Under Bazel >= 7.1.0, this breaks for root
target Labels under Bzlmod, which start with `@@//`.

`adjust_main_repo_prefix` updates the patterns accordingly in
`_partition_patterns` from `scala_toolchain.bzl`.
`apparent_repo_label_string` makes `_phase_dependency()` more resilient
when comparing target Labels against filters containing external
apparent repo names.

---

Fixes the `alias` targets generated by `_jvm_import_external` from
`scala_maven_import_external.bzl` by setting the `target` to the correct
apparent repo name.

Added `apparent_repo_name(repository_ctx.name)` to
`_jvm_import_external` to avoid this familiar error when running
`dt_patches/test_dt_patches` tests:

```txt
$ bazel build //...

ERROR: .../external/_main~compiler_source_repos~scala_reflect/BUILD:
  no such target '@@_main~compiler_source_repos~scala_reflect//:scala_reflect':
  target 'scala_reflect' not declared in package '' defined by
  .../external/_main~compiler_source_repos~scala_reflect/BUILD

ERROR: .../dt_patches/test_dt_patches/BUILD:11:22:
  no such target '@@_main~compiler_source_repos~scala_reflect//:scala_reflect':
  target 'scala_reflect' not declared in package '' defined by
  .../external/_main~compiler_source_repos~scala_reflect/BUILD
  and referenced by '//:dt_scala_toolchain_scala_compile_classpath_provider'

ERROR: Analysis of target
  '//:dt_scala_toolchain_scala_compile_classpath_provider' failed;
  build aborted: Analysis failed
```

---

As for why we need these macros, we can't rely on hacking 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`. 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:

- bazelbuild/bazel#21316

And the maintainers recently replaced the `~` delimiter with `+` in the
upcoming Bazel 8 release due to build performance issues on Windows:

- bazelbuild/bazel#22865

The core `apparent_repo_name` function assumes the only valid repo name
characters are letters, numbers, '_', '-', and '.'. 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
  • Loading branch information
mbland committed Oct 21, 2024
1 parent 1aeaad4 commit 2c076f2
Show file tree
Hide file tree
Showing 6 changed files with 118 additions and 24 deletions.
71 changes: 71 additions & 0 deletions scala/private/macros/bzlmod.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
"""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).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 repo 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
9 changes: 5 additions & 4 deletions scala/private/phases/phase_dependency.bzl
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
# Gathers information about dependency mode and analysis

load(
"@io_bazel_rules_scala//scala/private:dependency.bzl",
"//scala/private:dependency.bzl",
"get_compiler_deps_mode",
"get_strict_deps_mode",
"new_dependency_info",
)
load("//scala/private:macros/bzlmod.bzl", "apparent_repo_label_string")
load(
"@io_bazel_rules_scala//scala/private:paths.bzl",
"//scala/private:paths.bzl",
_get_files_with_extension = "get_files_with_extension",
_java_extension = "java_extension",
)
Expand Down Expand Up @@ -35,9 +36,9 @@ def _phase_dependency(
p,
unused_deps_always_off,
strict_deps_always_off):
toolchain = ctx.toolchains["@io_bazel_rules_scala//scala:toolchain_type"]
toolchain = ctx.toolchains[Label("//scala:toolchain_type")]

target_label = str(ctx.label)
target_label = apparent_repo_label_string(ctx.label)

included_in_strict_deps_analysis = _is_target_included(
target_label,
Expand Down
10 changes: 9 additions & 1 deletion scala/private/resources.bzl
Original file line number Diff line number Diff line change
@@ -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)
Expand All @@ -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
Expand Down
8 changes: 6 additions & 2 deletions scala/scala_maven_import_external.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -64,7 +65,10 @@ def _jvm_import_external(repository_ctx):
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")
name = repository_ctx.attr.generated_rule_name or repository_ctx.name
name = (
repository_ctx.attr.generated_rule_name or
apparent_repo_name(repository_ctx.name)
)
urls = repository_ctx.attr.jar_urls
if repository_ctx.attr.jar_sha256:
print("'jar_sha256' is deprecated. Please use 'artifact_sha256'")
Expand Down Expand Up @@ -136,7 +140,7 @@ def _jvm_import_external(repository_ctx):
"",
"alias(",
" name = \"jar\",",
" actual = \"@%s\"," % repository_ctx.name,
" actual = \"@%s\"," % apparent_repo_name(repository_ctx.name),
")",
"",
]))
Expand Down
26 changes: 12 additions & 14 deletions scala/scala_toolchain.bzl
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
load(
"@io_bazel_rules_scala//scala:providers.bzl",
_DepsInfo = "DepsInfo",
)
load("//scala/private:macros/bzlmod.bzl", "adjust_main_repo_prefix")
load("//scala:providers.bzl", _DepsInfo = "DepsInfo")
load(
"@io_bazel_rules_scala_config//:config.bzl",
"ENABLE_COMPILER_DEPENDENCY_TRACKING",
Expand Down Expand Up @@ -31,12 +29,12 @@ def _compute_dependency_tracking_method(

def _partition_patterns(patterns):
includes = [
pattern
adjust_main_repo_prefix(pattern)
for pattern in patterns
if not pattern.startswith("-")
]
excludes = [
pattern.lstrip("-")
adjust_main_repo_prefix(pattern.lstrip("-"))
for pattern in patterns
if pattern.startswith("-")
]
Expand Down Expand Up @@ -108,15 +106,15 @@ def _scala_toolchain_impl(ctx):

def _default_dep_providers():
dep_providers = [
"@io_bazel_rules_scala//scala:scala_xml_provider",
"@io_bazel_rules_scala//scala:parser_combinators_provider",
"@io_bazel_rules_scala//scala:scala_compile_classpath_provider",
"@io_bazel_rules_scala//scala:scala_library_classpath_provider",
"@io_bazel_rules_scala//scala:scala_macro_classpath_provider",
"scala_xml",
"parser_combinators",
"scala_compile_classpath",
"scala_library_classpath",
"scala_macro_classpath",
]
if SCALA_MAJOR_VERSION.startswith("2"):
dep_providers.append("@io_bazel_rules_scala//scala:semanticdb_provider")
return dep_providers
if SCALA_MAJOR_VERSION.startswith("2."):
dep_providers.append("semanticdb")
return [Label("//scala:%s_provider" % p) for p in dep_providers]

scala_toolchain = rule(
_scala_toolchain_impl,
Expand Down
18 changes: 15 additions & 3 deletions third_party/repositories/repositories.bzl
Original file line number Diff line number Diff line change
@@ -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",
Expand Down Expand Up @@ -102,14 +103,25 @@ 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:
if id not in artifacts:
fail("artifact %s not in third_party/repositories/scala_%s.bzl" % (
id,
major_scala_version.replace(".", "_"),
))

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"],
server_urls = maven_servers,
deps = [dep + suffix for dep in artifacts[id].get("deps", [])],
runtime_deps = artifacts[id].get("runtime_deps", []),
runtime_deps = [
dep + suffix
for dep in artifacts[id].get("runtime_deps", [])
],
testonly_ = artifacts[id].get("testonly", False),
fetch_sources = fetch_sources,
)
Expand All @@ -118,13 +130,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(
Expand Down

0 comments on commit 2c076f2

Please sign in to comment.