Skip to content

Commit

Permalink
fix: share artifact and build dependencies
Browse files Browse the repository at this point in the history
This prevents collisions for transitive dependencies

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
  • Loading branch information
rvolosatovs committed Dec 22, 2022
1 parent 548b252 commit 385bba3
Showing 1 changed file with 24 additions and 18 deletions.
42 changes: 24 additions & 18 deletions src/cargo/ops/cargo_compile/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<CompileKind>,
) -> (Vec<Unit>, Vec<Unit>, UnitGraph) {
let mut result = UnitGraph::new();
// Map of the old unit to the new unit, used to avoid recursing into units
Expand Down Expand Up @@ -595,7 +602,7 @@ fn traverse_and_share(
new_graph: &mut UnitGraph,
unit_graph: &UnitGraph,
unit: &Unit,
to_host: CompileKind,
to_host: Option<CompileKind>,
) -> Unit {
if let Some(new_unit) = memo.get(unit) {
// Already computed, no need to recompute.
Expand All @@ -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,
Expand Down

0 comments on commit 385bba3

Please sign in to comment.