Do not cache rustc info in deps resolver #3107
Merged
+16
−7
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
The cargo_tree_resolver discovers dependencies by executing `cargo tree` for different host and target triples. This procedure breaks when cargo successfully but incorrectly caches rustc info. Incorrect cache hit may happen because cache key does not take into account HOST_TRIPLE env variable used by `rules_rust` to force rustc to report it's built for different host. The caching may happen if one has target/ directory in repo, or it was created through running some commands outside of bazel. Details: After recent fixes to support properly bzlmod (bazelbuild#3034) noticed issues under windows when building bazel-lsp: cameron-martin/bazel-lsp#92 Cargo may cache rustc info in target/.rustc_info.json : https://github.com/rust-lang/cargo/blob/769f622e12db0001431d8ae36d1093fb8727c5d9/src/cargo/util/rustc.rs#L163 The rules_rust hacks rustc by setting HOST_TRIPLE: https://github.com/bazelbuild/rules_rust/blob/3aecdbedba0c001d0660ff631aeccb35d94ff3a7/crate_universe/src/metadata/cargo_tree_resolver.rs#L152 The HOST_TRIPLE env variable is not taken into account by cargo when checking whether cached rustc info is valid: https://github.com/rust-lang/cargo/blob/769f622e12db0001431d8ae36d1093fb8727c5d9/src/cargo/util/rustc.rs#L320
hauserx
force-pushed
the
dont_cache_rustc_info
branch
from
f35dacf to
8b3538b
Compare
UebelAndre
requested changes
Dec 16, 2024
| .unwrap(); | ||
|
|
||
| // Put Cargo.toml into writable directory structure and create target/ directory to verify that | ||
| // cargo does not incorrectly cache rustc info in target/.rustc_info.json file. |
There was a problem hiding this comment.
Should there be an assert that this file was never created?
UebelAndre
approved these changes
Dec 16, 2024
havasd
added a commit
to havasd/rules_rust
that referenced
this pull request
Dec 16, 2024
This reverts commit 834df35.
|
@hauserx Could I bother you to file an issue (or PR!) against Cargo for this behaviour? It seems wrong that they're caching something and omitting relevant input from the cache key? Thanks for the PR here! |
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.
The cargo_tree_resolver discovers dependencies by executing
cargo treefor different host and target triples. This procedure breaks when cargo successfully but incorrectly caches rustc info. Incorrect cache hit may happen because cache key does not take into account HOST_TRIPLE env variable used byrules_rustto force rustc to report it's built for different host.The caching may happen if one has target/ directory in repo, or it was created through running some commands outside of bazel.
Details
After recent fixes to support properly bzlmod
(Added bzlmod support to core rules_rust #3034) noticed issues under windows when building bazel-lsp: Build fails under Windows locally: unresolved import
winapi::shared::winerrorcameron-martin/bazel-lsp#92Cargo may cache rustc info in target/.rustc_info.json : https://github.com/rust-lang/cargo/blob/769f622e12db0001431d8ae36d1093fb8727c5d9/src/cargo/util/rustc.rs#L163
The rules_rust hacks rustc by setting HOST_TRIPLE:
rules_rust/crate_universe/src/metadata/cargo_tree_resolver.rs
Line 152 in 3aecdbe
The HOST_TRIPLE env variable is not taken into account by cargo when checking whether cached rustc info is valid:
https://github.com/rust-lang/cargo/blob/769f622e12db0001431d8ae36d1093fb8727c5d9/src/cargo/util/rustc.rs#L320