Consolidate duplicated targeted dependencies.

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};
@@ -137,13 +137,54 @@ impl CrateDependencyContext {
  .cloned()
  .collect();
  }
+
+ pub fn add(&mut self, other: &CrateDependencyContext) {
+ self.dependencies = self
+ .dependencies
+ .union(&other.dependencies)
+ .cloned()
+ .collect();
+ self.proc_macro_dependencies = self
+ .proc_macro_dependencies
+ .union(&other.proc_macro_dependencies)
+ .cloned()
+ .collect();
+ self.build_dependencies = self
+ .build_dependencies
+ .union(&other.build_dependencies)
+ .cloned()
+ .collect();
+ self.build_proc_macro_dependencies = self
+ .build_proc_macro_dependencies
+ .union(&other.build_proc_macro_dependencies)
+ .cloned()
+ .collect();
+ self.dev_dependencies = self
+ .dev_dependencies
+ .union(&other.dev_dependencies)
+ .cloned()
+ .collect();
+ }
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
 pub struct CrateTargetedDepContext {
- pub target: String,
- pub deps: CrateDependencyContext,
  pub platform_targets: Vec<String>,
+ pub deps: CrateDependencyContext,
+}
+
+impl PlatformCrateAttribute<CrateDependencyContext> for CrateTargetedDepContext {
+ fn new(platforms: Vec<String>, attrs: Vec<CrateDependencyContext>) -> Self {
+ let deps = attrs.iter().skip(1).fold(attrs[0].clone(), |mut acc, hs| {
+ acc.add(hs);
+ acc
+ });
+
+ CrateTargetedDepContext {
+ platform_targets: platforms,
+ deps,
+ }
+ }
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]

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<String>,
 }
 
+impl PlatformCrateAttribute<String> for TargetedFeatures {
+ fn new(platforms: Vec<String>, attrs: Vec<String>) -> 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<String, Vec<String>> = 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<String>, Vec<String>> = 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<String> = common_features.iter().cloned().collect();
- common_vec.sort();
-
- let targeted_features: Vec<TargetedFeatures> = platforms_to_features
+ let platform_features: BTreeMap<String, BTreeSet<String>> = 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::<String, TargetedFeatures>(
+ platform_features,
+ ),
  },
  )
 }

impl/src/planning.rs

@@ -16,6 +16,11 @@ mod crate_catalog;
 mod license;
 mod subplanners;
 
+use std::{
+ collections::{BTreeMap, BTreeSet},
+ fmt::Debug,
+};
+
 use anyhow::Result;
 use cargo_lock::Lockfile;
 
@@ -79,6 +84,42 @@ impl BuildPlannerImpl {
  }
 }
 
+/// Items that need to be rendered with platform select blocks.
+pub trait PlatformCrateAttribute<T> {
+ fn new(platforms: Vec<String>, attrs: Vec<T>) -> Self;
+}
+
+/// Group PlatformCrateAttribute items into concise select blocks, with no duplicates.
+pub fn consolidate_platform_attributes<
+ AttrType: Ord + Clone + Debug,
+ T: PlatformCrateAttribute<AttrType>,
+>(
+ platform_attributes: BTreeMap<String, BTreeSet<AttrType>>,
+) -> Vec<T> {
+ let mut platform_map: BTreeMap<AttrType, Vec<String>> = BTreeMap::new();
+ for (platform, pfs) in platform_attributes {
+ for attr in pfs {
+ platform_map
+ .entry(attr)
+ .and_modify(|e| e.push(platform.clone()))
+ .or_insert_with(|| vec![platform.clone()]);
+ }
+ }
+
+ let mut platforms_to_attrs: BTreeMap<Vec<String>, Vec<AttrType>> = BTreeMap::new();
+ for (attr, platforms) in platform_map {
+ platforms_to_attrs
+ .entry(platforms.clone())
+ .and_modify(|e| e.push(attr.clone()))
+ .or_insert_with(|| vec![attr.clone()]);
+ }
+
+ platforms_to_attrs
+ .iter()
+ .map(|(platforms, attrs)| T::new(platforms.to_vec(), attrs.to_vec()))
+ .collect()
+}
+
 #[cfg(test)]
 pub mod tests {
  use std::{collections::BTreeMap, collections::HashMap, collections::HashSet};
@@ -412,6 +453,114 @@ pub mod tests {
  assert_eq!(flate2.targeted_deps[0].deps.dependencies.len(), 0);
  }
 
+ #[test]
+ fn test_plan_includes_all_not_unknown_targets() {
+ let mut settings = dummy_raze_settings();
+ settings.genmode = GenMode::Remote;
+ let mut triples = HashSet::new();
+ triples.insert("aarch64-apple-darwin".to_string());
+ triples.insert("aarch64-unknown-linux-gnu".to_string());
+ triples.insert("i686-apple-darwin".to_string());
+ triples.insert("i686-pc-windows-msvc".to_string());
+ triples.insert("i686-unknown-linux-gnu".to_string());
+ triples.insert("x86_64-apple-darwin".to_string());
+ triples.insert("x86_64-pc-windows-msvc".to_string());
+ triples.insert("x86_64-unknown-linux-gnu".to_string());
+ triples.insert("wasm32-wasi".to_string());
+ triples.insert("wasm32-unknown-unknown".to_string());
+ settings.targets = Some(triples);
+
+ let planner = BuildPlannerImpl::new(
+ dummy_workspace_crate_metadata(templates::TARGET_OS_IS_NOT_UNKNOWN),
+ settings,
+ );
+ let planned_build = planner.plan_build(None).unwrap();
+ let async_std = planned_build
+ .crate_contexts
+ .iter()
+ .find(|ctx| ctx.pkg_name == "async-std")
+ .unwrap();
+
+ // The wasm-targeted deps should have both wasm platforms
+ let wasm_targeted_dep_context = async_std
+ .targeted_deps
+ .iter()
+ .find(|dep_context| {
+ dep_context
+ .deps
+ .dependencies
+ .iter()
+ .find(|dep| dep.name == "wasm-bindgen-futures")
+ .is_some()
+ })
+ .unwrap();
+ assert_eq!(
+ wasm_targeted_dep_context.platform_targets,
+ vec!["wasm32-unknown-unknown", "wasm32-wasi",]
+ );
+
+ // The os-targeted deps should only have wasm32-wasi
+ let os_targeted_dep_context = async_std
+ .targeted_deps
+ .iter()
+ .find(|dep_context| {
+ dep_context
+ .deps
+ .dependencies
+ .iter()
+ .find(|dep| dep.name == "async-io")
+ .is_some()
+ })
+ .unwrap();
+ assert_eq!(
+ os_targeted_dep_context.platform_targets,
+ vec![
+ "aarch64-apple-darwin",
+ "aarch64-unknown-linux-gnu",
+ "i686-apple-darwin",
+ "i686-pc-windows-msvc",
+ "i686-unknown-linux-gnu",
+ "wasm32-wasi",
+ "x86_64-apple-darwin",
+ "x86_64-pc-windows-msvc",
+ "x86_64-unknown-linux-gnu"
+ ]
+ );
+ }
+
+ #[test]
+ // Tests the fix for https://github.com/google/cargo-raze/issues/451.
+ // Bazel errors out if mutually exclusive select branches contain the
+ // same dependency. rust-errno is a real world example of this problem,
+ // where libc is listed under 'cfg(unix)' and 'cfg(target_os="wasi")'.
+ fn test_plan_build_consolidates_targets_across_platforms() {
+ let mut settings = dummy_raze_settings();
+ settings.genmode = GenMode::Remote;
+ let mut triples = HashSet::new();
+ triples.insert("wasm32-wasi".to_string());
+ triples.insert("x86_64-unknown-linux-gnu".to_string());
+ settings.target = None;
+ settings.targets = Some(triples);
+
+ let planner = BuildPlannerImpl::new(
+ dummy_workspace_crate_metadata(templates::SUBPLAN_CONSOLIDATES_TARGETED_DEPS),
+ settings,
+ );
+ let planned_build = planner.plan_build(None).unwrap();
+
+ let errno = planned_build
+ .crate_contexts
+ .iter()
+ .find(|ctx| ctx.pkg_name == "errno")
+ .unwrap();
+
+ assert_eq!(errno.targeted_deps.len(), 1);
+ assert_eq!(
+ errno.targeted_deps[0].platform_targets,
+ vec!["wasm32-wasi", "x86_64-unknown-linux-gnu",]
+ );
+ }
+
  fn dummy_binary_dependency_metadata(is_remote_genmode: bool) -> (RazeMetadata, RazeSettings) {
  let (mut fetcher, server, index_dir) = dummy_raze_metadata_fetcher();