Support Bzlmod and add rules_scala to bazel-central-registry #1482

sfc-gh-pbennes · 2023-03-16T23:26:47Z

Greetings!

It looks like rules_scala isn't part of the Bzlmod effort or added to bazel-central-registry yet. I've opened an issue both here and in https://github.com/bazelbuild/bazel-central-registry to request it: bazelbuild/bazel-central-registry#522

chrislovecnm · 2023-07-11T15:30:04Z

Just a note to anyone that starts the work native.register_toolchains is not supported when using bzlmod.

sluongng · 2023-07-11T15:35:37Z

@chrislovecnm it's still available here no? https://bazel.build/rules/lib/globals/module#register_toolchains

chrislovecnm · 2023-07-11T15:42:51Z

@sluongng When I last tried it, it did not work when using it inside an extension. In rules_python we are skipping the native call:

https://github.com/bazelbuild/rules_python/blob/36082079b514529d4009a2b104475fde1cdd5b30/python/repositories.bzl#L569

And build the toolchains here:

https://github.com/bazelbuild/rules_python/blob/36082079b514529d4009a2b104475fde1cdd5b30/python/extensions/private/pythons_hub.bzl#L56

We build the toolchains and use the following:

https://github.com/bazelbuild/rules_python/blob/36082079b514529d4009a2b104475fde1cdd5b30/MODULE.bazel#L49

sluongng · 2023-07-11T15:47:13Z

Yeah you would only want to prepare the toolchain (as a new repository) inside the extension I think. Registering it must be called from the MODULE.bazel file.

So native.register_toolchains call from the extension's starlark does not work, but having the extension create @pythons_hub//:all and then MODULE.bazel calling register_toolchains("@pythons_hub//:all") worked. 👍

mateuszkuta256 · 2024-01-24T08:56:24Z

I prepared a minimal bzlmod support here and gonna split it into a few PRs
TODOs after merging it:

write documentation on how to use rules_scala with bzlmod (some properties like SCALA_VERSION will probably be configurable via repo-env)
prepare release notes
contribute repo to the Bazel Central Registry
add tests that the project builds using both WORKSPACE and MODULE.bazel
migrate test/external workspaces to blzmod too

pcj · 2024-06-30T17:14:28Z

@mateuszkuta256 thanks for getting this started! Any progress updates or blockers?

mateuszkuta256 · 2024-07-01T08:54:25Z

@mateuszkuta256 thanks for getting this started! Any progress updates or blockers?

hi, unfortunately I'm working on other things now and won't continue with this PR in the foreseeable future
I remember this migration was on hold because interfered with 'cross compilation support'
It looks like great progress was done it this area, so maybe someone can already take over the PR

mbland · 2024-10-02T17:41:34Z

I'd like to take on the task of Bzlmodifying this repo through a series of pull requests.

I've already created a (mostly) working branch in my own fork. Though I saw a couple draft pull requests here, I ended up taking a different approach and got it mostly working. In fact, I did exactly what @sluongng suggested in #1482 (comment). (I didn't notice this comment before just now—I might've read it, but not understood it at the time—but I did study rules_python and rules_go, and ended up doing exactly that.)

There are still issues I need help to address (recorded in some of the commit messages), I didn't strictly maintain WORKSPACE compatibility (fixable), and I only tested against Scala 2.13. But I can start teasing chunks out of it in a methodical fashion, to ensure that I maintain WORKSPACE and cross-version compatibility.

For an example of what it looks like from a client perspective, here's what the MODULE.bazel stanza from my local EngFlow/example repo looks like (as opposed to the current stanza, explanatory comment notwithstanding):

bazel_dep(name = "rules_scala", repo_name = "io_bazel_rules_scala")
local_path_override(
    module_name = "rules_scala",
    path = "../../bazelbuild/rules_scala"
)   

scala_dev_deps = use_extension(
    "@io_bazel_rules_scala//scala/extensions:deps.bzl",
    "scala_deps",
    dev_dependency = True,
)   
scala_dev_deps.toolchains(
    scalatest = True,
)

So if folks are game for me to do this, I'll start carving off pieces as separate pull requests, and we can resolve any outstanding problems in the process.

cc: @BillyAutrey @jayconrod @benjaminp @TheGrizzlyDev

This begins the Bzlmod compatibility migration by updating Bazel to version 7.3.2 and adding initial `MODULE.bazel` and `WORKSPACE.bzlmod` files. Part of: bazelbuild#1482 Though Bzlmod remains disabled, updating to Bazel 7.3.2 requred updating or adding the following packages to maintain `WORKSPACE` compatibility. In `rules_scala_setup()`: - bazel_skylib: 1.4.1 => 1.7.1 - rules_cc: 0.0.6 => 0.0.10 - rules_java: 5.4.1 => 7.9.0 - rules_proto: 5.3.0-21.7 => 6.0.2 Dev dependencies in `WORKSPACE`: - com_google_protobuf: 28.2 - rules_pkg: 1.0.1 - rules_jvm_external: 6.4 - com_google_absl: abseil-cpp-20240722.0 - zlib: 1.3.1 Of all of the new, explicit dev dependencies, only `com_google_protobuf` will be necessary to include in `MODULE.bazel`. The Bzlmod mechanism will discover these other transitive dev dependencies automatically. Also removed the `rules_java_extra` repo from `WORKSPACE`, which appeared unused. --- Though the current `rules_java` version is 7.12.1, and largely works with this repo, it requires a few temporary workarounds. Rather than commit the workarounds, upgrading only to 7.9.0 now seems less crufty. What follows is a very detailed explanation of what happens with 7.12.1 with Bazel 7.3.2, just to have it on the record. --- The workaround is to change a few toolchain and macro file targets from `@bazel_tools//tools/jdk:` to `@rules_java//toolchains:`. This isn't a terribly bad or invasive workaround, but `@bazel_tools//tools/jdk:` is clearly the canonical path. Best to keep it that way, lest we build up technical debt. Without the workaround, these targets would fail: - //test/src/main/resources/java_sources:CompiledWithJava11 - //test/src/main/resources/java_sources:CompiledWithJava8 - //test/toolchains:java21_toolchain - //test:JunitRuntimePlatform - //test:JunitRuntimePlatform_test_runner - //test:scala_binary_jdk_11 with this error: ```txt ERROR: .../external/rules_java_builtin/toolchains/BUILD:254:14: While resolving toolchains for target @@rules_java_builtin//toolchains:platformclasspath (096dcc8): No matching toolchains found for types @@bazel_tools//tools/jdk:bootstrap_runtime_toolchain_type. ``` This appears to be a consequence of both upgrading the Bazel version from 6.3.0 to 7.3.2 and updating `rules_java` to 7.12.1. The `rules_java_builtin` repo is part of the `WORKSPACE` prefix that adds implicit dependencies: - https://bazel.build/external/migration#builtin-default-deps This repo was added to 7.0.0-pre.20231011.2 in the following change, mapped to `@rules_java` within the scope of the `@bazel_tools` repo: - bazelbuild/bazel: Add rules_java_builtin to the users of Java modules bazelbuild/bazel@ff1abb2 This change tried to ensure `rules_java` remained compatible with earlier Bazel versions. However, it changed all instances of `@bazel_tools//tools/jdk:bootstrap_runtime_toolchain_type` to `//toolchains:bootstrap_runtime_toolchain_type`: - bazelbuild/rules_java: Make rules_java backwards compatible with Bazel 6.3.0 bazelbuild/rules_java@30ecf3f Bazel has bumped `rules_java` in its `workspace_deps.bzl` from 7.9.0 to 7.11.0, but it's only available as of 8.0.0-pre.20240911.1. - bazelbuild/bazel: Update rules_java 7.11.1 / java_tools 13.8 bazelbuild/bazel#23571 bazelbuild/bazel@f92124a --- What I believe is happening is, under Bazel 7.3.2 and `rules_java` 7.12.1: - Bazel creates `rules_java` 7.9.0 as `@rules_java_builtin` in the `WORKSPACE` prefix. - `@bazel_tools` has `@rules_java` mapped to `@rules_java_builtin` when initialized during the `WORKSPACE` prefix, during which `@bazel_tools//tools/jdk` registers `alias()` targets to `@rules_java` toolchain targets. These aliased toolchains specify `@bazel_tools//tools/jdk:bootstrap_runtime_toolchain_type` in their `toolchains` attribute. - `WORKSPACE` loads `@rules_java` 7.12.1 and registers all its toolchains with type `@rules_java//toolchains:bootstrap_runtime_toolchain_type`. - Some `@rules_java` rules explicitly specifying toolchains from `@bazel_tools//tools/jdk` can't find them, because the `@bazel_tools//tools/jdk` toolchain aliases expect toolchains of type `@bazel_tools//tools/jdk:bootstrap_runtime_toolchain_type`. This has broken other projects in the same way: - bazelbuild/bazel: [Bazel CI] Downstream project broken by rules_java upgrade #23619 bazelbuild/bazel#23619 These problems don't appear under Bzlmod, and `@rules_java_builtin` was never required. This is because `WORKSPACE` executes its statements sequentially, while Bzlmod builds the module dependency graph _before_ instantiating repositories (within module extensions). It seems a fix is on the way that removes `@rules_java_builtin` from the `WORKSPACE` prefix, and adds `@rules_java` to the suffix. At this moment, though, it's not even in a prerelase: - bazelbuild/bazel: Remove rules_java_builtin in WORKSPACE prefix bazelbuild/bazel@7506690 --- Note that the error message matches that from the following resolved issue, but that issue was for non-Bzlmod child repos when `WORKSPACE` was disabled. - bazelbuild/bazel: Undefined @@rules_java_builtin repository with --noenable_workspace option bazelbuild/bazel#22754

Related to bazelbuild#1482, bazelbuild#1618, and bazelbuild#1619. Results from the investigation documented at: - bazelbuild#1619 (comment) Updates `_import_paths()` in `scala_proto_aspect.bzl` to handle differences `ProtoInfo.proto_source_root` and `ProtoInfo.direct_sources` values between Bazel 6 and Bazel 7. Without this change, `_import_paths()` emits incorrect values under Bazel 7, causing targets containing generated `.proto` inputs to fail, e.g. `//test/proto3:test_generated_proto`. See also: - Fix paths for sibling repository setup and generated .proto files bazelbuild/bazel@6c6c196 - The docstring for `ProtoInfo.proto_source_root` in the Bazel sources: https://github.com/bazelbuild/bazel/blob/7.3.2/src/main/starlark/builtins_bzl/common/proto/proto_info.bzl#L155-L172 - Remove incompatible_generated_protos_in_virtual_imports bazelbuild/bazel@3efaa32 - Comment from: Generated Protos are no longer considered as virtual_imports in Bazel 7 bazelbuild/bazel#21075 (comment) --- I cherrypicked this commit into bazelbuild#1618. While it fixed the `//test/proto3` build failure, it does _not_ fix the hanging scalapb_workers from the ProtoScalaPBRule aspect. I'll have to investiate further whether than hang is related to Bazel, rules_proto, com_google_protobuf, or some mixture thereof. Still, progress!

Part of bazelbuild#1482. Splits the last component off of canonical repo names to produce the expected repo name. Without Bzlmod, it returns the original name. With Bzlmod enabled, it avoids generating output like: scala_import( name = "_main~scala_deps~io_bazel_rules_scala_scala_compiler", jars = ["scala-compiler-2.12.18.jar"], ) resulting in errors like: ``` 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' ``` Also fixes the following error when attaching resources from custom repos to targets under Bzlmod: ```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) ``` Can be replaced with a future bazel-skylib implementation, if accepted into that repo. --- We can't rely on 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`. For extensions > hosted in the root module, the `module_repo_canonical_name` part is > replaced with the string `_main`. 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 This function assumes the only valid `repo_name` characters are letters, numbers, '_', '-', and '.'. It finds the last character not in this set, and returns the contents of `name` following this character. 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

Related to #1482, #1618, and #1619. Results from the investigation documented at: - #1619 (comment) Updates `_import_paths()` in `scala_proto_aspect.bzl` to handle differences `ProtoInfo.proto_source_root` and `ProtoInfo.direct_sources` values between Bazel 6 and Bazel 7. Without this change, `_import_paths()` emits incorrect values under Bazel 7, causing targets containing generated `.proto` inputs to fail, e.g. `//test/proto3:test_generated_proto`. See also: - Fix paths for sibling repository setup and generated .proto files bazelbuild/bazel@6c6c196 - The docstring for `ProtoInfo.proto_source_root` in the Bazel sources: https://github.com/bazelbuild/bazel/blob/7.3.2/src/main/starlark/builtins_bzl/common/proto/proto_info.bzl#L155-L172 - Remove incompatible_generated_protos_in_virtual_imports bazelbuild/bazel@3efaa32 - Comment from: Generated Protos are no longer considered as virtual_imports in Bazel 7 bazelbuild/bazel#21075 (comment) --- I cherrypicked this commit into #1618. While it fixed the `//test/proto3` build failure, it does _not_ fix the hanging scalapb_workers from the ProtoScalaPBRule aspect. I'll have to investiate further whether than hang is related to Bazel, rules_proto, com_google_protobuf, or some mixture thereof. Still, progress!

Part of bazelbuild#1482. Splits the last component off of canonical repo names to produce the expected repo name. Without Bzlmod, it returns the original name. With Bzlmod enabled, it avoids generating output like: scala_import( name = "_main~scala_deps~io_bazel_rules_scala_scala_compiler", jars = ["scala-compiler-2.12.18.jar"], ) resulting in errors like: ``` 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' ``` Also fixes the following error when attaching resources from custom repos to targets under Bzlmod: ```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) ``` Can be replaced with a future bazel-skylib implementation, if accepted into that repo. --- We can't rely on 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`. For extensions > hosted in the root module, the `module_repo_canonical_name` part is > replaced with the string `_main`. 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 This function assumes the only valid `repo_name` characters are letters, numbers, '_', '-', and '.'. It finds the last character not in this set, and returns the contents of `name` following this character. 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

mbland · 2024-10-11T16:49:17Z

A quick update for visibility: I'm very close to having the Bzlmodified rules_scala passing 100% of the tests, down to the last couple of failures:

I need to replace the bind() calls from twitter_scrooge with alias() or some such to get test_version.sh to pass.
I need to fix the Scala 3.1.2 and Scala 3.3.3 failures for the Scalafmt targets from test/shell/test_cross_build.sh (corresponding to the test3 and library3 test cases in test_scalafmt()).

I also need to get test_lint.sh working again, and update the dt_patches repos for Bzlmod. But the following all pass:

test_rules_scala.sh
test_reproducibility.sh
test_examples.sh
test_coverage.sh

Hopefully I can get these fixed up today, and I'll start peeling off the next pull request or two. And thanks to @simuons and @liucijus for reviewing and merging #1619 and #1620 already.

liucijus · 2024-10-11T17:37:28Z

I need to replace the bind() calls from twitter_scrooge with alias() or some such to get test_version.sh to pass.

If I recall correctly, scrooge rules support dep configuration via toolchain. I think we should purge the outdated use of bind() in favor of using toolchains. alias() creates new label in the current workspace, it won't allow to replace external names.

mbland · 2024-10-11T22:54:31Z

BTW, I ended up getting ./test_lint.sh back in shape, and got all the dt_patches stuff working with Bzlmod, except for one test case when passing in a label to a compiler srcjar. Try bazel build //... --repo_env=SCALA_VERSION=2.12.16 //... inside dt_patches/test_dt_patches_user_srcjar to see what I mean.

FWIW, the test_compiler_patch 2.13.0 case from dt_patches/dt_test_patches.sh also fails on master under WORKSPACE. So I wonder if it's worth commenting out that case.

After that, I think it's just the bind() and Scala 3 + Scalafmt issues I mentioned earlier that are standing in the way. Hoping to tackle those this weekend.

BillyAutrey · 2024-10-12T00:16:44Z

After that, I think it's just the bind() and Scala 3 + Scalafmt issues I mentioned earlier that are standing in the way. Hoping to tackle those this weekend.

Let me know if this becomes an issue, I'm coincidentally dealing with related fun for other crossbuild reasons.

mbland · 2024-10-12T23:22:19Z

Got everything working now except the test/shell/test_cross_build.sh failures for Scala 3 + Scalafmt.

I also fixed the test_compiler_patch 2.13.0 case from dt_patches/dt_test_patches.sh, FWIW.

@liucijus I can see a path to ripping out the bind() calls while maintaining WORKSPACE compatibility, if you'd like me to try that. I'm not currently planning to do it in my bzlmod working branch, but could do it when I create a pull request to Bzlmodify twitter_scrooge specifically.

agluszak · 2024-10-13T11:27:38Z

You're a hero, @mbland!

agluszak · 2024-10-13T17:56:34Z

Btw I really enjoy reading your commit messages in https://github.com/mbland/rules_scala/commits/bzlmod/ :)

sfc-gh-pbennes mentioned this issue Mar 16, 2023

wanted: bazelbuild/rules_scala bazelbuild/bazel-central-registry#522

Open

1 task

liucijus added the help wanted label Mar 17, 2023

mateuszkuta256 linked a pull request Jan 24, 2024 that will close this issue

Draft: Add bzlmod minimal support #1541

Draft

mbland mentioned this issue Apr 15, 2024

Convert WORKSPACE to MODULE.bazel, update README EngFlow/example#312

Merged

agluszak mentioned this issue Apr 22, 2024

Migrate from sbt to bazel build-server-protocol/build-server-protocol#615

Merged

agluszak linked a pull request Aug 7, 2024 that will close this issue

Draft: Add bzlmod minimal support (continuation) #1595

Draft

mbland mentioned this issue Oct 2, 2024

Transfer rules_scala to bazel-contrib #1616

Open

4 tasks

This was referenced Oct 3, 2024

Bump Bazel and dependencies to prepare for Bzlmod #1618

Closed

Bazel 7 compatibility updates #1619

Merged

mbland mentioned this issue Oct 5, 2024

Fix Bazel 7 related protobuf build failures #1620

Merged

liucijus closed this as completed in #1619 Oct 8, 2024

simuons reopened this Oct 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Bzlmod and add rules_scala to bazel-central-registry #1482

Support Bzlmod and add rules_scala to bazel-central-registry #1482

sfc-gh-pbennes commented Mar 16, 2023 •

edited

Loading

chrislovecnm commented Jul 11, 2023

sluongng commented Jul 11, 2023

chrislovecnm commented Jul 11, 2023 •

edited

Loading

sluongng commented Jul 11, 2023

mateuszkuta256 commented Jan 24, 2024

pcj commented Jun 30, 2024 •

edited

Loading

mateuszkuta256 commented Jul 1, 2024

mbland commented Oct 2, 2024

mbland commented Oct 11, 2024

liucijus commented Oct 11, 2024

mbland commented Oct 11, 2024

BillyAutrey commented Oct 12, 2024

mbland commented Oct 12, 2024

agluszak commented Oct 13, 2024

agluszak commented Oct 13, 2024

Support Bzlmod and add rules_scala to bazel-central-registry #1482

Support Bzlmod and add rules_scala to bazel-central-registry #1482

Comments

sfc-gh-pbennes commented Mar 16, 2023 • edited Loading

chrislovecnm commented Jul 11, 2023

sluongng commented Jul 11, 2023

chrislovecnm commented Jul 11, 2023 • edited Loading

sluongng commented Jul 11, 2023

mateuszkuta256 commented Jan 24, 2024

pcj commented Jun 30, 2024 • edited Loading

mateuszkuta256 commented Jul 1, 2024

mbland commented Oct 2, 2024

mbland commented Oct 11, 2024

liucijus commented Oct 11, 2024

mbland commented Oct 11, 2024

BillyAutrey commented Oct 12, 2024

mbland commented Oct 12, 2024

agluszak commented Oct 13, 2024

agluszak commented Oct 13, 2024

sfc-gh-pbennes commented Mar 16, 2023 •

edited

Loading

chrislovecnm commented Jul 11, 2023 •

edited

Loading

pcj commented Jun 30, 2024 •

edited

Loading