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

cargo metadata downloads crate files for unused targets #8981

Closed
jonhoo opened this issue Dec 16, 2020 · 4 comments · Fixed by #8987
Closed

cargo metadata downloads crate files for unused targets #8981

jonhoo opened this issue Dec 16, 2020 · 4 comments · Fixed by #8987
Labels
C-bug Category: bug

Comments

@jonhoo
Copy link
Contributor

jonhoo commented Dec 16, 2020

Problem
cargo metadata (and cargo tree, possibly also others) downloads the .crate files for dependencies that are not used under the current cfg parameters. This means that it will, for example, download windows-only dependencies even if run on a UNIX platform. cargo build and friends do not do this. This occurs even with --filter-platform/--target is passed to filter for a particular target (I understand cargo metadata is supposed to cover all targets unless explicitly otherwise specified).

Steps

  1. Construct a Cargo.toml with a cfg-guarded dependency:
    [package]
    name = "bug"
    version = "0.1.0"
    edition = "2018"
    
    [target.'cfg(bogus)'.dependencies]
    serde = "1"
  2. Run cargo check, and notice that it does not download serde.
  3. Run cargo metadata --filter-platform x86_64-unknown-linux-gnu, and notice that it does download serde.

Possible Solution(s)
The cause of the bug is here:

// Download all Packages. This is needed to serialize the information
// for every package. In theory this could honor target filtering,
// but that would be somewhat complex.
let package_map: BTreeMap<PackageId, Package> = ws_resolve

As the comment notes, that logic should do target filtering as expressed in PackageSet::download_accessible:

pub fn download_accessible(

Notes

Output of cargo version: cargo 1.48.0 (65cbdd2 2020-10-14)

@jonhoo jonhoo added the C-bug Category: bug label Dec 16, 2020
@jonhoo
Copy link
Contributor Author

jonhoo commented Dec 16, 2020

Arguably the same bug exists in cargo fetch, except that there it appears to be intentional:

// If no target was specified then all dependencies are
// fetched.
if options.targets.is_empty() {
return true;
}

@jonhoo
Copy link
Contributor Author

jonhoo commented Dec 16, 2020

Hmm, this will be complicated by the fact that for cargo metadata filtering by platform is opt-in using --filter-platform, not opt-out (like cargo tree and friends). So, if we want tools like rust-analyzer to not download unnecessary dependencies, they will need to also pass in --filter-platform, rather than us doing the conservative thing by default and requiring explicit opt-in (with --target all) to fetch all targets.

@alexcrichton
Copy link
Member

Sorry I'm still catching up on the issues so if this is irrelevant by this point ignore me, but I believe this is intentional. Without --filter-platform the metadata subcommand assumes everything could match. If the dependency is downloaded with --filter-platform that's almost certainly a bug to be fixed.

@jonhoo
Copy link
Contributor Author

jonhoo commented Dec 17, 2020

Ah, sorry missed your question:

Yes, the bug here is that cargo metadata (and cargo tree) would download dependencies for all targets even when --filter-platform/--target are passed. That's what #8987 fixes. I'll update the issue text to point this out.

My point above was more that since the default for cargo metadata defaults to all targets, the onus is on downstream tools to remember to pass --filter-platform if they don't want that behavior. I've already fixed that issue in rust-analyzer (rust-lang/rust-analyzer#6912), though the fix would have been nicer if there was a --filter-platform host.

bors added a commit that referenced this issue Dec 19, 2020
Make cargo metadata and tree respect target

Previously, the `metadata` and `tree` subcommands would download
dependencies for all targets, regardless of whether those targets were
actually enabled. This PR updates them so that they only download the
same dependencies that building would do with the requested target(s).

`cargo metadata` still includes all targets by default; you can only opt
_out_ using `--filter-platform`. Ideally it should use `--target` the
same way `tree` does, but it's too late to change that now.

Fixes #8981.
@bors bors closed this as completed in 7367e43 Dec 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants