-
Notifications
You must be signed in to change notification settings - Fork 904
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
cargo fmt --all downloads dependencies #4599
Comments
The requisite upstream feature we need is rust-lang/cargo#7483 |
Previously, the location for path-based (i.e., local) packages was redacted from metadata output. This was done way back when so that `Cargo.lock` would not include path sources: rust-lang#7483 (comment) This PR makes this redaction optional so that `cargo metadata` and friends _will_ output the source location, which is useful for tools like `rustfmt` which need to know the paths to local packages (see rust-lang#7483 and rust-lang/rustfmt#4599). Interestingly enough, no tests fail even though `disable_path_serialization` is never currently called. I don't know if that's because there's no test that cover where redaction is needed, or because redaction is simply no longer needed (presumably because lockfile generation now uses custom logic). If it is the latter, this PR can be simplified to not support opting into redaction. Fixes rust-lang#7483
Thanks for the report, but I'm going to close as a duplicate as there's a few open issues where this has been discussed already (e.g. #4247) and summarized in #4247 (comment) |
I'm terrible at searching, my apologies! I'll try to help pushing on upstream to figure out a fix :) |
No worries, and thanks for your work in this space! It's a tricky issue on both sides and has negative impacts on some portion of the user base so getting this resolved would be fantastic |
Describe the bug
When running
cargo fmt
with the--all
argument, it will download all dependencies in the current workspace, including for targets not used on the current platform.To Reproduce
serde = "1"
.serde-*.crate
from~/.cargo/registry/cache/github.com-*/
cargo fmt
serde-*.crate
files were downloaded into~/.cargo/registry/cache/
cargo fmt --all
serde-*.crate
was downloaded into~/.cargo/registry/cache/
Furthermore, do the above with a
cfg
-gated dependency onserde
usingNotice that
serde
is still downloaded with--all
, even though it does not even meet the target platform.Expected behavior
cargo fmt
should never download dependencies, whether run with--all
or not.Meta
rustfmt 1.4.24-stable (eb894d53 2020-11-05)
rustup
cargo-fmt
Diagnosis
The origin of this issue is that
--all
invokescargo metadata
without--no-deps
rustfmt/src/cargo-fmt/main.rs
Line 456 in 81ad114
This behavior was introduced in #3664, and the reasoning is spelled out in #3664 (comment). I think this probably requires a fix to upstream, but I'm not sure (ideas welcome!). I figured that regardless, a tracking issue would be useful.
The
target
issue could conceivably be fixed by passing--filter-platform
tocargo metadata
(it considers all targets by default). But, that would still only work subject to rust-lang/cargo#8987, and would also mean thatcfg
-guarded workspace members would not be walked byrustfmt
(I think?).The text was updated successfully, but these errors were encountered: