Query RustcTargetData during feature resolution

Author: fee1-dead

benches/benchsuite/benches/resolve.rs

@@ -25,7 +25,7 @@ struct ResolveInfo<'cfg> {
 fn do_resolve<'cfg>(config: &'cfg Config, ws_root: &Path) -> ResolveInfo<'cfg> {
     let requested_kinds = [CompileKind::Host];
     let ws = Workspace::new(&ws_root.join("Cargo.toml"), config).unwrap();
-    let target_data = RustcTargetData::new(&ws, &requested_kinds).unwrap();
+    let mut target_data = RustcTargetData::new(&ws, &requested_kinds).unwrap();
     let cli_features = CliFeatures::from_command_line(&[], false, true).unwrap();
     let pkgs = cargo::ops::Packages::Default;
     let specs = pkgs.to_package_id_specs(&ws).unwrap();
@@ -35,7 +35,7 @@ fn do_resolve<'cfg>(config: &'cfg Config, ws_root: &Path) -> ResolveInfo<'cfg> {
     // not confuse criterion's warmup.
     let ws_resolve = cargo::ops::resolve_ws_with_opts(
         &ws,
-        &target_data,
+        &mut target_data,
         &requested_kinds,
         &cli_features,
         &specs,

src/cargo/core/compiler/build_context/target_info.rs

@@ -942,7 +942,7 @@ impl<'cfg> RustcTargetData<'cfg> {
     }
 
     /// Insert `kind` into our `target_info` and `target_config` members if it isn't present yet.
-    fn merge_compile_kind(&mut self, kind: CompileKind) -> CargoResult<()> {
+    pub fn merge_compile_kind(&mut self, kind: CompileKind) -> CargoResult<()> {
         if let CompileKind::Target(target) = kind {
             if !self.target_config.contains_key(&target) {
                 self.target_config

src/cargo/core/compiler/standard_lib.rs

@@ -62,7 +62,7 @@ pub(crate) fn std_crates(config: &Config, units: Option<&[Unit]>) -> Option<Vec<
 /// Resolve the standard library dependencies.
 pub fn resolve_std<'cfg>(
     ws: &Workspace<'cfg>,
-    target_data: &RustcTargetData<'cfg>,
+    target_data: &mut RustcTargetData<'cfg>,
     build_config: &BuildConfig,
     crates: &[String],
 ) -> CargoResult<(PackageSet<'cfg>, Resolve, ResolvedFeatures)> {

src/cargo/core/resolver/features.rs

@@ -46,6 +46,7 @@ use crate::core::{FeatureValue, PackageId, PackageIdSpec, PackageSet, Workspace}
 use crate::util::interning::InternedString;
 use crate::util::CargoResult;
 use anyhow::bail;
+use itertools::Itertools;
 use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
 use std::rc::Rc;
 
@@ -408,7 +409,7 @@ pub type DiffMap = BTreeMap<PackageFeaturesKey, BTreeSet<InternedString>>;
 /// [module-level documentation]: crate::core::resolver::features
 pub struct FeatureResolver<'a, 'cfg> {
     ws: &'a Workspace<'cfg>,
-    target_data: &'a RustcTargetData<'cfg>,
+    target_data: &'a mut RustcTargetData<'cfg>,
     /// The platforms to build for, requested by the user.
     requested_targets: &'a [CompileKind],
     resolve: &'a Resolve,
@@ -445,7 +446,7 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
     /// with the result.
     pub fn resolve(
         ws: &Workspace<'cfg>,
-        target_data: &RustcTargetData<'cfg>,
+        target_data: &'a mut RustcTargetData<'cfg>,
         resolve: &Resolve,
         package_set: &'a PackageSet<'cfg>,
         cli_features: &CliFeatures,
@@ -544,7 +545,7 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
             // features that enable other features.
             return Ok(());
         }
-        for (dep_pkg_id, deps) in self.deps(pkg_id, fk) {
+        for (dep_pkg_id, deps) in self.deps(pkg_id, fk)? {
             for (dep, dep_fk) in deps {
                 if dep.is_optional() {
                     // Optional dependencies are enabled in `activate_fv` when
@@ -647,7 +648,7 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
             .deferred_weak_dependencies
             .remove(&(pkg_id, fk, dep_name));
         // Activate the optional dep.
-        for (dep_pkg_id, deps) in self.deps(pkg_id, fk) {
+        for (dep_pkg_id, deps) in self.deps(pkg_id, fk)? {
             for (dep, dep_fk) in deps {
                 if dep.name_in_toml() != dep_name {
                     continue;
@@ -681,7 +682,7 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
         dep_feature: InternedString,
         weak: bool,
     ) -> CargoResult<()> {
-        for (dep_pkg_id, deps) in self.deps(pkg_id, fk) {
+        for (dep_pkg_id, deps) in self.deps(pkg_id, fk)? {
             for (dep, dep_fk) in deps {
                 if dep.name_in_toml() != dep_name {
                     continue;
@@ -777,12 +778,17 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
 
     /// Returns the dependencies for a package, filtering out inactive targets.
     fn deps(
-        &self,
+        &mut self,
         pkg_id: PackageId,
         fk: FeaturesFor,
-    ) -> Vec<(PackageId, Vec<(&'a Dependency, FeaturesFor)>)> {
+    ) -> CargoResult<Vec<(PackageId, Vec<(&'a Dependency, FeaturesFor)>)>> {
         // Helper for determining if a platform is activated.
-        let platform_activated = |dep: &Dependency| -> bool {
+        fn platform_activated(
+            dep: &Dependency,
+            fk: FeaturesFor,
+            target_data: &RustcTargetData<'_>,
+            requested_targets: &[CompileKind],
+        ) -> bool {
             // We always count platforms as activated if the target stems from an artifact
             // dependency's target specification. This triggers in conjunction with
             // `[target.'cfg(…)'.dependencies]` manifest sections.
@@ -791,18 +797,17 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
                     // We always care about build-dependencies, and they are always
                     // Host. If we are computing dependencies "for a build script",
                     // even normal dependencies are host-only.
-                    self.target_data
-                        .dep_platform_activated(dep, CompileKind::Host)
+                    target_data.dep_platform_activated(dep, CompileKind::Host)
                 }
-                (_, FeaturesFor::NormalOrDev) => self
-                    .requested_targets
+                (_, FeaturesFor::NormalOrDev) => requested_targets
                     .iter()
-                    .any(|kind| self.target_data.dep_platform_activated(dep, *kind)),
-                (_, FeaturesFor::ArtifactDep(target)) => self
-                    .target_data
-                    .dep_platform_activated(dep, CompileKind::Target(target)),
+                    .any(|kind| target_data.dep_platform_activated(dep, *kind)),
+                (_, FeaturesFor::ArtifactDep(target)) => {
+                    target_data.dep_platform_activated(dep, CompileKind::Target(target))
+                }
             }
-        };
+        }
+
         self.resolve
             .deps(pkg_id)
             .map(|(dep_id, deps)| {
@@ -811,7 +816,12 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
                     .filter(|dep| {
                         if dep.platform().is_some()
                             && self.opts.ignore_inactive_targets
-                            && !platform_activated(dep)
+                            && !platform_activated(
+                                dep,
+                                fk,
+                                self.target_data,
+                                self.requested_targets,
+                            )
                         {
                             return false;
                         }
@@ -820,7 +830,9 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
                         }
                         true
                     })
-                    .flat_map(|dep| {
+                    .collect_vec() // collect because the next closure mutably borrows `self.target_data`
+                    .into_iter()
+                    .map(|dep| {
                         // Each `dep`endency can be built for multiple targets. For one, it
                         // may be a library target which is built as initially configured
                         // by `fk`. If it appears as build dependency, it must be built
@@ -852,28 +864,49 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
                         };
 
                         // `artifact_target_keys` are produced to fulfil the needs of artifacts that have a target specification.
-                        let artifact_target_keys = dep.artifact().map(|artifact| {
-                            (
-                                artifact.is_lib(),
-                                artifact.target().map(|target| match target {
-                                    ArtifactTarget::Force(target) => {
-                                        vec![FeaturesFor::ArtifactDep(target)]
-                                    }
-                                    ArtifactTarget::BuildDependencyAssumeTarget => self
-                                        .requested_targets
-                                        .iter()
-                                        .map(|kind| match kind {
-                                            CompileKind::Host => {
-                                                let host_triple = self.target_data.rustc.host;
-                                                CompileTarget::new(&host_triple).unwrap()
-                                            }
-                                            CompileKind::Target(target) => *target,
+                        let artifact_target_keys = dep
+                            .artifact()
+                            .map(|artifact| {
+                                let host_triple = self.target_data.rustc.host;
+                                // not all targets may be queried before resolution since artifact dependencies
+                                // and per-pkg-targets are not immediately known.
+                                let mut activate_target = |target| {
+                                    self.target_data
+                                        .merge_compile_kind(CompileKind::Target(target))
+                                };
+                                CargoResult::Ok((
+                                    artifact.is_lib(),
+                                    artifact
+                                        .target()
+                                        .map(|target| {
+                                            CargoResult::Ok(match target {
+                                                ArtifactTarget::Force(target) => {
+                                                    activate_target(target)?;
+                                                    vec![FeaturesFor::ArtifactDep(target)]
+                                                }
+                                                // FIXME: this needs to interact with the `default-target` and `forced-target` values
+                                                // of the dependency
+                                                ArtifactTarget::BuildDependencyAssumeTarget => self
+                                                    .requested_targets
+                                                    .iter()
+                                                    .map(|kind| match kind {
+                                                        CompileKind::Host => {
+                                                            CompileTarget::new(&host_triple)
+                                                                .unwrap()
+                                                        }
+                                                        CompileKind::Target(target) => *target,
+                                                    })
+                                                    .map(|target| {
+                                                        activate_target(target)?;
+                                                        Ok(FeaturesFor::ArtifactDep(target))
+                                                    })
+                                                    .collect::<CargoResult<_>>()?,
+                                            })
                                         })
-                                        .map(FeaturesFor::ArtifactDep)
-                                        .collect(),
-                                }),
-                            )
-                        });
+                                        .transpose()?,
+                                ))
+                            })
+                            .transpose()?;
 
                         let dep_fks = match artifact_target_keys {
                             // The artifact is also a library and does specify custom
@@ -893,12 +926,13 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
                             // Use the standard feature key without any alteration.
                             Some((_, None)) | None => vec![lib_fk],
                         };
-                        dep_fks.into_iter().map(move |dep_fk| (dep, dep_fk))
+                        Ok(dep_fks.into_iter().map(move |dep_fk| (dep, dep_fk)))
                     })
-                    .collect::<Vec<_>>();
-                (dep_id, deps)
+                    .flatten_ok()
+                    .collect::<CargoResult<Vec<_>>>()?;
+                Ok((dep_id, deps))
             })
-            .filter(|(_id, deps)| !deps.is_empty())
+            .filter(|res| res.as_ref().map_or(true, |(_id, deps)| !deps.is_empty()))
             .collect()
     }
 

src/cargo/ops/cargo_compile/mod.rs

@@ -237,7 +237,7 @@ pub fn create_bcx<'a, 'cfg>(
     }
     config.validate_term_config()?;
 
-    let target_data = RustcTargetData::new(ws, &build_config.requested_kinds)?;
+    let mut target_data = RustcTargetData::new(ws, &build_config.requested_kinds)?;
 
     let specs = spec.to_package_id_specs(ws)?;
     let has_dev_units = {
@@ -263,7 +263,7 @@ pub fn create_bcx<'a, 'cfg>(
     };
     let resolve = ops::resolve_ws_with_opts(
         ws,
-        &target_data,
+        &mut target_data,
         &build_config.requested_kinds,
         cli_features,
         &specs,
@@ -279,7 +279,7 @@ pub fn create_bcx<'a, 'cfg>(
 
     let std_resolve_features = if let Some(crates) = &config.cli_unstable().build_std {
         let (std_package_set, std_resolve, std_features) =
-            standard_lib::resolve_std(ws, &target_data, &build_config, crates)?;
+            standard_lib::resolve_std(ws, &mut target_data, &build_config, crates)?;
         pkg_set.add_set(std_package_set);
         Some((std_resolve, std_features))
     } else {

src/cargo/ops/cargo_fetch.rs

@@ -31,7 +31,7 @@ pub fn fetch<'a>(
         &options.targets,
         CompileMode::Build,
     )?;
-    let data = RustcTargetData::new(ws, &build_config.requested_kinds)?;
+    let mut data = RustcTargetData::new(ws, &build_config.requested_kinds)?;
     let mut fetched_packages = HashSet::new();
     let mut deps_to_fetch = ws.members().map(|p| p.package_id()).collect::<Vec<_>>();
     let mut to_download = Vec::new();
@@ -70,7 +70,8 @@ pub fn fetch<'a>(
     // If -Zbuild-std was passed, download dependencies for the standard library.
     // We don't know ahead of time what jobs we'll be running, so tell `std_crates` that.
     if let Some(crates) = standard_lib::std_crates(config, None) {
-        let (std_package_set, _, _) = standard_lib::resolve_std(ws, &data, &build_config, &crates)?;
+        let (std_package_set, _, _) =
+            standard_lib::resolve_std(ws, &mut data, &build_config, &crates)?;
         packages.add_set(std_package_set);
     }
 

src/cargo/ops/cargo_output_metadata.rs

@@ -126,7 +126,7 @@ fn build_resolve_graph(
     // How should this work?
     let requested_kinds =
         CompileKind::from_requested_targets(ws.config(), &metadata_opts.filter_platforms)?;
-    let target_data = RustcTargetData::new(ws, &requested_kinds)?;
+    let mut target_data = RustcTargetData::new(ws, &requested_kinds)?;
     // Resolve entire workspace.
     let specs = Packages::All.to_package_id_specs(ws)?;
     let force_all = if metadata_opts.filter_platforms.is_empty() {
@@ -139,7 +139,7 @@ fn build_resolve_graph(
     // as that is the behavior of download_accessible.
     let ws_resolve = ops::resolve_ws_with_opts(
         ws,
-        &target_data,
+        &mut target_data,
         &requested_kinds,
         &metadata_opts.cli_features,
         &specs,

src/cargo/ops/fix.rs

@@ -243,11 +243,12 @@ fn check_resolver_change(ws: &Workspace<'_>, opts: &FixOptions) -> CargoResult<(
     // 2018 without `resolver` set must be V1
     assert_eq!(ws.resolve_behavior(), ResolveBehavior::V1);
     let specs = opts.compile_opts.spec.to_package_id_specs(ws)?;
-    let target_data = RustcTargetData::new(ws, &opts.compile_opts.build_config.requested_kinds)?;
-    let resolve_differences = |has_dev_units| -> CargoResult<(WorkspaceResolve<'_>, DiffMap)> {
+    let mut target_data =
+        RustcTargetData::new(ws, &opts.compile_opts.build_config.requested_kinds)?;
+    let mut resolve_differences = |has_dev_units| -> CargoResult<(WorkspaceResolve<'_>, DiffMap)> {
         let ws_resolve = ops::resolve_ws_with_opts(
             ws,
-            &target_data,
+            &mut target_data,
             &opts.compile_opts.build_config.requested_kinds,
             &opts.compile_opts.cli_features,
             &specs,
@@ -258,7 +259,7 @@ fn check_resolver_change(ws: &Workspace<'_>, opts: &FixOptions) -> CargoResult<(
         let feature_opts = FeatureOpts::new_behavior(ResolveBehavior::V2, has_dev_units);
         let v2_features = FeatureResolver::resolve(
             ws,
-            &target_data,
+            &mut target_data,
             &ws_resolve.targeted_resolve,
             &ws_resolve.pkg_set,
             &opts.compile_opts.cli_features,

src/cargo/ops/resolve.rs

@@ -124,7 +124,7 @@ pub fn resolve_ws<'a>(ws: &Workspace<'a>) -> CargoResult<(PackageSet<'a>, Resolv
 /// members. In this case, `opts.all_features` must be `true`.
 pub fn resolve_ws_with_opts<'cfg>(
     ws: &Workspace<'cfg>,
-    target_data: &RustcTargetData<'cfg>,
+    target_data: &mut RustcTargetData<'cfg>,
     requested_targets: &[CompileKind],
     cli_features: &CliFeatures,
     specs: &[PackageIdSpec],

src/cargo/ops/tree/mod.rs

@@ -135,7 +135,7 @@ pub fn build_and_print(ws: &Workspace<'_>, opts: &TreeOptions) -> CargoResult<()
     // TODO: Target::All is broken with -Zfeatures=itarget. To handle that properly,
     // `FeatureResolver` will need to be taught what "all" means.
     let requested_kinds = CompileKind::from_requested_targets(ws.config(), &requested_targets)?;
-    let target_data = RustcTargetData::new(ws, &requested_kinds)?;
+    let mut target_data = RustcTargetData::new(ws, &requested_kinds)?;
     let specs = opts.packages.to_package_id_specs(ws)?;
     let has_dev = if opts
         .edge_kinds
@@ -152,7 +152,7 @@ pub fn build_and_print(ws: &Workspace<'_>, opts: &TreeOptions) -> CargoResult<()
     };
     let ws_resolve = ops::resolve_ws_with_opts(
         ws,
-        &target_data,
+        &mut target_data,
         &requested_kinds,
         &opts.cli_features,
         &specs,