Don't allow default deps in planning of platform deps. (#437)

* Don't allow default deps in platform dep planning.
* rustfmt
* Switch data structures to BTreeSet, add test.
* Remove eprintln

impl/src/context.rs

@@ -12,7 +12,10 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use std::{collections::BTreeMap, path::PathBuf};
+use std::{
+  collections::{BTreeMap, BTreeSet},
+  path::PathBuf,
+};
 
 use crate::settings::CrateSettings;
 use semver::Version;
@@ -83,15 +86,15 @@ pub struct SourceDetails {
 
 #[derive(Default, Debug, Clone, Serialize, PartialEq, Eq, PartialOrd, Ord)]
 pub struct CrateDependencyContext {
-  pub dependencies: Vec<BuildableDependency>,
-  pub proc_macro_dependencies: Vec<BuildableDependency>,
+  pub dependencies: BTreeSet<BuildableDependency>,
+  pub proc_macro_dependencies: BTreeSet<BuildableDependency>,
   // data_dependencies can only be set when using cargo-raze as a library at the moment.
-  pub data_dependencies: Vec<BuildableDependency>,
-  pub build_dependencies: Vec<BuildableDependency>,
-  pub build_proc_macro_dependencies: Vec<BuildableDependency>,
+  pub data_dependencies: BTreeSet<BuildableDependency>,
+  pub build_dependencies: BTreeSet<BuildableDependency>,
+  pub build_proc_macro_dependencies: BTreeSet<BuildableDependency>,
   // build_data_dependencies can only be set when using cargo-raze as a library at the moment.
-  pub build_data_dependencies: Vec<BuildableDependency>,
-  pub dev_dependencies: Vec<BuildableDependency>,
+  pub build_data_dependencies: BTreeSet<BuildableDependency>,
+  pub dev_dependencies: BTreeSet<BuildableDependency>,
   /// Aliased dependencies, sorted/keyed by their `target` name in the `DependencyAlias` struct.
   pub aliased_dependencies: BTreeMap<String, DependencyAlias>,
 }
@@ -105,6 +108,34 @@ impl CrateDependencyContext {
       || self.build_proc_macro_dependencies.iter().any(condition)
       || self.dev_dependencies.iter().any(condition)
   }
+
+  pub fn subtract(&mut self, other: &CrateDependencyContext) {
+    self.dependencies = self
+      .dependencies
+      .difference(&other.dependencies)
+      .cloned()
+      .collect();
+    self.proc_macro_dependencies = self
+      .proc_macro_dependencies
+      .difference(&other.proc_macro_dependencies)
+      .cloned()
+      .collect();
+    self.build_dependencies = self
+      .build_dependencies
+      .difference(&other.build_dependencies)
+      .cloned()
+      .collect();
+    self.build_proc_macro_dependencies = self
+      .build_proc_macro_dependencies
+      .difference(&other.build_proc_macro_dependencies)
+      .cloned()
+      .collect();
+    self.dev_dependencies = self
+      .dev_dependencies
+      .difference(&other.dev_dependencies)
+      .cloned()
+      .collect();
+  }
 }
 
 #[derive(Debug, Clone, Serialize, PartialEq, Eq, PartialOrd, Ord)]

impl/src/planning.rs

@@ -81,7 +81,7 @@ impl BuildPlannerImpl {
 
 #[cfg(test)]
 mod tests {
-  use std::{collections::HashMap, path::PathBuf};
+  use std::{collections::HashMap, collections::HashSet, path::PathBuf};
 
   use crate::{
     metadata::tests::{
@@ -373,6 +373,40 @@ mod tests {
     );
   }
 
+  #[test]
+  // Tests the fix for https://github.com/google/cargo-raze/issues/389
+  // as implemented in https://github.com/google/cargo-raze/pull/437
+  fn test_plan_build_deduplicates_target_dependencies() {
+    let mut settings = dummy_raze_settings();
+    settings.genmode = GenMode::Remote;
+    let mut triples = HashSet::new();
+    triples.insert("wasm32-unknown-unknown".to_string());
+    triples.insert("x86_64-unknown-linux-gnu".to_string());
+    settings.targets = Some(triples);
+
+    let planner = BuildPlannerImpl::new(
+      dummy_workspace_crate_metadata(
+        templates::SUBPLAN_OMITS_PLATFORM_DEPS_ALREADY_IN_DEFAULT_DEPS,
+      ),
+      settings,
+    );
+    let planned_build = planner.plan_build(None).unwrap();
+
+    let flate2 = planned_build
+      .crate_contexts
+      .iter()
+      .find(|ctx| ctx.pkg_name == "flate2")
+      .unwrap();
+
+    let miniz_oxide = flate2
+      .default_deps
+      .dependencies
+      .iter()
+      .find(|dep| dep.name == "miniz_oxide");
+    assert!(miniz_oxide.is_some());
+    assert_eq!(flate2.targeted_deps[0].deps.dependencies.len(), 0);
+  }
+
   fn dummy_binary_dependency_metadata(is_remote_genmode: bool) -> (RazeMetadata, RazeSettings) {
     let (mut fetcher, server, index_dir) = dummy_raze_metadata_fetcher();
 

impl/src/planning/subplanners.rs

@@ -292,6 +292,11 @@ impl<'planner> CrateSubplanner<'planner> {
     // Take the default deps that are not bound to platform targets
     let default_deps = deps.remove(&None).unwrap_or_default();
 
+    // Remove anything in default_deps
+    for ctx in deps.values_mut() {
+      ctx.subtract(&default_deps);
+    }
+
     // Build a list of dependencies while addression a potential allowlist of target triples
     let mut targeted_deps = deps
       .into_iter()
@@ -486,14 +491,6 @@ impl<'planner> CrateSubplanner<'planner> {
       }
     }
 
-    for set in dep_production.values_mut() {
-      set.build_proc_macro_dependencies.sort();
-      set.build_dependencies.sort();
-      set.dev_dependencies.sort();
-      set.proc_macro_dependencies.sort();
-      set.dependencies.sort();
-    }
-
     Ok(dep_production)
   }
 
@@ -516,16 +513,16 @@ impl<'planner> CrateSubplanner<'planner> {
 
     use DependencyKind::*;
     match dep.kind {
-      Build if is_proc_macro => dep_set.build_proc_macro_dependencies.push(build_dep),
-      Build => dep_set.build_dependencies.push(build_dep),
-      Development => dep_set.dev_dependencies.push(build_dep),
-      Normal if is_proc_macro => dep_set.proc_macro_dependencies.push(build_dep),
+      Build if is_proc_macro => dep_set.build_proc_macro_dependencies.insert(build_dep),
+      Build => dep_set.build_dependencies.insert(build_dep),
+      Development => dep_set.dev_dependencies.insert(build_dep),
+      Normal if is_proc_macro => dep_set.proc_macro_dependencies.insert(build_dep),
       Normal => {
         // sys crates may generate DEP_* env vars that must be visible to direct dep builds
         if is_sys_crate {
-          dep_set.build_dependencies.push(build_dep.clone());
+          dep_set.build_dependencies.insert(build_dep.clone());
         }
-        dep_set.dependencies.push(build_dep)
+        dep_set.dependencies.insert(build_dep)
       }
       kind => {
         return Err(

impl/src/testing.rs

@@ -50,6 +50,8 @@ pub mod templates {
   pub const SEMVER_MATCHING: &str = "semver_matching.json.template";
   pub const SUBPLAN_PRODUCES_CRATE_ROOT_WITH_FORWARD_SLASH: &str =
     "subplan_produces_crate_root_with_forward_slash.json.template";
+  pub const SUBPLAN_OMITS_PLATFORM_DEPS_ALREADY_IN_DEFAULT_DEPS: &str =
+    "subplan_omits_platform_deps_already_in_default_deps.json.template";
 }
 
 pub const fn basic_toml_contents() -> &'static str {