Skip to content

Commit 0ad7a77

Browse files
max-hellermax-heller
committed
fix!: correctly determine common deps and features
Currently, "common" deps and features in selects in generated lockfiles are not always common to all target platforms in the presence of conditional dependencies. This change ensures that common deps and features for a crate are common to all supported targets, not just those targets for which the crate dependency is enabled.
1 parent 60515a1 commit 0ad7a77

File tree

10 files changed

+6933
-742
lines changed

10 files changed

+6933
-742
lines changed

‎crate_universe/src/metadata/cargo_tree_resolver.rs‎

Lines changed: 24 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ pub(crate) struct CargoTreeEntry {
6565
}
6666

6767
impl CargoTreeEntry {
68+
#[cfg(test)]
6869
pub fn new() -> Self {
6970
Self {
7071
features: BTreeSet::new(),
@@ -374,48 +375,33 @@ impl TreeResolver {
374375
}
375376
}
376377

377-
// Collect all metadata into a mapping of crate to it's metadata per target.
378+
// Collect all metadata into a mapping of crate to its metadata per target.
378379
let mut result = TreeResolverMetadata::new();
379380
for (crate_id, tree_data) in metadata.into_iter() {
380-
let common = CargoTreeEntry {
381-
features: tree_data
382-
.iter()
383-
.fold(
384-
None,
385-
|common: Option<BTreeSet<String>>, (_, data)| match common {
386-
Some(common) => {
387-
Some(common.intersection(&data.features).cloned().collect())
388-
}
389-
None => Some(data.features.clone()),
390-
},
391-
)
392-
.unwrap_or_default(),
393-
deps: tree_data
394-
.iter()
395-
.fold(
396-
None,
397-
|common: Option<BTreeSet<CrateId>>, (_, data)| match common {
398-
Some(common) => {
399-
Some(common.intersection(&data.deps).cloned().collect())
400-
}
401-
None => Some(data.deps.clone()),
402-
},
403-
)
404-
.unwrap_or_default(),
381+
// Determine the features and deps common to all targets
382+
let common = if target_triples
383+
.iter()
384+
.all(|triple| tree_data.contains_key(triple))
385+
{
386+
let mut tree_data = tree_data.values();
387+
let mut common = tree_data.next().cloned().unwrap_or_default();
388+
for CargoTreeEntry { features, deps } in tree_data {
389+
common.features.retain(|feat| features.contains(feat));
390+
common.deps.retain(|dep| deps.contains(dep));
391+
}
392+
common
393+
} else {
394+
// The crate is not included on all targets, so it cannot have
395+
// any features or deps common to all targets
396+
CargoTreeEntry::default()
405397
};
406398
let mut select: Select<CargoTreeEntry> = Select::default();
407-
for (target_triple, data) in tree_data {
408-
let mut entry = CargoTreeEntry::new();
409-
entry.features.extend(
410-
data.features
411-
.into_iter()
412-
.filter(|f| !common.features.contains(f)),
413-
);
414-
entry
415-
.deps
416-
.extend(data.deps.into_iter().filter(|d| !common.deps.contains(d)));
417-
if !entry.is_empty() {
418-
select.insert(entry, Some(target_triple.to_bazel()));
399+
for (target_triple, mut data) in tree_data {
400+
// Filter out common features and deps to get target-specific features and deps
401+
data.features.retain(|feat| !common.features.contains(feat));
402+
data.deps.retain(|dep| !common.deps.contains(dep));
403+
if !data.is_empty() {
404+
select.insert(data, Some(target_triple.to_bazel()));
419405
}
420406
}
421407
if !common.is_empty() {

‎crate_universe/tests/cargo_integration_test.rs‎

Lines changed: 61 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -152,54 +152,59 @@ fn feature_generator() {
152152

153153
assert_eq!(
154154
json!({
155-
"common": {
156-
"deps": [
157-
"arrayvec 0.7.2",
158-
"bitflags 1.3.2",
159-
"fxhash 0.2.1",
160-
"log 0.4.17",
161-
"naga 0.10.0",
162-
"parking_lot 0.12.1",
163-
"profiling 1.0.7",
164-
"raw-window-handle 0.5.0",
165-
"thiserror 1.0.37",
166-
"wgpu-types 0.14.1",
167-
],
168-
"features": [
169-
"default",
170-
],
171-
},
172155
"selects": {
173156
"x86_64-apple-darwin": {
174157
"deps": [
158+
"arrayvec 0.7.2",
159+
"bitflags 1.3.2",
175160
"block 0.1.6",
176161
"core-graphics-types 0.1.1",
177162
"foreign-types 0.3.2",
163+
"fxhash 0.2.1",
164+
"log 0.4.17",
178165
"metal 0.24.0",
166+
"naga 0.10.0",
179167
"objc 0.2.7",
168+
"parking_lot 0.12.1",
169+
"profiling 1.0.7",
170+
"raw-window-handle 0.5.0",
171+
"thiserror 1.0.37",
172+
"wgpu-types 0.14.1",
180173
],
181174
"features": [
182175
"block",
176+
"default",
183177
"foreign-types",
184178
"metal",
185179
],
186180
},
187181
"x86_64-pc-windows-msvc": {
188182
"deps": [
183+
"arrayvec 0.7.2",
189184
"ash 0.37.1+1.3.235",
190185
"bit-set 0.5.3",
186+
"bitflags 1.3.2",
191187
"d3d12 0.5.0",
188+
"fxhash 0.2.1",
192189
"gpu-alloc 0.5.3",
193190
"gpu-descriptor 0.2.3",
194191
"libloading 0.7.4",
192+
"log 0.4.17",
193+
"naga 0.10.0",
194+
"parking_lot 0.12.1",
195+
"profiling 1.0.7",
195196
"range-alloc 0.1.2",
197+
"raw-window-handle 0.5.0",
196198
"renderdoc-sys 0.7.1",
197199
"smallvec 1.10.0",
200+
"thiserror 1.0.37",
201+
"wgpu-types 0.14.1",
198202
"winapi 0.3.9",
199203
],
200204
"features": [
201205
"ash",
202206
"bit-set",
207+
"default",
203208
"dx11",
204209
"dx12",
205210
"gpu-alloc",
@@ -211,21 +216,33 @@ fn feature_generator() {
211216
"renderdoc-sys",
212217
"smallvec",
213218
"vulkan",
219+
214220
],
215221
},
216222
"x86_64-unknown-linux-gnu": {
217223
"deps": [
224+
"arrayvec 0.7.2",
218225
"ash 0.37.1+1.3.235",
226+
"bitflags 1.3.2",
227+
"fxhash 0.2.1",
219228
"glow 0.11.2",
220229
"gpu-alloc 0.5.3",
221230
"gpu-descriptor 0.2.3",
222231
"khronos-egl 4.1.0",
223232
"libloading 0.7.4",
233+
"log 0.4.17",
234+
"naga 0.10.0",
235+
"parking_lot 0.12.1",
236+
"profiling 1.0.7",
237+
"raw-window-handle 0.5.0",
224238
"renderdoc-sys 0.7.1",
225239
"smallvec 1.10.0",
240+
"thiserror 1.0.37",
241+
"wgpu-types 0.14.1",
226242
],
227243
"features": [
228244
"ash",
245+
"default",
229246
"egl",
230247
"gles",
231248
"glow",
@@ -544,33 +561,38 @@ fn host_specific_build_deps() {
544561

545562
assert_eq!(
546563
json!({
547-
"common": {
548-
"deps": [
549-
"bitflags 2.6.0",
550-
],
551-
"features": [
552-
"alloc",
553-
"default",
554-
"fs",
555-
"libc-extra-traits",
556-
"std",
557-
"use-libc-auxv",
558-
],
559-
},
560564
// Note that there is no `wasm32-unknown-unknown` or `x86_64-pc-windows-msvc` entry
561565
// since these platforms do not depend on `rustix`. The chain breaks due to the
562566
// conditions here: https://github.com/Stebalien/tempfile/blob/v3.11.0/Cargo.toml#L25-L33
563567
"selects": {
564568
"x86_64-apple-darwin": {
565569
"deps": [
570+
"bitflags 2.6.0",
566571
"errno 0.3.9",
567572
"libc 0.2.158",
568573
],
574+
"features": [
575+
"alloc",
576+
"default",
577+
"fs",
578+
"libc-extra-traits",
579+
"std",
580+
"use-libc-auxv",
581+
],
569582
},
570583
"x86_64-unknown-linux-gnu": {
571584
"deps": [
585+
"bitflags 2.6.0",
572586
"linux-raw-sys 0.4.14",
573587
],
588+
"features": [
589+
"alloc",
590+
"default",
591+
"fs",
592+
"libc-extra-traits",
593+
"std",
594+
"use-libc-auxv",
595+
],
574596
},
575597
},
576598
}),
@@ -579,28 +601,30 @@ fn host_specific_build_deps() {
579601

580602
assert_eq!(
581603
json!({
582-
"common": {
583-
"deps": [
584-
"cfg-if 1.0.0",
585-
"fastrand 2.1.1",
586-
"once_cell 1.19.0",
587-
],
588-
},
589604
// Note that windows does not contain `rustix` and instead `windows-sys`.
590605
// This shows correct detection of exec platform constraints.
591606
"selects": {
592607
"x86_64-apple-darwin": {
593608
"deps": [
609+
"cfg-if 1.0.0",
610+
"fastrand 2.1.1",
611+
"once_cell 1.19.0",
594612
"rustix 0.38.36",
595613
],
596614
},
597615
"x86_64-pc-windows-msvc": {
598616
"deps": [
617+
"cfg-if 1.0.0",
618+
"fastrand 2.1.1",
619+
"once_cell 1.19.0",
599620
"windows-sys 0.59.0",
600621
],
601622
},
602623
"x86_64-unknown-linux-gnu": {
603624
"deps": [
625+
"cfg-if 1.0.0",
626+
"fastrand 2.1.1",
627+
"once_cell 1.19.0",
604628
"rustix 0.38.36",
605629
],
606630
},

0 commit comments

Comments
 (0)