Skip to content
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

Only use metadata for current target #6908

Closed
jonhoo opened this issue Dec 16, 2020 · 2 comments · Fixed by #6912
Closed

Only use metadata for current target #6908

jonhoo opened this issue Dec 16, 2020 · 2 comments · Fixed by #6912

Comments

@jonhoo
Copy link
Contributor

jonhoo commented Dec 16, 2020

Unlike most other cargo tools, cargo metadata counter-intuitively defaults to including all targets. This means that with the current logic, rust-analyzer ends up processing all targets unless one a custom target is specified:
https://github.com/rust-analyzer/rust-analyzer/blob/41321d96789ed918eebda02ada76758765d19d16/crates/project_model/src/cargo_workspace.rs#L169-L171

I think rust-analyzer should probably always specify --filter-platform, and just specify the host platform if target is not specified. Cargo finds the host platform by running rustc -Vv and extracing the host: line, which rust-analyzer could probably also do pretty easily:
https://github.com/rust-lang/cargo/blob/fde8ee3ba0cba4eef75f197d42586ef4664f057b/src/cargo/util/rustc.rs#L48-L68

There's also precedent for relying on rustc to determine these kinds of things, such as using it to find the sysroot:
https://github.com/rust-analyzer/rust-analyzer/blob/48802e54d1669c68fd27b007533dd6aa5221d089/crates/project_model/src/sysroot.rs#L126-L127

One way to make this nicer would be if cargo metadata supported a "special" value for --filter-platform called host. I'll look at submitting that as a PR upstream, but the more immediate fix (and also one that would continue to work for older compiler releases) would be to just use rustc -Vv.

bors bot added a commit that referenced this issue Dec 17, 2020
6912: Default to host platform for cargo metadata r=jonhoo a=jonhoo

This modifies the logic for calling cargo metadata so that it will use
the host platform if no explicit target platform is given. This is
needed since cargo metadata defaults to outputting information for _all_
targets.

Fixes #6908.

Co-authored-by: Jon Gjengset <jongje@amazon.com>
Co-authored-by: Jon Gjengset <jon@thesquareplanet.com>
@bors bors bot closed this as completed in 9802c0c Dec 17, 2020
Matthias-Fauconneau pushed a commit to Matthias-Fauconneau/rust-analyzer that referenced this issue Feb 7, 2021
This modifies the logic for calling cargo metadata so that it will use
the host platform if no explicit target platform is given. This is
needed since cargo metadata defaults to outputting information for _all_
targets.

Fixes rust-lang#6908.
@steveyko
Copy link

steveyko commented Mar 5, 2021

I am using Rust toolchain 1.37.0 due to some LLVM compatibility issues with other codebases. In that version, cargo doesn't have --filter-platform. I use coc.nvim with rust-analyzer, and my nvim always complains that cargo metadata wasn't expecting --filter-platform. I thought I'd report this in case people here are interested in backward compatibility.

@chorman0773
Copy link

I'm going to second the backcompat issue. I'm currently using 1,38 in a program to ensure it can be built directly from mrustc and get the same issue. Maybe an opt-out would be useful?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants