Skip to content

Commit

Permalink
Toolchainize all testing toolchains (#1653)
Browse files Browse the repository at this point in the history
* Toolchainize all testing toolchains

Moves the Scalatest, JUnit, and Specs2 toolchains into
`@io_bazel_rules_scala_toolchains//testing`. Part of #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` (#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 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.)

* Move `scala_toolchains` to `scala/toolchains.bzl`

Removes this symbol from `scala/scala.bzl` as well as
`setup_scala_testing_toolchain`, and deletes
`scala/private/macros/toolchains.bzl`. Part of #1482 and #1652.

This is required for Bazel 8 and `rules_java` 8 compatibility, but is
also compatible with Bazel 6 and 7. In #1652, @hvadehra suggested
partitioning the `.bzl` files such that `WORKSPACE` doesn't `load` a
file that tries to `load` symbols from `rules_java`. I successfully did
so in a separate branch, and along with other minor changes, got
`rules_scala` to build with `rules_java` 8.5.1.

The other changes will come in separate pull requests, but it makes
sense to land this change now before adding any other toolchains to
`scala_toolchains`.

---

Arguably, we should remove all macros exported from `scala/scala.bzl`
that only instantiate toolchain dependencies and define toolchains. That
may be a breaking change for some users, but will ultimately be
necessary for these macros to remain compatible with Bazel 8.

* Extract versioned `_JUNIT_DEPS` in `test/BUILD`

Eliminates reliance on the default `@io_bazel_rules_scala_junit_junit`
artifact repository.

* Update `{junit,specs2_junit}_toolchain()`

These macros now mirror the implementation of `scalatest_toolchain()`.

However, I realized that the old pattern of calling
`scalatest_repositories()` followed by `scalatest_toolchain()` will no
longer work without first calling `scala_toolchains(scalatest = True)`.
This is because the `alias` targets in `testing/BUILD` that replace the
previous implementations all point to
`@io_bazel_rules_scala_toolchains`.

So if we want to keep these macros, it seems like we should maybe
restore the original toolchain targets in `testing/BUILD`. If we don't,
we can remove these macros, but we can document these as breaking
changes, and update other documentation accordingly.

* Move `scala/{private/macros/,}toolchains_repo.bzl`

Like "Move `scala_toolchains` to `scala/toolchains.bzl`",
removes the `scala_toolchains_repo` symbol from `scala/scala.bzl` and
makes it available from `scala/toolchains_repo.bzl`.

This avoids a future `test_scala_version 2.12.20` failure during Bazel 8
builds after adding `twitter_scrooge` toolchain support in the new
`test_version/version_specific_tests_dir/scrooge_repositories.bzl` file.
Otherwise, this new file would load `toolchains_repo` from
`scala/scala.bzl`. The `test_version/test_scala_version_.../WORKSPACE`
file generated from `test_version/WORKSPACE.template` would then
transitively load `.bzl` files with `rules_java` symbols, breaking the
test.

```txt
$ RULES_SCALA_TEST_ONLY="test_scala_version 2.12.20" ./test_version.sh

ERROR: Traceback (most recent call last):
  File ".../external/rules_scala/scala/private/common_attributes.bzl",
  line 18, column 28, in <toplevel>
    "deps": attr.label_list(

Error in label_list:
  Illegal argument: element in 'providers' is of unexpected type.
  Either all elements should be providers,
  or all elements should be lists of providers,
  but got list with an element of type NoneType.

ERROR: Error computing the main repository mapping:
  at test_version/test_scala_version_.../scrooge_repositories.bzl:1:6:
  at .../scala/scala.bzl:30:5:
  at .../scala/private/rules/scala_junit_test.bzl:5:5:
initialization of module 'scala/private/common_attributes.bzl' failed
```
  • Loading branch information
mbland authored Jan 14, 2025
1 parent 719f353 commit 6e36591
Show file tree
Hide file tree
Showing 38 changed files with 501 additions and 430 deletions.
16 changes: 5 additions & 11 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,14 @@ load("@io_bazel_rules_scala//:scala_config.bzl", "scala_config")

scala_config(enable_compiler_dependency_tracking = True)

load("//scala:scala.bzl", "scala_toolchains")
load("//scala:toolchains.bzl", "scala_toolchains")

scala_toolchains(fetch_sources = True)
scala_toolchains(
fetch_sources = True,
testing = True,
)

register_toolchains(
"//testing:testing_toolchain",
"//scala:unused_dependency_checker_error_toolchain",
"//test/proto:scalapb_toolchain",
"@io_bazel_rules_scala_toolchains//...:all",
Expand Down Expand Up @@ -69,14 +71,6 @@ load("//scala_proto:scala_proto.bzl", "scala_proto_repositories")

scala_proto_repositories()

load("//scalatest:scalatest.bzl", "scalatest_repositories")

scalatest_repositories()

load("//specs2:specs2_junit.bzl", "specs2_junit_repositories")

specs2_junit_repositories()

load("//scala/scalafmt:scalafmt_repositories.bzl", "scalafmt_default_config", "scalafmt_repositories")

scalafmt_default_config()
Expand Down
22 changes: 17 additions & 5 deletions docs/testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ Test framework dependencies are configured via testing toolchain. For convenienc
`setup_scala_testing_toolchain` can be used to define such toolchains.

```starlark
load("@io_bazel_rules_scala//scala:scala.bzl", "setup_scala_testing_toolchain")
load(
"@io_bazel_rules_scala//testing:testing.bzl",
"setup_scala_testing_toolchain",
)
```

Attributes
Expand All @@ -53,7 +56,10 @@ Examples (assumes maven deps are managed with rules_jvm_external):

```starlark
# BUILD
load("@io_bazel_rules_scala//scala:scala.bzl", "setup_scala_testing_toolchain")
load(
"@io_bazel_rules_scala//testing:testing.bzl",
"setup_scala_testing_toolchain",
)

setup_scala_testing_toolchain(
name = "scalatest_toolchain",
Expand All @@ -77,7 +83,10 @@ register_toolchains('//:scalatest_toolchain')
#### JUnit 4
```starlark
# BUILD
load("@io_bazel_rules_scala//scala:scala.bzl", "setup_scala_testing_toolchain")
load(
"@io_bazel_rules_scala//testing:testing.bzl",
"setup_scala_testing_toolchain",
)

setup_scala_testing_toolchain(
name = "junit_toolchain",
Expand All @@ -98,7 +107,10 @@ For Specs2 rules to work, `junit_classpath`, `specs2_junit_classpath` and `specs
be configured.
```starlark
# BUILD
load("@io_bazel_rules_scala//scala:scala.bzl", "setup_scala_testing_toolchain")
load(
"@io_bazel_rules_scala//testing:testing.bzl",
"setup_scala_testing_toolchain",
)

setup_scala_testing_toolchain(
name = "specs2_toolchain",
Expand All @@ -122,4 +134,4 @@ Register the toolchain
```starlark
# WORKSPACE
register_toolchains('//:specs2_toolchain')
```
```
2 changes: 1 addition & 1 deletion dt_patches/test_dt_patches/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ load("@compiler_sources//:extensions.bzl", "import_compiler_source_repos")

import_compiler_source_repos()

load("@io_bazel_rules_scala//scala:scala.bzl", "scala_toolchains")
load("@io_bazel_rules_scala//scala:toolchains.bzl", "scala_toolchains")

scala_toolchains(
fetch_sources = True,
Expand Down
2 changes: 1 addition & 1 deletion dt_patches/test_dt_patches_user_srcjar/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ srcjars_by_version = {
},
}

load("@io_bazel_rules_scala//scala:scala.bzl", "scala_toolchains")
load("@io_bazel_rules_scala//scala:toolchains.bzl", "scala_toolchains")

scala_toolchains(
fetch_sources = True,
Expand Down
10 changes: 2 additions & 8 deletions examples/crossbuild/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ scala_config(
],
)

load("@io_bazel_rules_scala//scala:scala.bzl", "scala_toolchains")
load("@io_bazel_rules_scala//scala:toolchains.bzl", "scala_toolchains")

scala_toolchains()
scala_toolchains(scalatest = True)

register_toolchains("@io_bazel_rules_scala_toolchains//...:all")

Expand All @@ -59,9 +59,3 @@ rules_proto_toolchains()
load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")

protobuf_deps()

load("@io_bazel_rules_scala//testing:scalatest.bzl", "scalatest_repositories", "scalatest_toolchain")

scalatest_repositories()

scalatest_toolchain()
2 changes: 1 addition & 1 deletion examples/scala3/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ load("@io_bazel_rules_scala//:scala_config.bzl", "scala_config")

scala_config(scala_version = "3.6.2")

load("@io_bazel_rules_scala//scala:scala.bzl", "scala_toolchains")
load("@io_bazel_rules_scala//scala:toolchains.bzl", "scala_toolchains")

scala_toolchains(fetch_sources = True)

Expand Down
2 changes: 1 addition & 1 deletion examples/semanticdb/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ load("@io_bazel_rules_scala//:scala_config.bzl", "scala_config")

scala_config(scala_version = "2.13.15")

load("@io_bazel_rules_scala//scala:scala.bzl", "scala_toolchains")
load("@io_bazel_rules_scala//scala:toolchains.bzl", "scala_toolchains")

scala_toolchains(fetch_sources = True)

Expand Down
11 changes: 6 additions & 5 deletions examples/testing/multi_frameworks_toolchain/BUILD
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
load("@io_bazel_rules_scala//scala:scala.bzl", "setup_scala_testing_toolchain")
load("@io_bazel_rules_scala//scala:scala_cross_version.bzl", "version_suffix")
load("@io_bazel_rules_scala_config//:config.bzl", "SCALA_VERSION")
load(
"@io_bazel_rules_scala//testing:testing.bzl",
"setup_scala_testing_toolchain",
)

setup_scala_testing_toolchain(
name = "testing_toolchain",
junit_classpath = [
"@io_bazel_rules_scala_junit_junit",
"@io_bazel_rules_scala_org_hamcrest_hamcrest_core",
],
scalatest_classpath = [dep + version_suffix(SCALA_VERSION) for dep in [
scalatest_classpath = [
"@io_bazel_rules_scala_scalactic",
"@io_bazel_rules_scala_scalatest",
"@io_bazel_rules_scala_scalatest_compatible",
Expand All @@ -21,7 +22,7 @@ setup_scala_testing_toolchain(
"@io_bazel_rules_scala_scalatest_matchers_core",
"@io_bazel_rules_scala_scalatest_mustmatchers",
"@io_bazel_rules_scala_scalatest_shouldmatchers",
]],
],
specs2_classpath = [
"@io_bazel_rules_scala_org_specs2_specs2_common",
"@io_bazel_rules_scala_org_specs2_specs2_core",
Expand Down
17 changes: 5 additions & 12 deletions examples/testing/multi_frameworks_toolchain/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,12 @@ load("@io_bazel_rules_scala//:scala_config.bzl", "scala_config")

scala_config()

load("@io_bazel_rules_scala//scala:scala.bzl", "scala_toolchains")
load("@io_bazel_rules_scala//scala:toolchains.bzl", "scala_toolchains")

scala_toolchains(fetch_sources = True)
scala_toolchains(
fetch_sources = True,
testing = True,
)

register_toolchains(
":testing_toolchain",
Expand All @@ -55,13 +58,3 @@ rules_proto_toolchains()
load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")

protobuf_deps()

load("@io_bazel_rules_scala//testing:scalatest.bzl", "scalatest_repositories")
load("@io_bazel_rules_scala//testing:junit.bzl", "junit_repositories")
load("@io_bazel_rules_scala//testing:specs2_junit.bzl", "specs2_junit_repositories")

scalatest_repositories()

junit_repositories()

specs2_junit_repositories()
13 changes: 5 additions & 8 deletions examples/testing/scalatest_repositories/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,12 @@ load("@io_bazel_rules_scala//:scala_config.bzl", "scala_config")

scala_config()

load("@io_bazel_rules_scala//scala:scala.bzl", "scala_toolchains")
load("@io_bazel_rules_scala//scala:toolchains.bzl", "scala_toolchains")

scala_toolchains(fetch_sources = True)
scala_toolchains(
fetch_sources = True,
scalatest = True,
)

register_toolchains("@io_bazel_rules_scala_toolchains//...:all")

Expand All @@ -52,9 +55,3 @@ rules_proto_toolchains()
load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")

protobuf_deps()

load("@io_bazel_rules_scala//testing:scalatest.bzl", "scalatest_repositories", "scalatest_toolchain")

scalatest_repositories()

scalatest_toolchain()
13 changes: 5 additions & 8 deletions examples/testing/specs2_junit_repositories/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,12 @@ load("@io_bazel_rules_scala//:scala_config.bzl", "scala_config")

scala_config()

load("@io_bazel_rules_scala//scala:scala.bzl", "scala_toolchains")
load("@io_bazel_rules_scala//scala:toolchains.bzl", "scala_toolchains")

scala_toolchains(fetch_sources = True)
scala_toolchains(
fetch_sources = True,
specs2 = True,
)

register_toolchains("@io_bazel_rules_scala_toolchains//...:all")

Expand All @@ -52,9 +55,3 @@ rules_proto_toolchains()
load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")

protobuf_deps()

load("@io_bazel_rules_scala//testing:specs2_junit.bzl", "specs2_junit_repositories", "specs2_junit_toolchain")

specs2_junit_repositories()

specs2_junit_toolchain()
11 changes: 7 additions & 4 deletions junit/junit.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,17 @@ load(
)
load("//third_party/repositories:repositories.bzl", "repositories")

def junit_artifact_ids():
return [
"io_bazel_rules_scala_junit_junit",
"io_bazel_rules_scala_org_hamcrest_hamcrest_core",
]

def junit_repositories(
maven_servers = _default_maven_server_urls(),
fetch_sources = True):
repositories(
for_artifact_ids = [
"io_bazel_rules_scala_junit_junit",
"io_bazel_rules_scala_org_hamcrest_hamcrest_core",
],
for_artifact_ids = junit_artifact_ids(),
fetch_sources = fetch_sources,
maven_servers = maven_servers,
)
66 changes: 0 additions & 66 deletions scala/private/macros/toolchains.bzl

This file was deleted.

Loading

0 comments on commit 6e36591

Please sign in to comment.