Skip to content

Add tools/sync-bazelversion.sh, commit results #1629

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

Merged
merged 2 commits into from
Oct 24, 2024

Conversation

mbland
Copy link
Contributor

@mbland mbland commented Oct 23, 2024

Description

Ensures Bazelisk uses the same Bazel version in every workspace in the project. Part of #1482.

The sync-bazelversion.sh script is currently in tools/. #1608 introduced the scripts/ directory, and I'm happy to move it there, but that directory seems focused on third_party/repositories/scala_*.bzl update automation.

Motivation

I discovered that the workspaces under the main workspace were running the latest mainline Blaze release I had installed, instead of 6.5.0 from .bazelversion. Now all the workspaces are synchronized on the same Bazel version.

As noted in the tools/sync-bazelversion.sh comments, symlinks or an import ../.bazelversion mechanism would be preferable. However, the former can surprise Windows users, and Bazelisk doesn't support the latter.

Ensures Bazelisk uses the same Bazel version in every workspace in the
project. Part of bazel-contrib#1482.

I discovered that the workspaces under the main workspace were running
the latest mainline Blaze release I had installed, instead of 6.5.0 from
`.bazelversion`. Now all the workspaces are synchronized on the same
Bazel version.

As noted in the `tools/sync-bazelversion.sh` comments, symlinks or an
`import ../.bazelversion` mechanism would be preferable. However, the
former can surprise Windows users, and Bazelisk doesn't support the
latter.
mbland added a commit to mbland/rules_scala that referenced this pull request Oct 23, 2024
Updates targets broken under the Scala 2.13.0 test. Also removes the
redundant 2.12.1 test, and cleans the `test_dt_patches_user_srcjar` repo
before the `test_compiler_srcjar{,_nonhermetic}` tests. Part of bazel-contrib#1482.

This fix and the `.bazelversion` sync from bazel-contrib#1629 can land in either
order. Both are required for `dt_patches/dt_patch_test.sh` to be fully
functional.

---

The equivalent standalone command in `dt_patches/test_dt_patches`
produced the error:

```txt
$ bazel build //... --repo_env=SCALA_VERSION=2.13.0 //...

ERROR: .../rules_scala~/src/java/io/bazel/rulesscala/scalac/reporter/BUILD:5:13:
  Building external/rules_scala~/src/java/io/bazel/rulesscala/scalac/reporter/libreporter.jar
  (2 source files) [for tool] failed: (Exit 1): java failed:
  error executing Javac command
  (from target @@rules_scala~//src/java/io/bazel/rulesscala/scalac/reporter:reporter)
  external/rules_java~~toolchains~remotejdk21_macos_aarch64/bin/java
    '--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED'
    '--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED' ...
    (remaining 19 arguments skipped)

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/deps_tracking_reporter/
  after_2_12_13_and_before_2_13_12/DepsTrackingReporter.java:85:
  error: method does not override or implement a method from a supertype
  @OverRide
  ^

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/deps_tracking_reporter/
  after_2_12_13_and_before_2_13_12/DepsTrackingReporter.java:94:
  error: cannot find symbol
    ((FilteringReporter) delegateReporter).doReport(pos, msg, severity);
                                          ^
  symbol:   method doReport(Position,String,Reporter.Severity)
  location: interface FilteringReporter

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/deps_tracking_reporter/after_2_12_13_and_before_2_13_12/DepsTrackingReporter.java:99:
  error: cannot find symbol
    super.doReport(pos, msg, severity);
         ^
  symbol: method doReport(Position,String,Reporter.Severity)

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/reporter/after_2_12_13_and_before_2_13_12/ProtoReporter.java:49:
  error: method does not override or implement a method from a supertype
    @OverRide
    ^

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/reporter/after_2_12_13_and_before_2_13_12/ProtoReporter.java:51:
  error: cannot find symbol
    super.doReport(pos, msg, severity);
         ^
  symbol: method doReport(Position,String,Reporter.Severity)

Target //dummy:dummy failed to build
```

I had the thought that maybe 2.13.0 could use the same `srcs` as the
`before_2_12_13` targets. So I abused the `any` matcher to pick exactly
2.13.0 and assign it the same values as `before_2_12_13. Now it works,
and `dt_patches/dt_patch_test.sh` fully passes.

---

Cleaning the `test_dt_patches_user_srcjar` repository helps ensure that
the `test_compiler_srcjar_nonhermetic` runs, in particular, don't fail
after the first run.
mbland added a commit to mbland/rules_scala that referenced this pull request Oct 23, 2024
Updates targets broken under the Scala 2.13.0 test. Also removes the
redundant 2.12.1 test, and cleans the `test_dt_patches_user_srcjar` repo
before the `test_compiler_srcjar{,_nonhermetic}` tests. Part of bazel-contrib#1482.

This fix and the `.bazelversion` sync from bazel-contrib#1629 can land in either
order. Both are required for `dt_patches/dt_patch_test.sh` to be fully
functional.

---

The equivalent standalone command in `dt_patches/test_dt_patches`
produced the error:

```txt
$ bazel build //... --repo_env=SCALA_VERSION=2.13.0 //...

ERROR: .../rules_scala~/src/java/io/bazel/rulesscala/scalac/reporter/BUILD:5:13:
  Building external/rules_scala~/src/java/io/bazel/rulesscala/scalac/reporter/libreporter.jar
  (2 source files) [for tool] failed: (Exit 1): java failed:
  error executing Javac command
  (from target @@rules_scala~//src/java/io/bazel/rulesscala/scalac/reporter:reporter)
  external/rules_java~~toolchains~remotejdk21_macos_aarch64/bin/java
    '--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED'
    '--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED' ...
    (remaining 19 arguments skipped)

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/deps_tracking_reporter/
  after_2_12_13_and_before_2_13_12/DepsTrackingReporter.java:85:
  error: method does not override or implement a method from a supertype
  @OverRide
  ^

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/deps_tracking_reporter/
  after_2_12_13_and_before_2_13_12/DepsTrackingReporter.java:94:
  error: cannot find symbol
    ((FilteringReporter) delegateReporter).doReport(pos, msg, severity);
                                          ^
  symbol:   method doReport(Position,String,Reporter.Severity)
  location: interface FilteringReporter

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/deps_tracking_reporter/after_2_12_13_and_before_2_13_12/DepsTrackingReporter.java:99:
  error: cannot find symbol
    super.doReport(pos, msg, severity);
         ^
  symbol: method doReport(Position,String,Reporter.Severity)

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/reporter/after_2_12_13_and_before_2_13_12/ProtoReporter.java:49:
  error: method does not override or implement a method from a supertype
    @OverRide
    ^

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/reporter/after_2_12_13_and_before_2_13_12/ProtoReporter.java:51:
  error: cannot find symbol
    super.doReport(pos, msg, severity);
         ^
  symbol: method doReport(Position,String,Reporter.Severity)

Target //dummy:dummy failed to build
```

I had the thought that maybe 2.13.0 could use the same `srcs` as the
`before_2_12_13` targets. So I abused the `any` matcher to pick exactly
2.13.0 and assign it the same values as `before_2_12_13`. Now it works,
and `dt_patches/dt_patch_test.sh` fully passes.

---

Cleaning the `test_dt_patches_user_srcjar` repository helps ensure that
the `test_compiler_srcjar_nonhermetic` runs, in particular, don't fail
after the first run.
Copy link
Collaborator

@simuons simuons left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, @mbland, thanks for PR!

So tools/scripts is somewhat confusing and to be honest I'm not 100% sure about difference.

Just to note: scripts existed before #1608

After looking at content of each directory, I think tools contain something that is used during bazel build or can be invoked via bazel run. While scripts holds anything else (most likely automate repetitive tasks).

I'm not sure such distinction is needed. We gonna deal with it later.

So I you don't mind I'll ask to move sync-bazelversion.sh to scripts/

Anything else looks awesome.

@mbland
Copy link
Contributor Author

mbland commented Oct 24, 2024

Whoops, you're right—a git log scripts/ could've told me it was there before #1608. If I'd've thought to check that before, that's where I would've put it to begin with. D'oh!

Just pushed a new commit to move it to scripts/.

mbland added a commit to mbland/rules_scala that referenced this pull request Oct 24, 2024
Updates targets broken under the Scala 2.13.0 test. Also removes the
redundant 2.12.1 test, and cleans the `test_dt_patches_user_srcjar` repo
before the `test_compiler_srcjar{,_nonhermetic}` tests. Part of bazel-contrib#1482.

This fix and the `.bazelversion` sync from bazel-contrib#1629 can land in either
order. Both are required for `dt_patches/dt_patch_test.sh` to be fully
functional.

---

The equivalent standalone command in `dt_patches/test_dt_patches`
produced the error:

```txt
$ bazel build //... --repo_env=SCALA_VERSION=2.13.0 //...

ERROR: .../rules_scala~/src/java/io/bazel/rulesscala/scalac/reporter/BUILD:5:13:
  Building external/rules_scala~/src/java/io/bazel/rulesscala/scalac/reporter/libreporter.jar
  (2 source files) [for tool] failed: (Exit 1): java failed:
  error executing Javac command
  (from target @@rules_scala~//src/java/io/bazel/rulesscala/scalac/reporter:reporter)
  external/rules_java~~toolchains~remotejdk21_macos_aarch64/bin/java
    '--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED'
    '--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED' ...
    (remaining 19 arguments skipped)

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/deps_tracking_reporter/
  after_2_12_13_and_before_2_13_12/DepsTrackingReporter.java:85:
  error: method does not override or implement a method from a supertype
  @OverRide
  ^

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/deps_tracking_reporter/
  after_2_12_13_and_before_2_13_12/DepsTrackingReporter.java:94:
  error: cannot find symbol
    ((FilteringReporter) delegateReporter).doReport(pos, msg, severity);
                                          ^
  symbol:   method doReport(Position,String,Reporter.Severity)
  location: interface FilteringReporter

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/deps_tracking_reporter/after_2_12_13_and_before_2_13_12/DepsTrackingReporter.java:99:
  error: cannot find symbol
    super.doReport(pos, msg, severity);
         ^
  symbol: method doReport(Position,String,Reporter.Severity)

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/reporter/after_2_12_13_and_before_2_13_12/ProtoReporter.java:49:
  error: method does not override or implement a method from a supertype
    @OverRide
    ^

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/reporter/after_2_12_13_and_before_2_13_12/ProtoReporter.java:51:
  error: cannot find symbol
    super.doReport(pos, msg, severity);
         ^
  symbol: method doReport(Position,String,Reporter.Severity)

Target //dummy:dummy failed to build
```

I had the thought that maybe 2.13.0 could use the same `srcs` as the
`before_2_12_13` targets. So I abused the `any` matcher to pick exactly
2.13.0 and assign it the same values as `before_2_12_13`. Now it works,
and `dt_patches/dt_patch_test.sh` fully passes.

---

Cleaning the `test_dt_patches_user_srcjar` repository helps ensure that
the `test_compiler_srcjar_nonhermetic` runs, in particular, don't fail
after the first run.
@simuons simuons merged commit 6a23700 into bazel-contrib:master Oct 24, 2024
2 checks passed
mbland added a commit to mbland/rules_scala that referenced this pull request Oct 24, 2024
Updates targets broken under the Scala 2.13.0 test. Also removes the
redundant 2.12.1 test, and cleans the `test_dt_patches_user_srcjar` repo
before the `test_compiler_srcjar{,_nonhermetic}` tests. Part of bazel-contrib#1482.

This fix and the `.bazelversion` sync from bazel-contrib#1629 can land in either
order. Both are required for `dt_patches/dt_patch_test.sh` to be fully
functional.

---

The equivalent standalone command in `dt_patches/test_dt_patches`
produced the error:

```txt
$ bazel build //... --repo_env=SCALA_VERSION=2.13.0 //...

ERROR: .../rules_scala~/src/java/io/bazel/rulesscala/scalac/reporter/BUILD:5:13:
  Building external/rules_scala~/src/java/io/bazel/rulesscala/scalac/reporter/libreporter.jar
  (2 source files) [for tool] failed: (Exit 1): java failed:
  error executing Javac command
  (from target @@rules_scala~//src/java/io/bazel/rulesscala/scalac/reporter:reporter)
  external/rules_java~~toolchains~remotejdk21_macos_aarch64/bin/java
    '--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED'
    '--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED' ...
    (remaining 19 arguments skipped)

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/deps_tracking_reporter/
  after_2_12_13_and_before_2_13_12/DepsTrackingReporter.java:85:
  error: method does not override or implement a method from a supertype
  @OverRide
  ^

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/deps_tracking_reporter/
  after_2_12_13_and_before_2_13_12/DepsTrackingReporter.java:94:
  error: cannot find symbol
    ((FilteringReporter) delegateReporter).doReport(pos, msg, severity);
                                          ^
  symbol:   method doReport(Position,String,Reporter.Severity)
  location: interface FilteringReporter

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/deps_tracking_reporter/after_2_12_13_and_before_2_13_12/DepsTrackingReporter.java:99:
  error: cannot find symbol
    super.doReport(pos, msg, severity);
         ^
  symbol: method doReport(Position,String,Reporter.Severity)

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/reporter/after_2_12_13_and_before_2_13_12/ProtoReporter.java:49:
  error: method does not override or implement a method from a supertype
    @OverRide
    ^

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/reporter/after_2_12_13_and_before_2_13_12/ProtoReporter.java:51:
  error: cannot find symbol
    super.doReport(pos, msg, severity);
         ^
  symbol: method doReport(Position,String,Reporter.Severity)

Target //dummy:dummy failed to build
```

I had the thought that maybe 2.13.0 could use the same `srcs` as the
`before_2_12_13` targets. So I abused the `any` matcher to pick exactly
2.13.0 and assign it the same values as `before_2_12_13`. Now it works,
and `dt_patches/dt_patch_test.sh` fully passes.

---

Cleaning the `test_dt_patches_user_srcjar` repository helps ensure that
the `test_compiler_srcjar_nonhermetic` runs, in particular, don't fail
after the first run.
@mbland mbland deleted the bzlmod-sync-bazelversion branch October 24, 2024 19:08
mbland added a commit to mbland/rules_scala that referenced this pull request Oct 25, 2024
Updates targets broken under the Scala 2.13.0 test. Also removes the
redundant 2.12.1 test, and cleans the `test_dt_patches_user_srcjar` repo
before the `test_compiler_srcjar{,_nonhermetic}` tests. Part of bazel-contrib#1482.

This fix and the `.bazelversion` sync from bazel-contrib#1629 can land in either
order. Both are required for `dt_patches/dt_patch_test.sh` to be fully
functional.

---

The equivalent standalone command in `dt_patches/test_dt_patches`
produced the error:

```txt
$ bazel build //... --repo_env=SCALA_VERSION=2.13.0 //...

ERROR: .../rules_scala~/src/java/io/bazel/rulesscala/scalac/reporter/BUILD:5:13:
  Building external/rules_scala~/src/java/io/bazel/rulesscala/scalac/reporter/libreporter.jar
  (2 source files) [for tool] failed: (Exit 1): java failed:
  error executing Javac command
  (from target @@rules_scala~//src/java/io/bazel/rulesscala/scalac/reporter:reporter)
  external/rules_java~~toolchains~remotejdk21_macos_aarch64/bin/java
    '--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED'
    '--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED' ...
    (remaining 19 arguments skipped)

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/deps_tracking_reporter/
  after_2_12_13_and_before_2_13_12/DepsTrackingReporter.java:85:
  error: method does not override or implement a method from a supertype
  @OverRide
  ^

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/deps_tracking_reporter/
  after_2_12_13_and_before_2_13_12/DepsTrackingReporter.java:94:
  error: cannot find symbol
    ((FilteringReporter) delegateReporter).doReport(pos, msg, severity);
                                          ^
  symbol:   method doReport(Position,String,Reporter.Severity)
  location: interface FilteringReporter

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/deps_tracking_reporter/after_2_12_13_and_before_2_13_12/DepsTrackingReporter.java:99:
  error: cannot find symbol
    super.doReport(pos, msg, severity);
         ^
  symbol: method doReport(Position,String,Reporter.Severity)

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/reporter/after_2_12_13_and_before_2_13_12/ProtoReporter.java:49:
  error: method does not override or implement a method from a supertype
    @OverRide
    ^

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/reporter/after_2_12_13_and_before_2_13_12/ProtoReporter.java:51:
  error: cannot find symbol
    super.doReport(pos, msg, severity);
         ^
  symbol: method doReport(Position,String,Reporter.Severity)

Target //dummy:dummy failed to build
```

I had the thought that maybe 2.13.0 could use the same `srcs` as the
`before_2_12_13` targets. So I abused the `any` matcher to pick exactly
2.13.0 and assign it the same values as `before_2_12_13`. Now it works,
and `dt_patches/dt_patch_test.sh` fully passes.

---

Cleaning the `test_dt_patches_user_srcjar` repository helps ensure that
the `test_compiler_srcjar_nonhermetic` runs, in particular, don't fail
after the first run.
mbland added a commit to mbland/rules_scala that referenced this pull request Oct 26, 2024
Missed this one in third_party/test/proto earlier. Also removed
unnecessary `USE_BAZEL_VERSION` expression in
test_scala_proto_library.sh. If `USE_BAZEL_VERSION` is set, it takes
precedence to begin with, and third_party/test/proto/.bazelversion
exists now after bazel-contrib#1629.
mbland added a commit to mbland/rules_scala that referenced this pull request Oct 26, 2024
Missed this one in third_party/test/proto earlier. Also removed
unnecessary `USE_BAZEL_VERSION` expression in
test_scala_proto_library.sh. If `USE_BAZEL_VERSION` is set, it takes
precedence to begin with, and third_party/test/proto/.bazelversion
exists now after bazel-contrib#1629.
liucijus pushed a commit that referenced this pull request Oct 29, 2024
Updates targets broken under the Scala 2.13.0 test. Also removes the
redundant 2.12.1 test, and cleans the `test_dt_patches_user_srcjar` repo
before the `test_compiler_srcjar{,_nonhermetic}` tests. Part of #1482.

This fix and the `.bazelversion` sync from #1629 can land in either
order. Both are required for `dt_patches/dt_patch_test.sh` to be fully
functional.

---

The equivalent standalone command in `dt_patches/test_dt_patches`
produced the error:

```txt
$ bazel build //... --repo_env=SCALA_VERSION=2.13.0 //...

ERROR: .../rules_scala~/src/java/io/bazel/rulesscala/scalac/reporter/BUILD:5:13:
  Building external/rules_scala~/src/java/io/bazel/rulesscala/scalac/reporter/libreporter.jar
  (2 source files) [for tool] failed: (Exit 1): java failed:
  error executing Javac command
  (from target @@rules_scala~//src/java/io/bazel/rulesscala/scalac/reporter:reporter)
  external/rules_java~~toolchains~remotejdk21_macos_aarch64/bin/java
    '--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED'
    '--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED' ...
    (remaining 19 arguments skipped)

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/deps_tracking_reporter/
  after_2_12_13_and_before_2_13_12/DepsTrackingReporter.java:85:
  error: method does not override or implement a method from a supertype
  @OverRide
  ^

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/deps_tracking_reporter/
  after_2_12_13_and_before_2_13_12/DepsTrackingReporter.java:94:
  error: cannot find symbol
    ((FilteringReporter) delegateReporter).doReport(pos, msg, severity);
                                          ^
  symbol:   method doReport(Position,String,Reporter.Severity)
  location: interface FilteringReporter

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/deps_tracking_reporter/after_2_12_13_and_before_2_13_12/DepsTrackingReporter.java:99:
  error: cannot find symbol
    super.doReport(pos, msg, severity);
         ^
  symbol: method doReport(Position,String,Reporter.Severity)

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/reporter/after_2_12_13_and_before_2_13_12/ProtoReporter.java:49:
  error: method does not override or implement a method from a supertype
    @OverRide
    ^

external/rules_scala~/src/java/io/bazel/rulesscala/scalac/reporter/after_2_12_13_and_before_2_13_12/ProtoReporter.java:51:
  error: cannot find symbol
    super.doReport(pos, msg, severity);
         ^
  symbol: method doReport(Position,String,Reporter.Severity)

Target //dummy:dummy failed to build
```

I had the thought that maybe 2.13.0 could use the same `srcs` as the
`before_2_12_13` targets. So I abused the `any` matcher to pick exactly
2.13.0 and assign it the same values as `before_2_12_13`. Now it works,
and `dt_patches/dt_patch_test.sh` fully passes.

---

Cleaning the `test_dt_patches_user_srcjar` repository helps ensure that
the `test_compiler_srcjar_nonhermetic` runs, in particular, don't fail
after the first run.
mbland added a commit to mbland/rules_scala that referenced this pull request Oct 29, 2024
Missed this one in third_party/test/proto earlier. Also removed
unnecessary `USE_BAZEL_VERSION` expression in
test_scala_proto_library.sh. If `USE_BAZEL_VERSION` is set, it takes
precedence to begin with, and third_party/test/proto/.bazelversion
exists now after bazel-contrib#1629.
mbland added a commit to mbland/rules_scala that referenced this pull request Nov 4, 2024
Missed this one in third_party/test/proto earlier. Also removed
unnecessary `USE_BAZEL_VERSION` expression in
test_scala_proto_library.sh. If `USE_BAZEL_VERSION` is set, it takes
precedence to begin with, and third_party/test/proto/.bazelversion
exists now after bazel-contrib#1629.
mbland added a commit to mbland/rules_scala that referenced this pull request Nov 11, 2024
Missed this one in third_party/test/proto earlier. Also removed
unnecessary `USE_BAZEL_VERSION` expression in
test_scala_proto_library.sh. If `USE_BAZEL_VERSION` is set, it takes
precedence to begin with, and third_party/test/proto/.bazelversion
exists now after bazel-contrib#1629.
mbland added a commit to mbland/rules_scala that referenced this pull request Nov 12, 2024
Missed this one in third_party/test/proto earlier. Also removed
unnecessary `USE_BAZEL_VERSION` expression in
test_scala_proto_library.sh. If `USE_BAZEL_VERSION` is set, it takes
precedence to begin with, and third_party/test/proto/.bazelversion
exists now after bazel-contrib#1629.
liucijus pushed a commit that referenced this pull request Nov 27, 2024
* Toolchainize //scala:toolchain_type

Moves the `toolchain` targets for `//scala:toolchain_type` to a new
`@io_bazel_rules_scala_toolchains` repository as a step towards
Bzlmodification. Part of #1482.

Instantiating toolchains in their own repository enables module
extensions to define the the repositories required by those toolchains
within the extension's own namespace. Bzlmod users can then register the
toolchains from this repository without having to import all the
toolchains' dependencies into their own namespace via `use_repo()`.

---

The `scala_toolchains_repo()` macro wraps the underlying repository rule
and assigns it the standard name `io_bazel_rules_scala_toolchains`.
Right now it's only instantiating the main Scala toolchain via the
default `scala = True` parameter. Future changes will expand this
macro and repository rule with more boolean parameters to instantiate
other toolchains, specifically:

- `scalatest`
- `junit`
- `specs2`
- `twitter_scrooge`
- `jmh`
- `scala_proto` and `scala_proto_enable_all_options`
- `testing` (includes all of `scalatest`, `junit`, and `specs2`)
- `scalafmt`

---

`WORKSPACE` users will now have to import and call the
`scala_toolchains_repo()` macro to instantiate
`@io_bazel_rules_scala_toolchains`.

```py
load("@io_bazel_rules_scala//:scala_config.bzl", "scala_config")

scala_config()

load(
    "//scala:scala.bzl",
    "rules_scala_setup",
    "rules_scala_toolchain_deps_repositories",
    "scala_toolchains_repo",
)

rules_scala_setup()

rules_scala_toolchain_deps_repositories()

scala_toolchains_repo()

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

This is what the corresponding `MODULE.bazel` setup would look like:

```py
module(name = "rules_scala", version = "7.0.0")

scala_config = use_extension(
    "//scala/extensions:config.bzl", "scala_config"
)
scala_config.settings(scala_version = "2.13.14")

scala_deps = use_extension("//scala/extensions:deps.bzl", "scala_deps")
scala_deps.toolchains()
```

The `register_toolchains()` call in `WORKSPACE` isn't strictly required
at this point, but is recommended. However, all the `WORKSPACE` files in
this repo already register their required toolchains using existing
macros, which have been updated in this change.

In fact, calling `register_toolchains()` right after
`scala_toolchains_repo()` as shown above breaks two tests that depend on
the existing `WORKSPACE` toolchain registration:

- `test_compilation_fails_with_plus_one_deps_undefined` from
  `test/shell/test_compilation.sh` depends on
  `scala_register_unused_deps_toolchains()` setting up its toolchain to
  resolve first. `//scala:unused_dependency_checker_error_toolchain`
  sets the `scala_toolchain()` parameters `dependency_tracking_method =
  "ast-plus"` and `unused_dependency_checker_mode = "error"`, and the
  `@io_bazel_rules_scala_toolchains//scala` toolchains don't.

- `test_scala_binary_allows_opt_in_to_use_of_argument_file_in_runner_for_improved_performance`
  from `test/shell/test_scala_binary.sh` depends on the
  `use_argument_file_in_runner` parameter of `scala_toolchain` being
  `False`. This is the default, but the
  `@io_bazel_rules_scala_toolchains//scala` toolchains explicitly set
  this to `True` instead.

In the Bzlmod case, the `register_toolchains()` call isn't necessary at
all. This is because `@io_bazel_rules_scala_toolchains` includes one
package per set of toolchains, and the rules_scala `MODULE.bazel` calls
`register_toolchains("@io_bazel_rules_scala_toolchains//...:all")`. This
will automatically register all configured rules_scala toolchains, while
allowing users to override any of them using `register_toolchains()` in
their own `MODULE.bazel` files.

Technically, the `scala_deps.toolchains()` call isn't required when only
using the default `scala = True` parameter; the rules_scala
`MODULE.bazel` will instantiate this automatically, too.

* Add scala_toolchains macro for WORKSPACE, Bzlmod

Extracted a single `scala_toolchains` macro to share between `WORKSPACE`
and the `deps.bzl` module extension. This will make it easier to ensure
`WORKSPACE` compatibility, and to add a Bazel module extension as a thin
layer on top. Part of #1482.

This change includes updates to `rules_scala_setup` and
`scala_repositories` to support this, while preserving compatibility
with existing `WORKSPACE` calls. The next commit will replace many
existing `WORKSPACE` calls with `scala_toolchains`.

* Replace WORKSPACE calls with scala_toolchains()

Also added `@io_bazel_rules_scala_toolchains//...:all` to
`register_toolchains()` calls everywhere, even when not specifically
necessary. This proves the mechanism is safe and works with `WORKSPACE`
now, and will make future updates to consolidate other toolchains less
noisy.

* Remove obsolete WORKSPACE toolchain calls

Should've been included in the previous commit. All tests still pass.

* Replace scala_register_unused_deps_toolchains call

Missed this one in third_party/test/proto earlier. Also removed
unnecessary `USE_BAZEL_VERSION` expression in
test_scala_proto_library.sh. If `USE_BAZEL_VERSION` is set, it takes
precedence to begin with, and third_party/test/proto/.bazelversion
exists now after #1629.

* Give scala_toolchains_repo a default name argument

This turns out to be helpful when adapting
`test_version/WORKSPACE.template` to the toolchainized version of
`twitter_scrooge` for testing. In this case, we want `twitter_scooge` to
be in its own customized repo, separate from the one generated by
`scala_toolchains`.

This seems like it might be generally useful when writing module
extensions for alternative toolchains.

* Update Scala toolchainizaion per @simuons in #1633

- Removes an extraneous `compiler_sources_repo` call.

- `scala_toolchains` will now _always_ create the main Scala toolchain
  repository (there's no longer an option to turn it off).

- Registers `@io_bazel_rules_scala_toolchains//...:all` in
  `scala_register_toolchains.

* Extract load_rules_dependencies macro

Requested by @simuons in #1633 to make `rules_scala_setup` and
`scala_repositories` more readable while maintaining the existing APIs.
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.

2 participants