Skip to content

Commit

Permalink
fix: deduplicate dependencies by artifact target
Browse files Browse the repository at this point in the history
Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
  • Loading branch information
rvolosatovs committed Dec 13, 2022
1 parent 440517b commit e151080
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 11 deletions.
1 change: 1 addition & 0 deletions src/cargo/core/compiler/standard_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ pub fn generate_std_roots(
/*is_std*/ true,
/*dep_hash*/ 0,
IsArtifact::No,
None,
));
}
}
Expand Down
16 changes: 14 additions & 2 deletions src/cargo/core/compiler/unit.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use crate::core::compiler::{unit_dependencies::IsArtifact, CompileKind, CompileMode, CrateType};
use crate::core::compiler::unit_dependencies::IsArtifact;
use crate::core::compiler::{CompileKind, CompileMode, CompileTarget, CrateType};
use crate::core::manifest::{Target, TargetKind};
use crate::core::{profiles::Profile, Package};
use crate::core::profiles::Profile;
use crate::core::Package;
use crate::util::hex::short_hash;
use crate::util::interning::InternedString;
use crate::util::Config;
Expand Down Expand Up @@ -72,6 +74,14 @@ pub struct UnitInner {
/// This value initially starts as 0, and then is filled in via a
/// second-pass after all the unit dependencies have been computed.
pub dep_hash: u64,

/// This is only set for artifact dependencies which have their
/// `<target-triple>|target` set.
/// If so, this information is used as part of the key for resolving their features,
/// allowing for target-dependent feature resolution within the entire dependency tree.
/// Note that this target corresponds to the target used to build the units in that
/// dependency tree, too, but this copy of it is specifically used for feature lookup.
pub artifact_target_for_features: Option<CompileTarget>,
}

impl UnitInner {
Expand Down Expand Up @@ -184,6 +194,7 @@ impl UnitInterner {
is_std: bool,
dep_hash: u64,
artifact: IsArtifact,
artifact_target_for_features: Option<CompileTarget>,
) -> Unit {
let target = match (is_std, target.kind()) {
// This is a horrible hack to support build-std. `libstd` declares
Expand Down Expand Up @@ -216,6 +227,7 @@ impl UnitInterner {
is_std,
dep_hash,
artifact,
artifact_target_for_features,
});
Unit { inner }
}
Expand Down
6 changes: 6 additions & 0 deletions src/cargo/core/compiler/unit_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,11 @@ fn new_unit_dep_with_profile(
.resolve()
.is_public_dep(parent.pkg.package_id(), pkg.package_id());
let features_for = unit_for.map_to_features_for(artifact);
let artifact_target_for_features = if let FeaturesFor::ArtifactDep(target) = features_for {
Some(target)
} else {
None
};
let features = state.activated_features(pkg.package_id(), features_for);
let unit = state.interner.intern(
pkg,
Expand All @@ -896,6 +901,7 @@ fn new_unit_dep_with_profile(
state.is_std,
/*dep_hash*/ 0,
artifact.map_or(IsArtifact::No, |_| IsArtifact::Yes),
artifact_target_for_features,
);
Ok(UnitDep {
unit,
Expand Down
23 changes: 14 additions & 9 deletions src/cargo/ops/cargo_compile/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,10 +428,14 @@ 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);
if host_kind_requested
|| unit_graph
.iter()
.any(|unit| unit.0.artifact_target_for_features.is_some())
{
// Rebuild the unit graph, replacing the explicit host targets with
// CompileKind::Host, merging any dependencies shared with build
Expand All @@ -441,7 +445,7 @@ pub fn create_bcx<'a, 'cfg>(
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 @@ -575,7 +579,7 @@ fn rebuild_unit_graph_shared(
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 @@ -608,7 +612,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 @@ -628,10 +632,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 All @@ -643,6 +646,7 @@ fn traverse_and_share(
unit.is_std,
new_dep_hash,
unit.artifact,
None,
);
assert!(memo.insert(unit.clone(), new_unit.clone()).is_none());
new_graph.entry(new_unit.clone()).or_insert(new_deps);
Expand Down Expand Up @@ -801,6 +805,7 @@ fn override_rustc_crate_types(
unit.is_std,
unit.dep_hash,
unit.artifact,
unit.artifact_target_for_features,
)
};
units[0] = match unit.target.kind() {
Expand Down
1 change: 1 addition & 0 deletions src/cargo/ops/cargo_compile/unit_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ impl<'a> UnitGenerator<'a, '_> {
/*is_std*/ false,
/*dep_hash*/ 0,
IsArtifact::No,
None,
)
})
.collect()
Expand Down

0 comments on commit e151080

Please sign in to comment.