From 455761f7a970bef449b2b445644c5f09272100ed Mon Sep 17 00:00:00 2001 From: Robert Sayre Date: Sun, 26 Jun 2022 12:10:23 -0700 Subject: [PATCH] Checkpoint for a more generic implementation. --- impl/src/context.rs | 13 +++++-- impl/src/features.rs | 55 +++++++++++----------------- impl/src/planning.rs | 38 ++++++++++++++++++++ impl/src/planning/subplanners.rs | 62 +++++++++++--------------------- 4 files changed, 89 insertions(+), 79 deletions(-) diff --git a/impl/src/context.rs b/impl/src/context.rs index ef97f3e8b..b6795d52d 100644 --- a/impl/src/context.rs +++ b/impl/src/context.rs @@ -14,7 +14,7 @@ use std::collections::{BTreeMap, BTreeSet}; -use crate::{features::Features, settings::CrateSettings}; +use crate::{features::Features, planning::PlatformCrateAttribute, settings::CrateSettings}; use camino::Utf8PathBuf; use semver::Version; use serde::{Deserialize, Serialize}; @@ -141,8 +141,17 @@ impl CrateDependencyContext { #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)] pub struct CrateTargetedDepContext { - pub deps: CrateDependencyContext, pub platform_targets: Vec, + pub deps: CrateDependencyContext, +} + +impl PlatformCrateAttribute for CrateTargetedDepContext { + fn new(platforms: Vec, attrs: Vec) -> Self { + CrateTargetedDepContext { + platform_targets: platforms, + deps: attrs[0].clone(), + } + } } #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] diff --git a/impl/src/features.rs b/impl/src/features.rs index f674367ed..ffddaf220 100644 --- a/impl/src/features.rs +++ b/impl/src/features.rs @@ -17,6 +17,7 @@ use std::path::Path; use std::process::Command; use crate::error::RazeError; +use crate::planning::{consolidate_platform_attributes, PlatformCrateAttribute}; use crate::settings::RazeSettings; use crate::util::cargo_bin_path; use anyhow::{Error, Result}; @@ -47,6 +48,15 @@ pub struct TargetedFeatures { pub features: Vec, } +impl PlatformCrateAttribute for TargetedFeatures { + fn new(platforms: Vec, attrs: Vec) -> Self { + TargetedFeatures { + platforms, + features: attrs, + } + } +} + // A function that runs `cargo-tree` to analyze per-platform features. // This step should not need to be separate from cargo-metadata, but cargo-metadata's // output is incomplete in this respect. @@ -248,48 +258,23 @@ fn consolidate_features( let common_features = sets.iter().skip(1).fold(sets[0].clone(), |acc, hs| { acc.intersection(hs).cloned().collect() }); - - // Partition the platform features - let mut platform_map: BTreeMap> = BTreeMap::new(); - for (platform, pfs) in features { - for feature in pfs { - if !common_features.contains(&feature) { - platform_map - .entry(feature) - .and_modify(|e| e.push(platform.clone())) - .or_insert_with(|| vec![platform.clone()]); - } - } - } - - let mut platforms_to_features: BTreeMap, Vec> = BTreeMap::new(); - for (feature, platforms) in platform_map { - let key = platforms.clone(); - platforms_to_features - .entry(key) - .and_modify(|e| e.push(feature.clone())) - .or_insert_with(|| vec![feature]); - } - - let mut common_vec: Vec = common_features.iter().cloned().collect(); - common_vec.sort(); - - let targeted_features: Vec = platforms_to_features + let platform_features: BTreeMap> = features .iter() - .map(|ptf| { - let (platforms, features) = ptf; - TargetedFeatures { - platforms: platforms.to_vec(), - features: features.to_vec(), - } + .map(|(platform, pfs)| { + ( + platform.to_string(), + pfs.difference(&common_features).cloned().collect(), + ) }) .collect(); ( id, Features { - features: common_vec, - targeted_features, + features: common_features.iter().cloned().collect(), + targeted_features: consolidate_platform_attributes::( + platform_features, + ), }, ) } diff --git a/impl/src/planning.rs b/impl/src/planning.rs index ba2f4f10d..2ff636b34 100644 --- a/impl/src/planning.rs +++ b/impl/src/planning.rs @@ -16,6 +16,8 @@ mod crate_catalog; mod license; mod subplanners; +use std::collections::{BTreeMap, BTreeSet}; + use anyhow::Result; use cargo_lock::Lockfile; @@ -79,6 +81,42 @@ impl BuildPlannerImpl { } } +/// Items that need to be rendered with platform select blocks. +pub trait PlatformCrateAttribute { + fn new(platforms: Vec, attrs: Vec) -> Self; +} + +/// Group PlatformCrateAttribute items into concise select blocks, with no duplicates. +pub fn consolidate_platform_attributes< + AttrType: Ord + Clone, + T: PlatformCrateAttribute, +>( + platform_attributes: BTreeMap>, +) -> Vec { + let mut platform_map: BTreeMap> = BTreeMap::new(); + for (platform, pfs) in platform_attributes { + for feature in pfs { + platform_map + .entry(feature) + .and_modify(|e| e.push(platform.clone())) + .or_insert_with(|| vec![platform.clone()]); + } + } + + let mut platforms_to_features: BTreeMap, Vec> = BTreeMap::new(); + for (feature, platforms) in platform_map { + platforms_to_features + .entry(platforms.clone()) + .and_modify(|e| e.push(feature.clone())) + .or_insert_with(|| vec![feature.clone()]); + } + + platforms_to_features + .iter() + .map(|(platforms, features)| T::new(platforms.to_vec(), features.to_vec())) + .collect() +} + #[cfg(test)] pub mod tests { use std::{collections::BTreeMap, collections::HashMap, collections::HashSet}; diff --git a/impl/src/planning/subplanners.rs b/impl/src/planning/subplanners.rs index d2267ee30..ad6f31613 100644 --- a/impl/src/planning/subplanners.rs +++ b/impl/src/planning/subplanners.rs @@ -13,7 +13,7 @@ // limitations under the License. use std::{ - collections::{BTreeMap, HashMap, HashSet}, + collections::{BTreeMap, BTreeSet, HashMap, HashSet}, io, iter, str::FromStr, }; @@ -34,7 +34,7 @@ use crate::{ error::{RazeError, PLEASE_FILE_A_BUG}, features::Features, metadata::RazeMetadata, - planning::license, + planning::{consolidate_platform_attributes, license}, settings::{CrateSettings, GenMode, RazeSettings}, util, }; @@ -308,45 +308,23 @@ impl<'planner> CrateSubplanner<'planner> { ctx.subtract(&default_deps); } - // Build a list of dependencies while addressing a potential allowlist of target triples - let targeted_deps = deps - .into_iter() - .map(|(target, deps)| { - let target = target.unwrap(); - let platform_targets = util::get_matching_bazel_triples(&target, &self.settings.targets)? - .map(|x| x.to_string()) - .collect(); - - Ok(CrateTargetedDepContext { - deps, - platform_targets, - }) - }) - .filter(|res| match res { - Ok(ctx) => !ctx.platform_targets.is_empty(), - Err(_) => true, - }) - .collect::>>()?; - - // Consolidate any dependencies duplicated across platforms - let mut platform_map: BTreeMap> = BTreeMap::new(); - for targeted_dep in &targeted_deps { - platform_map - .entry(targeted_dep.deps.clone()) - .and_modify(|e| e.append(&mut targeted_dep.platform_targets.clone())) - .or_insert_with(|| targeted_dep.platform_targets.clone()); - } - let grouped_targeted_deps: Vec = platform_map - .iter() - .map(|dep| { - let mut targets = dep.1.clone(); - targets.sort(); - CrateTargetedDepContext { - deps: dep.0.clone(), - platform_targets: targets, - } - }) - .collect(); + // Build a list of dependencies while addressing a potential allowlist of target triples and consolidate any dependencies duplicated across platforms. + let targeted_deps = + consolidate_platform_attributes::( + deps + .into_iter() + .flat_map(|(target, dep_context)| { + let target = target.unwrap(); + if let Ok(triples) = util::get_matching_bazel_triples(&target, &self.settings.targets) { + triples + .map(|i| (i.to_string(), BTreeSet::from([dep_context.clone()]))) + .collect() + } else { + vec![] + } + }) + .collect(), + ); let mut workspace_member_dependents: Vec = Vec::new(); let mut workspace_member_dev_dependents: Vec = Vec::new(); @@ -422,7 +400,7 @@ impl<'planner> CrateSubplanner<'planner> { is_binary_dependency, is_proc_macro, default_deps, - targeted_deps: grouped_targeted_deps, + targeted_deps, workspace_path_to_crate: self.crate_catalog_entry.workspace_path(self.settings)?, build_script_target: build_script_target_opt, links: package.links.clone(),