From 107f266c50e86d2b6bb74b876c3bab55e5850737 Mon Sep 17 00:00:00 2001 From: Roman Volosatovs Date: Thu, 22 Dec 2022 14:44:05 +0100 Subject: [PATCH] fix: share artifact and build dependencies This prevents collisions for transitive dependencies Signed-off-by: Roman Volosatovs --- src/cargo/ops/cargo_compile/mod.rs | 42 +++++++++++++++++------------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/src/cargo/ops/cargo_compile/mod.rs b/src/cargo/ops/cargo_compile/mod.rs index e251b9357638..3022390dd6ee 100644 --- a/src/cargo/ops/cargo_compile/mod.rs +++ b/src/cargo/ops/cargo_compile/mod.rs @@ -415,20 +415,25 @@ pub fn create_bcx<'a, 'cfg>( remove_duplicate_doc(build_config, &units, &mut unit_graph); } - if build_config + let host_kind_requested = build_config .requested_kinds .iter() - .any(CompileKind::is_host) - { + .any(CompileKind::is_host); + let should_share_deps = host_kind_requested + || config.cli_unstable().bindeps + && unit_graph + .iter() + .any(|(unit, _)| unit.artifact_target_for_features.is_some()); + if should_share_deps { // Rebuild the unit graph, replacing the explicit host targets with - // CompileKind::Host, merging any dependencies shared with build - // dependencies. + // CompileKind::Host, removing `artifact_target_for_features` and merging any dependencies + // shared with build and artifact dependencies. let new_graph = rebuild_unit_graph_shared( interner, unit_graph, &units, &scrape_units, - explicit_host_kind, + host_kind_requested.then_some(explicit_host_kind), ); // This would be nicer with destructuring assignment. units = new_graph.0; @@ -540,29 +545,31 @@ pub fn create_bcx<'a, 'cfg>( /// This is used to rebuild the unit graph, sharing host dependencies if possible. /// /// This will translate any unit's `CompileKind::Target(host)` to -/// `CompileKind::Host` if the kind is equal to `to_host`. This also handles -/// generating the unit `dep_hash`, and merging shared units if possible. +/// `CompileKind::Host` if `to_host` is not `None` and the kind is equal to `to_host`. +/// This also handles generating the unit `dep_hash`, and merging shared units if possible. /// /// This is necessary because if normal dependencies used `CompileKind::Host`, /// there would be no way to distinguish those units from build-dependency -/// units. This can cause a problem if a shared normal/build dependency needs +/// units or artifact dependency units. +/// This can cause a problem if a shared normal/build/artifact dependency needs /// to link to another dependency whose features differ based on whether or -/// not it is a normal or build dependency. If both units used +/// not it is a normal, build or artifact dependency. If all units used /// `CompileKind::Host`, then they would end up being identical, causing a /// collision in the `UnitGraph`, and Cargo would end up randomly choosing one /// value or the other. /// -/// The solution is to keep normal and build dependencies separate when +/// The solution is to keep normal, build and artifact dependencies separate when /// building the unit graph, and then run this second pass which will try to /// combine shared dependencies safely. By adding a hash of the dependencies /// to the `Unit`, this allows the `CompileKind` to be changed back to `Host` -/// without fear of an unwanted collision. +/// and `artifact_target_for_features` to be removed without fear of an unwanted +/// collision for build or artifact dependencies. fn rebuild_unit_graph_shared( interner: &UnitInterner, unit_graph: UnitGraph, roots: &[Unit], scrape_units: &[Unit], - to_host: CompileKind, + to_host: Option, ) -> (Vec, Vec, UnitGraph) { let mut result = UnitGraph::new(); // Map of the old unit to the new unit, used to avoid recursing into units @@ -595,7 +602,7 @@ fn traverse_and_share( new_graph: &mut UnitGraph, unit_graph: &UnitGraph, unit: &Unit, - to_host: CompileKind, + to_host: Option, ) -> Unit { if let Some(new_unit) = memo.get(unit) { // Already computed, no need to recompute. @@ -615,10 +622,9 @@ fn traverse_and_share( }) .collect(); let new_dep_hash = dep_hash.finish(); - let new_kind = if unit.kind == to_host { - CompileKind::Host - } else { - unit.kind + let new_kind = match to_host { + Some(to_host) if to_host == unit.kind => CompileKind::Host, + _ => unit.kind, }; let new_unit = interner.intern( &unit.pkg,