Skip to content

Commit bdc4f31

Browse files
committed
fix: deduplicate dependencies by artifact target
Signed-off-by: Roman Volosatovs <[email protected]>
1 parent dca8c28 commit bdc4f31

File tree

5 files changed

+48
-19
lines changed

5 files changed

+48
-19
lines changed

src/cargo/core/compiler/standard_lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,7 @@ pub fn generate_std_roots(
214214
/*is_std*/ true,
215215
/*dep_hash*/ 0,
216216
IsArtifact::No,
217+
None,
217218
));
218219
}
219220
}

src/cargo/core/compiler/unit.rs

+15-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1-
use crate::core::compiler::{unit_dependencies::IsArtifact, CompileKind, CompileMode, CrateType};
1+
use crate::core::compiler::unit_dependencies::IsArtifact;
2+
use crate::core::compiler::{CompileKind, CompileMode, CompileTarget, CrateType};
23
use crate::core::manifest::{Target, TargetKind};
3-
use crate::core::{profiles::Profile, Package};
4+
use crate::core::profiles::Profile;
5+
use crate::core::Package;
46
use crate::util::hex::short_hash;
57
use crate::util::interning::InternedString;
68
use crate::util::Config;
@@ -72,6 +74,11 @@ pub struct UnitInner {
7274
/// This value initially starts as 0, and then is filled in via a
7375
/// second-pass after all the unit dependencies have been computed.
7476
pub dep_hash: u64,
77+
78+
/// This is used for target-dependent feature resolution and is copied from
79+
/// [`FeaturesFor::ArtifactDep`](crate::core::resolver::features::FeaturesFor::ArtifactDep), if the enum
80+
/// matches the variant.
81+
pub artifact_target_for_features: Option<CompileTarget>,
7582
}
7683

7784
impl UnitInner {
@@ -139,6 +146,10 @@ impl fmt::Debug for Unit {
139146
.field("mode", &self.mode)
140147
.field("features", &self.features)
141148
.field("artifact", &self.artifact.is_true())
149+
.field(
150+
"artifact_target_for_features",
151+
&self.artifact_target_for_features,
152+
)
142153
.field("is_std", &self.is_std)
143154
.field("dep_hash", &self.dep_hash)
144155
.finish()
@@ -184,6 +195,7 @@ impl UnitInterner {
184195
is_std: bool,
185196
dep_hash: u64,
186197
artifact: IsArtifact,
198+
artifact_target_for_features: Option<CompileTarget>,
187199
) -> Unit {
188200
let target = match (is_std, target.kind()) {
189201
// This is a horrible hack to support build-std. `libstd` declares
@@ -216,6 +228,7 @@ impl UnitInterner {
216228
is_std,
217229
dep_hash,
218230
artifact,
231+
artifact_target_for_features,
219232
});
220233
Unit { inner }
221234
}

src/cargo/core/compiler/unit_dependencies.rs

+5
Original file line numberDiff line numberDiff line change
@@ -885,6 +885,10 @@ fn new_unit_dep_with_profile(
885885
.resolve()
886886
.is_public_dep(parent.pkg.package_id(), pkg.package_id());
887887
let features_for = unit_for.map_to_features_for(artifact);
888+
let artifact_target = match features_for {
889+
FeaturesFor::ArtifactDep(target) => Some(target),
890+
_ => None,
891+
};
888892
let features = state.activated_features(pkg.package_id(), features_for);
889893
let unit = state.interner.intern(
890894
pkg,
@@ -896,6 +900,7 @@ fn new_unit_dep_with_profile(
896900
state.is_std,
897901
/*dep_hash*/ 0,
898902
artifact.map_or(IsArtifact::No, |_| IsArtifact::Yes),
903+
artifact_target,
899904
);
900905
Ok(UnitDep {
901906
unit,

src/cargo/ops/cargo_compile/mod.rs

+26-17
Original file line numberDiff line numberDiff line change
@@ -424,20 +424,24 @@ pub fn create_bcx<'a, 'cfg>(
424424
remove_duplicate_doc(build_config, &units, &mut unit_graph);
425425
}
426426

427-
if build_config
427+
let host_kind_requested = build_config
428428
.requested_kinds
429429
.iter()
430-
.any(CompileKind::is_host)
430+
.any(CompileKind::is_host);
431+
if host_kind_requested
432+
|| unit_graph
433+
.iter()
434+
.any(|(unit, _)| unit.artifact_target_for_features.is_some())
431435
{
432436
// Rebuild the unit graph, replacing the explicit host targets with
433-
// CompileKind::Host, merging any dependencies shared with build
434-
// dependencies.
437+
// CompileKind::Host, removing `artifact_target_for_features` and merging any dependencies
438+
// shared with build and artifact dependencies.
435439
let new_graph = rebuild_unit_graph_shared(
436440
interner,
437441
unit_graph,
438442
&units,
439443
&scrape_units,
440-
explicit_host_kind,
444+
host_kind_requested.then_some(explicit_host_kind),
441445
);
442446
// This would be nicer with destructuring assignment.
443447
units = new_graph.0;
@@ -549,29 +553,31 @@ pub fn create_bcx<'a, 'cfg>(
549553
/// This is used to rebuild the unit graph, sharing host dependencies if possible.
550554
///
551555
/// This will translate any unit's `CompileKind::Target(host)` to
552-
/// `CompileKind::Host` if the kind is equal to `to_host`. This also handles
553-
/// generating the unit `dep_hash`, and merging shared units if possible.
556+
/// `CompileKind::Host` if `to_host` is not `None` and the kind is equal to `to_host`.
557+
/// This also handles generating the unit `dep_hash`, and merging shared units if possible.
554558
///
555559
/// This is necessary because if normal dependencies used `CompileKind::Host`,
556560
/// there would be no way to distinguish those units from build-dependency
557-
/// units. This can cause a problem if a shared normal/build dependency needs
561+
/// units or artifact dependency units.
562+
/// This can cause a problem if a shared normal/build/artifact dependency needs
558563
/// to link to another dependency whose features differ based on whether or
559-
/// not it is a normal or build dependency. If both units used
564+
/// not it is a normal, build or artifact dependency. If all units used
560565
/// `CompileKind::Host`, then they would end up being identical, causing a
561566
/// collision in the `UnitGraph`, and Cargo would end up randomly choosing one
562567
/// value or the other.
563568
///
564-
/// The solution is to keep normal and build dependencies separate when
569+
/// The solution is to keep normal, build and artifact dependencies separate when
565570
/// building the unit graph, and then run this second pass which will try to
566571
/// combine shared dependencies safely. By adding a hash of the dependencies
567572
/// to the `Unit`, this allows the `CompileKind` to be changed back to `Host`
568-
/// without fear of an unwanted collision.
573+
/// and `artifact_target_for_features` to be removed without fear of an unwanted
574+
/// collision for build or artifact dependencies.
569575
fn rebuild_unit_graph_shared(
570576
interner: &UnitInterner,
571577
unit_graph: UnitGraph,
572578
roots: &[Unit],
573579
scrape_units: &[Unit],
574-
to_host: CompileKind,
580+
to_host: Option<CompileKind>,
575581
) -> (Vec<Unit>, Vec<Unit>, UnitGraph) {
576582
let mut result = UnitGraph::new();
577583
// Map of the old unit to the new unit, used to avoid recursing into units
@@ -604,7 +610,7 @@ fn traverse_and_share(
604610
new_graph: &mut UnitGraph,
605611
unit_graph: &UnitGraph,
606612
unit: &Unit,
607-
to_host: CompileKind,
613+
to_host: Option<CompileKind>,
608614
) -> Unit {
609615
if let Some(new_unit) = memo.get(unit) {
610616
// Already computed, no need to recompute.
@@ -624,10 +630,9 @@ fn traverse_and_share(
624630
})
625631
.collect();
626632
let new_dep_hash = dep_hash.finish();
627-
let new_kind = if unit.kind == to_host {
628-
CompileKind::Host
629-
} else {
630-
unit.kind
633+
let new_kind = match to_host {
634+
Some(to_host) if to_host == unit.kind => CompileKind::Host,
635+
_ => unit.kind,
631636
};
632637
let new_unit = interner.intern(
633638
&unit.pkg,
@@ -639,6 +644,9 @@ fn traverse_and_share(
639644
unit.is_std,
640645
new_dep_hash,
641646
unit.artifact,
647+
// Since `dep_hash` is now filled in, there's no need to specify the artifact target
648+
// for target-dependent feature resolution
649+
None,
642650
);
643651
assert!(memo.insert(unit.clone(), new_unit.clone()).is_none());
644652
new_graph.entry(new_unit.clone()).or_insert(new_deps);
@@ -797,6 +805,7 @@ fn override_rustc_crate_types(
797805
unit.is_std,
798806
unit.dep_hash,
799807
unit.artifact,
808+
unit.artifact_target_for_features,
800809
)
801810
};
802811
units[0] = match unit.target.kind() {

src/cargo/ops/cargo_compile/unit_generator.rs

+1
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,7 @@ impl<'a> UnitGenerator<'a, '_> {
171171
/*is_std*/ false,
172172
/*dep_hash*/ 0,
173173
IsArtifact::No,
174+
None,
174175
)
175176
})
176177
.collect()

0 commit comments

Comments
 (0)