fix: replace string with modern providers#7606
Merged
ujohnny merged 1 commit intobazelbuild:masterfrom Apr 15, 2025
Merged
Conversation
comius
requested review from
LeFrosch,
agluszak,
mai93 and
ujohnny
as code owners
github-actions
bot
added
the
awaiting-review
comius
changed the title
PiperOrigin-RevId: 745510931
comius
force-pushed
the
fix-legacy-struct-providers
branch
from
8af2fd4 to
b8e5d58
Compare
LeFrosch
approved these changes
Apr 14, 2025
Collaborator
LeFrosch
left a comment
LeFrosch
left a comment
There was a problem hiding this comment.
Looks good to me, CC @sellophane
ujohnny
approved these changes
Apr 14, 2025
| output_groups = depset(output_files, transitive = dep_outputs) | ||
|
|
||
| return struct(output_groups = {"ide-fast-build": output_groups}) | ||
| return [OutputGroupInfo(**{"ide-fast-build": output_groups})] |
Collaborator
There was a problem hiding this comment.
I guess we can later fix this unconventional group name.
Contributor
Author
There was a problem hiding this comment.
Yes, the **{} is used, because there are minuses in the name.
github-project-automation
bot
moved this from Untriaged
to Done
in Bazel IntelliJ Plugin
github-actions
bot
removed
the
awaiting-review
copybara-service bot
pushed a commit
to bazelbuild/bazel
that referenced
this pull request
Apr 15, 2025
Downstream run: https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/4596 The only failure in IntelliJ was fixed in bazelbuild/intellij#7606 RELNOTES[INC]: struct providers are not supported in aspects PiperOrigin-RevId: 747836848 Change-Id: Iab2d2eccc455173c53ea160e0a15d90339e6bbb6
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
fmeum
pushed a commit
to fmeum/bazel
that referenced
this pull request
Apr 25, 2025
Downstream run: https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/4596 The only failure in IntelliJ was fixed in bazelbuild/intellij#7606 RELNOTES[INC]: struct providers are not supported in aspects PiperOrigin-RevId: 747836848 Change-Id: Iab2d2eccc455173c53ea160e0a15d90339e6bbb6
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.
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
3 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.
This is cherrypick of: 15bdabe
PiperOrigin-RevId: 745510931
Strings used to refer to legacy struct providers, which were removed from Bazel.
Legacy struct providers have been deprecated by Bazel. Replacing them with modern providers, will make it possible to simplify and remove legacy handling from Blaze.
The change is a no-op.
More information: bazelbuild/bazel#25836