Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Toolchainize all testing toolchains #1653

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mbland
Copy link
Contributor

@mbland mbland commented Nov 27, 2024

Description

Moves the Scalatest, JUnit, and Specs2 toolchains into @io_bazel_rules_scala_toolchains//testing. Part of #1482.

Introduces more macros to return a framework's Maven artifact dependencies, rather than inlining them in a repositories call. These inlined lists are replaced by macro invocations, and now the scala_toolchains macro can invoke these macros to collect artifact IDs to pass to repositories. This also allows for future changes to introduce a scala_version parameter if necessary, similar to how scalafmt_artifact_ids already works.

Updates all WORKSPACE files to set the appropriate scala_toolchains parameters and to remove the unnecessary repository import and toolchain registration macros.

Adds a fetch_sources_by_id parameter to repositories from third_party/repositories/repositories.bzl.

Updates scala/scala_maven_import_external.bzl to generate a load line for //scala:scala_import.bzl based on the repo's canonical name, not @io_bazel_rules_scala.

As usual, includes several other opportunistic removals of the @io_bazel_rules_scala repo name prefix to avoid an internal dependency on that name.

Motivation

The macros returning artifact ID lists help avoid collisions when creating repositories dependencies upon which multiple frameworks depend under Bzlmod. WORKSPACE doesn't seem affected by these collisions, but Bzlmod will produce errors like the following, where both scala_proto and twitter_scrooge depend upon io_bazel_rules_scala_guava:

$ bazel build //src/...

ERROR: .../scala/scala_maven_import_external.bzl:299:24:
  Traceback (most recent call last):
    File ".../scala/extensions/deps.bzl",
      line 140, column 21, in _scala_deps_impl
        scala_toolchains(
    File ".../scala/private/macros/toolchains.bzl",
      line 140, column 17, in scala_toolchains
        _scrooge(
    File ".../twitter_scrooge/twitter_scrooge.bzl",
      line 96, column 17, in twitter_scrooge
        repositories(
    File ".../third_party/repositories/repositories.bzl",
      line 113, column 37, in repositories
        _scala_maven_import_external(
    File ".../scala/scala_maven_import_external.bzl",
      line 263, column 30, in scala_maven_import_external
        jvm_maven_import_external(
    File ".../scala/scala_maven_import_external.bzl",
      line 299, column 24, in jvm_maven_import_external
        jvm_import_external(jar_urls = jar_urls, srcjar_urls = srcjar_urls, coordinates = artifact, **kwargs)

Error in repository_rule: A repo named
  io_bazel_rules_scala_guava_2_13_15
  is already generated by this module extension at
  .../scala/scala_maven_import_external.bzl:299:24

ERROR: Analysis of target
  '//src/java/io/bazel/rulesscala/worker:worker_test'
  failed; build aborted:
  error evaluating module extension scala_deps in
  //scala/extensions:deps.bzl

Recent updates to scripts/create_repository.py (#1639, #1642) make it easy to emit full direct dependency lists for artifacts included in third_party/repositories/scala_*.bzl. This increases the likelihood of collisions, since this expanded metadata forces the macros that instantiate artifact repos to instantiate even more repos.

By fetching lists of artifact IDs from these macros, scala_toolchains can now consolidate them into dictionary keys. Then it passes these unique keys to repositories directly, avoiding the problem of instantiating the same repo multiple times in the same module extension.

This, in turn, also avoids the need to add parameters to the original WORKSPACE macros that instantiate dependencies to avoid collisions under Bzlmod. The scala_toolchains macro never needs to call these original macros, under either WORKSPACE or Bzlmod.

Finally, it also reduces duplication between these artifact ID lists and the _*_DEPS symbols originally from testing/BUILD (and now in testing/deps.bzl). The dependency labels are now generated programatically.

(Aside: As I mentioned, we may eventually need to pass a Scala version argument to these macros. It will be possible to cross that bridge without too much trouble if and when that day comes. Or I can try to future proof it in a follow up pull request.)

Adding the fetch_sources_by_id parameter to repositories() enables scala_toolchains to build the artifact_ids_to_fetch_sources mapping from artifact ID lists returned by these new macros. The values assigned to each id match the original fetch_sources settings in the corresponding original WORKSPACE macros.

Removing @io_bazel_rules_scala target prefixes means Bzlmod users won't necessarily have to set the repo_name parameter of bazel_dep when using rules_scala.

Moves the Scalatest, JUnit, and Specs2 toolchains into
`@io_bazel_rules_scala_toolchains//testing`. Part of bazelbuild#1482.

Updates all `WORKSPACE` files to set the appropriate `scala_toolchains`
parameters and to remove the unnecessary repository import and toolchain
registration macros.

Adds a `fetch_sources_by_id` parameter to `repositories` from
`third_party/repositories/repositories.bzl`. This enables
`scala_toolchains` to build the `artifact_ids_to_fetch_sources` mapping
from artifact ID lists returned by new macros extracted from `WORKSPACE`
macros. The values assigned to each id match the original
`fetch_sources` settings in the corresponding original `WORKSPACE`
macros.

Updates `scala/scala_maven_import_external.bzl` to generate a `load`
line for `//scala:scala_import.bzl` based on the repo's canonical name,
not `@io_bazel_rules_scala`.

As usual, includes several other opportunistic removals of the
`@io_bazel_rules_scala` repo name prefix to avoid an internal dependency
on that name. This means Bzlmod users won't necessarily have to set the
`repo_name` parameter of `bazel_dep` when using `rules_scala`.

---

Introduces more macros to return a framework's Maven artifact
dependencies, rather than inlining them in a `repositories` call. These
inlined lists are replaced by macro invocations, and now the
`scala_toolchains` macro can invoke these macros to collect artifact IDs
to pass to `repositories`. This also allows for future changes to
introduce a `scala_version` parameter if necessary, similar to how
`scalafmt_artifact_ids` already works.

This is important to avoid collisions when creating repositories for
artifacts upon which more than one framework depends under Bzlmod.
`WORKSPACE` doesn't seem affected by these collisions, but Bzlmod will
produce errors like the following, where both `scala_proto` and
`twitter_scrooge` depend upon `io_bazel_rules_scala_guava`:

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

ERROR: .../scala/scala_maven_import_external.bzl:299:24:
  Traceback (most recent call last):
    File ".../scala/extensions/deps.bzl",
      line 140, column 21, in _scala_deps_impl
        scala_toolchains(
    File ".../scala/private/macros/toolchains.bzl",
      line 140, column 17, in scala_toolchains
        _scrooge(
    File ".../twitter_scrooge/twitter_scrooge.bzl",
      line 96, column 17, in twitter_scrooge
        repositories(
    File ".../third_party/repositories/repositories.bzl",
      line 113, column 37, in repositories
        _scala_maven_import_external(
    File ".../scala/scala_maven_import_external.bzl",
      line 263, column 30, in scala_maven_import_external
        jvm_maven_import_external(
    File ".../scala/scala_maven_import_external.bzl",
      line 299, column 24, in jvm_maven_import_external
        jvm_import_external(jar_urls = jar_urls, srcjar_urls = srcjar_urls, coordinates = artifact, **kwargs)

Error in repository_rule: A repo named
  io_bazel_rules_scala_guava_2_13_15
  is already generated by this module extension at
  .../scala/scala_maven_import_external.bzl:299:24

ERROR: Analysis of target
  '//src/java/io/bazel/rulesscala/worker:worker_test'
  failed; build aborted:
  error evaluating module extension scala_deps in
  //scala/extensions:deps.bzl
```

Recent updates to `scripts/create_repository.py` (bazelbuild#1639, bazelbuild#1642) make it
easy to emit full direct dependency lists for artifacts included in
`third_party/repositories/scala_*.bzl`. This increases the likelihood of
collisions, since this expanded metadata forces the macros that
instantiate artifact repos to instantiate even more repos.

By fetching list of artifact IDs from these macros, `scala_toolchains`
can now consolidate them into dictionary keys. Then it passes these
unique keys to `repositories` directly, avoiding the problem of
instantiating the same repo multiple times in the same module extension.

This, in turn, also avoids the need to add parameters to the original
`WORKSPACE` macros that instantiate dependencies to avoid collisions
under Bzlmod. The `scala_toolchains` macro never needs to call these
original macros, under either `WORKSPACE` or Bzlmod.

Finally, it also reduces duplication between these artifact ID lists and
the `_*_DEPS` symbols originally from `testing/BUILD` (and now in
`testing/deps.bzl`). The dependency labels are now generated
programatically.

(Aside: As I mentioned, we may eventually need to pass a Scala version
argument to these macros. It will be possible to cross that bridge
without too much trouble if and when that day comes. Or I can try to
future proof it in a follow up pull request.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant