-
Notifications
You must be signed in to change notification settings - Fork 6
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
Fix dependencies for all supported bazel versions #332
Merged
martis42
merged 11 commits into
main
from
fix_dependencies_for_all_supported_bazel_versions
Jan 26, 2025
Merged
Fix dependencies for all supported bazel versions #332
martis42
merged 11 commits into
main
from
fix_dependencies_for_all_supported_bazel_versions
Jan 26, 2025
Conversation
This file contains 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
Bazel 8 no longer contains builtin symbol `PyInfo` and others. `rules_python` 0.37.0 is the first version not depending on this being a builtin symbol. We use the latest patch version to get important fixes as well. Although, our tests used Bazel 8 already for some time, the issue went undetected as `bazel_tools` pulled newer versions of our dependencies into the test setup without us noticing. This change requires raising the required minimum Bazel version to 6.4.0.
rules_cc has an annoying dependency on protobuf without providing a WORKSPACE macro allowing the user to automatically fetch the matching version. This protobuf dependency seems to be undesired, as rules_cc version 0.1.0 drops it. We raise the version only for WORKSPACE and not for bzlmod, as with bzlmod the protobuf dependency is automatically handled in the background. rules_cc version 0.15.0 is sufficient for us regarding C++ features and with bzlmod one should define the minimum required version, not the latest greatest compatible one.
Given we test Bazel 6.4 either way as current minimum required Bazel version, it makes no sense to test 6.5 as well. Also we stop using WORKSPACE with out aspect integration tests. We did so only for one Bazel 6 version either way. We do not expect WORKSPACE vs bzlmod to have any influence on the behavior of DWYU. However, testing only one setup simplifies the aspect integration test setup and maintenance. That DWYU works with WORKSPACE is being ensured by the examples and the dedicated workspace_integration tests.
This is required due to the recent upgrades to our rules_python dependency.
Setting up the hermetic toolchains is superfluous noise in the examples. Also, it requires maintaining which rules_python version we use there. Plus, loading rules_python in the examples on top of the DWYU dependencies might hide problems with the rules_python version DWYU depends on. Given we never had any issue with supporting multiple Python versions, we believe using the host interpreter in the examples is fine.
We abandon testing newer rules_python versions as the latest versions are not compatible to Bazel 6 anymore. Also testing newer rules_python via the aspect integration tests was always kinda hacky. If we want to test the latest and greatest versions of our dependencies we should setup dedicated tests for that similar to the dedicated workspace_integration tests.
On Windows we can only test a single Bazel and Python version pair due to bazelized Python on Windows being extremely slow. Our test matrix already has a default setup for fast testing without looking at the full test matrix. We should reuse it here to reduce the maintenance burden. With us dropping the WORKSPACE setup in those tests, specifying `-p 8.8.18` stopped working.
7b9ebd2
to
f02a74c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
6.4.0
0.37.2
0.1.0