Auto merge of #8987 - jonhoo:metadata-target-filtering, r=ehuss

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.

Author: bors

src/cargo/core/compiler/standard_lib.rs

@@ -3,7 +3,7 @@
 use crate::core::compiler::UnitInterner;
 use crate::core::compiler::{CompileKind, CompileMode, RustcTargetData, Unit};
 use crate::core::profiles::{Profiles, UnitFor};
-use crate::core::resolver::features::{FeaturesFor, ResolvedFeatures};
+use crate::core::resolver::features::{FeaturesFor, RequestedFeatures, ResolvedFeatures};
 use crate::core::resolver::{HasDevUnits, ResolveOpts};
 use crate::core::{Dependency, PackageId, PackageSet, Resolve, SourceId, Workspace};
 use crate::ops::{self, Packages};
@@ -109,8 +109,10 @@ pub fn resolve_std<'cfg>(
     };
     // dev_deps setting shouldn't really matter here.
     let opts = ResolveOpts::new(
-        /*dev_deps*/ false, &features, /*all_features*/ false,
-        /*uses_default_features*/ false,
+        /*dev_deps*/ false,
+        RequestedFeatures::from_command_line(
+            &features, /*all_features*/ false, /*uses_default_features*/ false,
+        ),
     );
     let resolve = ops::resolve_ws_with_opts(
         &std_ws,

src/cargo/core/package.rs

@@ -434,6 +434,10 @@ impl<'cfg> PackageSet<'cfg> {
         self.packages.keys().cloned()
     }
 
+    pub fn packages<'a>(&'a self) -> impl Iterator<Item = &'a Package> + 'a {
+        self.packages.values().filter_map(|p| p.borrow())
+    }
+
     pub fn enable_download<'a>(&'a self) -> CargoResult<Downloads<'a, 'cfg>> {
         assert!(!self.downloading.replace(true));
         let timeout = ops::HttpTimeout::new(self.config)?;

src/cargo/core/resolver/types.rs

@@ -147,20 +147,8 @@ impl ResolveOpts {
         }
     }
 
-    pub fn new(
-        dev_deps: bool,
-        features: &[String],
-        all_features: bool,
-        uses_default_features: bool,
-    ) -> ResolveOpts {
-        ResolveOpts {
-            dev_deps,
-            features: RequestedFeatures::from_command_line(
-                features,
-                all_features,
-                uses_default_features,
-            ),
-        }
+    pub fn new(dev_deps: bool, features: RequestedFeatures) -> ResolveOpts {
+        ResolveOpts { dev_deps, features }
     }
 }
 

src/cargo/ops/cargo_compile.rs

@@ -33,7 +33,7 @@ use crate::core::compiler::{BuildConfig, BuildContext, Compilation, Context};
 use crate::core::compiler::{CompileKind, CompileMode, CompileTarget, RustcTargetData, Unit};
 use crate::core::compiler::{DefaultExecutor, Executor, UnitInterner};
 use crate::core::profiles::{Profiles, UnitFor};
-use crate::core::resolver::features::{self, FeaturesFor};
+use crate::core::resolver::features::{self, FeaturesFor, RequestedFeatures};
 use crate::core::resolver::{HasDevUnits, Resolve, ResolveOpts};
 use crate::core::{FeatureValue, Package, PackageSet, Shell, Summary, Target};
 use crate::core::{PackageId, PackageIdSpec, TargetKind, Workspace};
@@ -336,7 +336,10 @@ pub fn create_bcx<'a, 'cfg>(
 
     let specs = spec.to_package_id_specs(ws)?;
     let dev_deps = ws.require_optional_deps() || filter.need_dev_deps(build_config.mode);
-    let opts = ResolveOpts::new(dev_deps, features, all_features, !no_default_features);
+    let opts = ResolveOpts::new(
+        dev_deps,
+        RequestedFeatures::from_command_line(features, all_features, !no_default_features),
+    );
     let has_dev_units = if filter.need_dev_deps(build_config.mode) {
         HasDevUnits::Yes
     } else {

src/cargo/ops/cargo_doc.rs

@@ -1,5 +1,5 @@
 use crate::core::compiler::RustcTargetData;
-use crate::core::resolver::{HasDevUnits, ResolveOpts};
+use crate::core::resolver::{features::RequestedFeatures, HasDevUnits, ResolveOpts};
 use crate::core::{Shell, Workspace};
 use crate::ops;
 use crate::util::CargoResult;
@@ -21,9 +21,11 @@ pub fn doc(ws: &Workspace<'_>, options: &DocOptions) -> CargoResult<()> {
     let specs = options.compile_opts.spec.to_package_id_specs(ws)?;
     let opts = ResolveOpts::new(
         /*dev_deps*/ true,
-        &options.compile_opts.features,
-        options.compile_opts.all_features,
-        !options.compile_opts.no_default_features,
+        RequestedFeatures::from_command_line(
+            &options.compile_opts.features,
+            options.compile_opts.all_features,
+            !options.compile_opts.no_default_features,
+        ),
     );
     let target_data = RustcTargetData::new(ws, &options.compile_opts.build_config.requested_kinds)?;
     let ws_resolve = ops::resolve_ws_with_opts(

src/cargo/ops/cargo_output_metadata.rs

@@ -1,7 +1,7 @@
 use crate::core::compiler::{CompileKind, RustcTargetData};
 use crate::core::dependency::DepKind;
 use crate::core::package::SerializedPackage;
-use crate::core::resolver::{HasDevUnits, Resolve, ResolveOpts};
+use crate::core::resolver::{features::RequestedFeatures, HasDevUnits, Resolve, ResolveOpts};
 use crate::core::{Dependency, Package, PackageId, Workspace};
 use crate::ops::{self, Packages};
 use crate::util::interning::InternedString;
@@ -115,28 +115,33 @@ fn build_resolve_graph(
     let target_data = RustcTargetData::new(ws, &requested_kinds)?;
     // Resolve entire workspace.
     let specs = Packages::All.to_package_id_specs(ws)?;
-    let resolve_opts = ResolveOpts::new(
-        /*dev_deps*/ true,
+    let requested_features = RequestedFeatures::from_command_line(
         &metadata_opts.features,
         metadata_opts.all_features,
         !metadata_opts.no_default_features,
     );
+    let resolve_opts = ResolveOpts::new(/*dev_deps*/ true, requested_features.clone());
+    let force_all = if metadata_opts.filter_platforms.is_empty() {
+        crate::core::resolver::features::ForceAllTargets::Yes
+    } else {
+        crate::core::resolver::features::ForceAllTargets::No
+    };
+
+    // Note that even with --filter-platform we end up downloading host dependencies as well,
+    // as that is the behavior of download_accessible.
     let ws_resolve = ops::resolve_ws_with_opts(
         ws,
         &target_data,
         &requested_kinds,
         &resolve_opts,
         &specs,
         HasDevUnits::Yes,
-        crate::core::resolver::features::ForceAllTargets::No,
+        force_all,
     )?;
-    // 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
         .pkg_set
-        .get_many(ws_resolve.pkg_set.package_ids())?
-        .into_iter()
+        .packages()
         // This is a little lazy, but serde doesn't handle Rc fields very well.
         .map(|pkg| (pkg.package_id(), Package::clone(pkg)))
         .collect();

src/cargo/ops/tree/graph.rs

@@ -351,6 +351,13 @@ fn add_pkg(
                 true
             })
             .collect();
+
+        // This dependency is eliminated from the dependency tree under
+        // the current target and feature set.
+        if deps.is_empty() {
+            continue;
+        }
+
         deps.sort_unstable_by_key(|dep| dep.name_in_toml());
         let dep_pkg = graph.package_map[&dep_id];
 

src/cargo/ops/tree/mod.rs

@@ -3,7 +3,9 @@
 use self::format::Pattern;
 use crate::core::compiler::{CompileKind, RustcTargetData};
 use crate::core::dependency::DepKind;
-use crate::core::resolver::{ForceAllTargets, HasDevUnits, ResolveOpts};
+use crate::core::resolver::{
+    features::RequestedFeatures, ForceAllTargets, HasDevUnits, ResolveOpts,
+};
 use crate::core::{Package, PackageId, PackageIdSpec, Workspace};
 use crate::ops::{self, Packages};
 use crate::util::{CargoResult, Config};
@@ -136,12 +138,12 @@ pub fn build_and_print(ws: &Workspace<'_>, opts: &TreeOptions) -> CargoResult<()
     let requested_kinds = CompileKind::from_requested_targets(ws.config(), &requested_targets)?;
     let target_data = RustcTargetData::new(ws, &requested_kinds)?;
     let specs = opts.packages.to_package_id_specs(ws)?;
-    let resolve_opts = ResolveOpts::new(
-        /*dev_deps*/ true,
+    let requested_features = RequestedFeatures::from_command_line(
         &opts.features,
         opts.all_features,
         !opts.no_default_features,
     );
+    let resolve_opts = ResolveOpts::new(/*dev_deps*/ true, requested_features.clone());
     let has_dev = if opts
         .edge_kinds
         .contains(&EdgeKind::Dep(DepKind::Development))
@@ -164,13 +166,10 @@ pub fn build_and_print(ws: &Workspace<'_>, opts: &TreeOptions) -> CargoResult<()
         has_dev,
         force_all,
     )?;
-    // Download all Packages. Some display formats need to display package metadata.
-    // This may trigger some unnecessary downloads, but trying to figure out a
-    // minimal set would be difficult.
+
     let package_map: HashMap<PackageId, &Package> = ws_resolve
         .pkg_set
-        .get_many(ws_resolve.pkg_set.package_ids())?
-        .into_iter()
+        .packages()
         .map(|pkg| (pkg.package_id(), pkg))
         .collect();
 

tests/testsuite/features2.rs

@@ -2214,29 +2214,19 @@ fn minimal_download() {
         .run();
     clear();
 
-    // This disables itarget, but leaves decouple_dev_deps enabled. However,
-    // `cargo tree` downloads everything anyways. Ideally `cargo tree` should
-    // be a little smarter, but that would make it a bit more complicated. The
-    // two "Downloading" lines are because `download_accessible` doesn't
-    // download enough (`cargo tree` really wants everything). Again, that
-    // could be cleaner, but is difficult.
+    // This disables itarget, but leaves decouple_dev_deps enabled.
     p.cargo("tree -e normal --target=all -Zfeatures=all")
         .masquerade_as_nightly_cargo()
         .with_stderr_unordered(
             "\
 [DOWNLOADING] crates ...
-[DOWNLOADING] crates ...
 [DOWNLOADED] normal v1.0.0 [..]
-[DOWNLOADED] dev_dep v1.0.0 [..]
 [DOWNLOADED] normal_pm v1.0.0 [..]
 [DOWNLOADED] build_dep v1.0.0 [..]
-[DOWNLOADED] dev_dep_pm v1.0.0 [..]
 [DOWNLOADED] build_dep_pm v1.0.0 [..]
 [DOWNLOADED] itarget_normal v1.0.0 [..]
-[DOWNLOADED] itarget_dev_dep v1.0.0 [..]
 [DOWNLOADED] itarget_normal_pm v1.0.0 [..]
 [DOWNLOADED] itarget_build_dep v1.0.0 [..]
-[DOWNLOADED] itarget_dev_dep_pm v1.0.0 [..]
 [DOWNLOADED] itarget_build_dep_pm v1.0.0 [..]
 ",
         )

tests/testsuite/metadata.rs

@@ -1,5 +1,7 @@
 //! Tests for the `cargo metadata` command.
 
+use cargo_test_support::install::cargo_home;
+use cargo_test_support::paths::CargoPathExt;
 use cargo_test_support::registry::Package;
 use cargo_test_support::{basic_bin_manifest, basic_lib_manifest, main_file, project, rustc_host};
 
@@ -2343,8 +2345,27 @@ fn filter_platform() {
     .replace("$ALT_TRIPLE", alt_target)
     .replace("$HOST_TRIPLE", &rustc_host());
 
+    // We're going to be checking that we don't download excessively,
+    // so we need to ensure that downloads will happen.
+    let clear = || {
+        cargo_home().join("registry/cache").rm_rf();
+        cargo_home().join("registry/src").rm_rf();
+        p.build_dir().rm_rf();
+    };
+
     // Normal metadata, no filtering, returns *everything*.
     p.cargo("metadata")
+        .with_stderr_unordered(
+            "\
+[UPDATING] [..]
+[WARNING] please specify `--format-version` flag explicitly to avoid compatibility problems
+[DOWNLOADING] crates ...
+[DOWNLOADED] normal-dep v0.0.1 [..]
+[DOWNLOADED] host-dep v0.0.1 [..]
+[DOWNLOADED] alt-dep v0.0.1 [..]
+[DOWNLOADED] cfg-dep v0.0.1 [..]
+",
+        )
         .with_json(
             &r#"
 {
@@ -2454,10 +2475,20 @@ fn filter_platform() {
             .replace("$FOO", &foo),
         )
         .run();
+    clear();
 
     // Filter on alternate, removes cfg and host.
     p.cargo("metadata --filter-platform")
         .arg(alt_target)
+        .with_stderr_unordered(
+            "\
+[WARNING] please specify `--format-version` flag explicitly to avoid compatibility problems
+[DOWNLOADING] crates ...
+[DOWNLOADED] normal-dep v0.0.1 [..]
+[DOWNLOADED] host-dep v0.0.1 [..]
+[DOWNLOADED] alt-dep v0.0.1 [..]
+",
+        )
         .with_json(
             &r#"
 {
@@ -2526,10 +2557,19 @@ fn filter_platform() {
             .replace("$FOO", &foo),
         )
         .run();
+    clear();
 
     // Filter on host, removes alt and cfg.
     p.cargo("metadata --filter-platform")
         .arg(rustc_host())
+        .with_stderr_unordered(
+            "\
+[WARNING] please specify `--format-version` flag explicitly to avoid compatibility problems
+[DOWNLOADING] crates ...
+[DOWNLOADED] normal-dep v0.0.1 [..]
+[DOWNLOADED] host-dep v0.0.1 [..]
+",
+        )
         .with_json(
             &r#"
 {
@@ -2598,11 +2638,21 @@ fn filter_platform() {
             .replace("$FOO", &foo),
         )
         .run();
+    clear();
 
     // Filter host with cfg, removes alt only
     p.cargo("metadata --filter-platform")
         .arg(rustc_host())
         .env("RUSTFLAGS", "--cfg=foobar")
+        .with_stderr_unordered(
+            "\
+[WARNING] please specify `--format-version` flag explicitly to avoid compatibility problems
+[DOWNLOADING] crates ...
+[DOWNLOADED] normal-dep v0.0.1 [..]
+[DOWNLOADED] host-dep v0.0.1 [..]
+[DOWNLOADED] cfg-dep v0.0.1 [..]
+",
+        )
         .with_json(
             &r#"
 {