Remove support for strings in attribute's "provides" parameters#25819
Closed
comius wants to merge 2 commits intobazelbuild:masterfrom
Closed
Remove support for strings in attribute's "provides" parameters#25819comius wants to merge 2 commits intobazelbuild:masterfrom
comius wants to merge 2 commits intobazelbuild:masterfrom
Conversation
Without legacy struct providers, we also can't be specifying them. RELNOTES[INC]: strings in attribute's "provides" parameters are no longer supported PiperOrigin-RevId: 741563954 Change-Id: I8f7a88dee9ca1f2d87f7e135cb45396d9a685588
mbland
added a commit
to mbland/rules_scala
that referenced
this pull request
Apr 16, 2025
Updates `_aspect_impl()` in `test/aspect/aspect.bzl` to return a new `VisitedInfo` provider instead of a `struct`. This fixes `//test/aspect:aspect_test` when building with the `last_green` bazel: - bazel-a91a6a77171c008598bdee53d144792c95211890-darwin-arm64 ```txt $ USE_BAZEL_VERSION=last_green \ bazel test --test_output=errors //test/aspect:aspect_test ERROR: testing/toolchain/BUILD:10:27: in //test/aspect:aspect.bzl%test_aspect aspect on testing_toolchain_deps rule //testing/toolchain:junit_classpath: Returning a struct from an aspect implementation function is deprecated. ERROR: Analysis of target '//testing/toolchain:junit_classpath' failed ERROR: Analysis of target '//test/aspect:aspect_test' failed; build aborted: Analysis failed ``` This `last_green` Bazel build contains the following commit, summarized as "Remove support for returning struct providers from aspects": - bazelbuild/bazel@07cddaf That commit is related to: - bazelbuild/bazel#25836 - bazelbuild/bazel#25819 - bazelbuild/intellij#7606
simuons
pushed a commit
to bazel-contrib/rules_scala
that referenced
this pull request
Apr 26, 2025
* Enable Bzlmod All but finishes #1482, save for publication of the first `rules_scala` release to the Bazel Central Registry. Closes #1625. The module extensions and underlying helper macros comprise the most significant part of the change. The extensions provide a thin layer over the existing `WORKSPACE` API. The `scala/private/macros/bzlmod.bzl` docstring explains the common pattern employed throughout. - `scala/extensions/config.bzl`: invokes `scala_config()` - `scala/extensions/deps.bzl`: invokes `scala_toolchains()` - `scala/extensions/protoc.bzl`: invokes `scala_protoc_toolchains()` - `scala/private/extensions/dev_deps.bzl`: internal testing dependencies - `scala/private/macros/bzlmod.bzl`: module extension helper macros Existing tests thoroughly validate `config.bzl`, `deps.bzl`, `dev_deps.bzl`, and `protoc.bzl`. `test/shell/test_bzlmod_macros.sh` tests the helpers from `scala/private/macros/bzlmod.bzl` using the new test files from `scala/private/macros/test/...`. Most of the new files are essentially boilerplate: - Adds `MODULE.bazel` files mirroring existing `WORKSPACE` configurations and copies of the `protobuf` toolchainization patch to the main repo and every nested repo. - Adds empty `WORKSPACE.bzlmod` files to ensure that Bzlmod won't evaluate the `WORKSPACE` files. These changes update CI: - Sets `--noenable_workspace` in `.bazelrc` and `tools/bazel.rc.buildkite`. - Reenables the `last_green` Bazel build in `.bazelci/presubmit.yml`. - Replaces `//tools:lint_check` in CI with `test_lint.sh`, which now runs `test/shell/test_bzlmod_tidy.sh` to lint `MODULE.bazel` files. All other changes: - Adds `.bazelignore` to work around bazelbuild/bazel#22208. - Adds `.bcr/` files for publishing to the Bazel Central Registry using the Publish to BCR GitHub app. See `.bcr/README.md`. - Adds `assert_matches` to `test/shell/test_helper.sh` for use in `test/shell/test_bzlmod_macros.sh`. - `test/shell/test_bzlmod_macros.sh` introduces a mechanism for automatically finding and skipping tests, documented in the comment at the bottom of the file. - Adds `bazel mod tidy` checks to `test_lint.sh` and `test_version.sh`. - Adds `bazel clean --expunge_async` to tests that create temporary repos. - Adds the `test_cleanup.sh` script to reclaim disk space. --- This change enables Bazel 7 and 8 users to migrate their `rules_scala` dependency from `WORKSPACE` to `MODULE.bazel` whenever they're ready. It also ensures that new changes are compatible with the `last_green` build of Bazel, and enables publishing releases to the Bazel Central Registry. Per the "Publish to BCR" workflow setup instructions linked from `.bcr/README.md`, we need to: - create a fork of `bazelbuild/bazel-central-registry` (presumably `simuons/bazel-central-registry`) - configure the "Publish to BCR" app for both that fork and `bazelbuild/rules_scala` Once these items are ready, we can push a release tag to publish the first `rules_scala` Bazel module to https://registry.bazel.build/. At that point, we can close #1482. * Update test aspect to fix last_green build Updates `_aspect_impl()` in `test/aspect/aspect.bzl` to return a new `VisitedInfo` provider instead of a `struct`. This fixes `//test/aspect:aspect_test` when building with the `last_green` bazel: - bazel-a91a6a77171c008598bdee53d144792c95211890-darwin-arm64 ```txt $ USE_BAZEL_VERSION=last_green \ bazel test --test_output=errors //test/aspect:aspect_test ERROR: testing/toolchain/BUILD:10:27: in //test/aspect:aspect.bzl%test_aspect aspect on testing_toolchain_deps rule //testing/toolchain:junit_classpath: Returning a struct from an aspect implementation function is deprecated. ERROR: Analysis of target '//testing/toolchain:junit_classpath' failed ERROR: Analysis of target '//test/aspect:aspect_test' failed; build aborted: Analysis failed ``` This `last_green` Bazel build contains the following commit, summarized as "Remove support for returning struct providers from aspects": - bazelbuild/bazel@07cddaf That commit is related to: - bazelbuild/bazel#25836 - bazelbuild/bazel#25819 - bazelbuild/intellij#7606 * Change `**/protobuf.patch` to a symlink Replaces all the `protobuf.patch` instances in every nested module with a symlink to `protoc/0001-protobuf-19679-rm-protoc-dep.patch`. This reduces the duplication of the identical `protobuf` patch everywhere. I'd originally copied the file because: - Bzlmod requires that patches reside in the same repo. It does not allow target labels to other repositories. - Target labels cannot contain a relative path. - Windows historically doesn't allow non-admin users to create symlinks. However, Windows should allow symlinks now if permissions are set properly, and the Bazel CI system seems to set them: - https://stackoverflow.com/questions/5917249/git-symbolic-links-in-windows/59761201#59761201 - https://github.com/bazelbuild/continuous-integration/blob/028b90080584ad1fbc600e647a1b801337c0abf3/buildkite/startup-windows-pdssd.ps1#L56-L60 * Source `test/shell/test_bzlmod_*.sh` Removes `trap` statements to allow running `test_bzlmod_scala.sh` by sourcing it from `test_rules_scala.sh`. Also removes `trap` statement from `test_bzlmod_tidy.sh` since it's not really necessary. This is an attempt to ensure the `test_bzlmod_scala.sh` test run on all the `test_rules_scala.sh` CI runs on different platforms. I'd noticed that the `test_bzlmod_scala.sh` tests were only executing on macOS runs of `test_rules_scala.sh` in: - https://buildkite.com/bazel/rules-scala-scala/builds/5538 `test_bzlmod_scala.sh` was the only script that `test_rules_scala.sh` ran normally instead of sourcing it. I'd originally set it up that way because the `trap` statements would cause later `test_rules_scala.sh` tests to fail. * Escape '{', '}' in `test/shell/test_bzlmod_macros` It turns out that escaping `{` and `}` in Bash regular expressions isn't necessary on macOS, but is necessary on Linux. Locally, on macOS 15.4.1, I'm running Bash 5.2.37(1)-release. I built the same ubuntu2004 image from bazelbuild/continuous-integration locally, and it uses Bash 5.0.17(1)-release. I can't find a definitive entry in https://tiswww.case.edu/php/chet/bash/CHANGES accounting for the difference. Different underlying regex library implementations, perhaps? * Fix `test_bzlmod_macros` on Windows, Java 21 build Copies `tools/bazel.rc` to the test repos generated by `test_bzlmod_macros` to fix the `test_rules_scala_jdk21` CI job. Updates `_print_error_msg()` to fix Windows error output, and uses relative paths with `local_path_override` to fix `test_rules_scala_win`. Escaping `}` and `{` characters in the previous commit allowed the `test_bzlmod_macros` tests to run on Linux and Windows. Then `test_rules_scala_jdk21` failed with: ```txt [FATAL 17:39:54.401 src/main/cpp/blaze.cc:1105] Unexpected error reading config file '/workdir/tmp/test_bzlmod_macros/tools/bazel.rc': (error: 2): No such file or directory" Test "test_bzlmod_single_tag_values_returns_defaults_when_no_root_tag" failed (0 sec) ``` `test_rules_scala_win` failed with a malformed looking error message: ```txt ERROR: <builtin>: fetching local_repository rule //:rules_scala+: java.io.IOException: Could not create symlink to repository "/c/b/bk-windows-j20f/bazel/rules-scala-scala" (absolute path: "/c/b/bk-windows-j20f/bazel/rules-scala-scala"): Cannot create junction (name=C: ools\msys64\hom\_bazel_b ules_scala+, target= Test "test_bzlmod_single_tag_values_returns_defaults_when_no_root_tag" failed (3 sec) ``` Recreating this in a local Windows VM revealed a problem with `_print_error_message()` that also mangled local error messages: ```txt Value did not match regular expression Expected: "foo bar baz$" test/shell/test_helper.sh: line 148: printf: missing unicode digit for \u Actual: "Computing main repo mapping: ERROR: <builtin>: fetching local_repository rule //:rules_scala+: java.io.IOException: Could not create symlink to repository "/c/Users/msb/src/bazelbuild/rules_ules_scala+, target=h: "/c/Users/msb/src/bazelbuild/rules_scala"): Cannot create junction (name=C:\users\msb\_bazel_msb\ieysfhucternal Test "test_bzlmod_single_tag_values_returns_defaults_when_no_root_tag" failed (0 sec) ``` Fixing it (changing the `%b` specifier for `printf` to `%s`) revealed a more comprehensible error: ```txt Value did not match regular expression Expected: "foo bar baz$" Actual: "Computing main repo mapping: ERROR: <builtin>: fetching local_repository rule //:rules_scala+: java.io.IOException: Could not create symlink to repository "/c/Users/msb/src/bazelbuild/rules_scala" (absolute path: "/c/Users/msb/src/bazelbuild/rules_scala"): Cannot create junction (name=C:\users\msb\_bazel_msb\ieysfhuc\external\rules_scala+, target=\c\Users\msb\src\bazelbuild\rules_scala): ERROR: src/main/native/windows/file-jni.cc(122): nativeCreateJunction( \\?\C:\users\msb\_bazel_msb\ieysfhuc\external\rules_scala+, \\?\\c\Users\msb\src\bazelbuild\rules_scala): ERROR: src/main/native/windows/file.cc(231): CreateJunction(\\?\\c\Users\msb\src\bazelbuild\rules_scala): expected an absolute Windows path for junction_target ERROR: Error computing the main repository mapping: error during computation of main repo mapping: Could not create symlink to repository "/c/Users/msb/src/bazelbuild/rules_scala" (absolute path: "/c/Users/msb/src/bazelbuild/rules_scala"): Cannot create junction (name=C:\users\msb\_bazel_msb\ieysfhuc\external\rules_scala+, target=\c\Users\msb\src\bazelbuild\rules_scala): ERROR: src/main/native/windows/file-jni.cc(122): nativeCreateJunction( \\?\C:\users\msb\_bazel_msb\ieysfhuc\external\rules_scala+, \\?\\c\Users\msb\src\bazelbuild\rules_scala): ERROR: src/main/native/windows/file.cc(231): CreateJunction(\\?\\c\Users\msb\src\bazelbuild\rules_scala): expected an absolute Windows path for junction_target" Test "test_bzlmod_single_tag_values_returns_defaults_when_no_root_tag" failed (0 sec) ``` This shows clearly that the MSYS2 mapping of `C:\` paths to `/c/` breaks the underlying symlink operation: - https://github.com/bazelbuild/bazel/blob/release-7.6.0/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java#L508-L517 The fix was to use relative paths instead of absolute paths in the `local_path_override` directives of the generated `MODULE.bazel` files. * Remove unnecessary `WORKSPACE.bzlmod` files Setting `--noenable_workspace` in `.bazelrc` suffices in Bazel 7, and Bazel 8 defaults to it anyway. * Replace `scala_deps.toolchains` with tag classes Replaces the `scala_deps.toolchains` tag class, which contained boolean parameters for selecting toolchains, with a series of individual tag classes for each toolchain. Suggested by @simuons in the following comment as an alternative implementation that could allow for adding attributes to each toolchain's tag class. - #1722 (comment) We already had tag classes for `scalafmt`, `scala_proto`, and `twitter_scrooge` options. Now the presence of these tag classes, along with the tag classes added in this commit, translates to a `True` argument for `scala_toolchains()`. * Fix `scala_deps.toolchains` docs and comments Should've been part of the previous commit, but I missed them.
Member
|
has been merged as 8fe29f0 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Without legacy struct providers, we also can't be specifying them.
RELNOTES[INC]: strings in attribute's "provides" parameters are no longer supported
PiperOrigin-RevId: 741563954
Change-Id: I8f7a88dee9ca1f2d87f7e135cb45396d9a685588