Skip to content

Commit e09a5b8

Browse files
committed
Auto merge of #14723 - elchukc:enhance_download_accessible, r=epage
download targeted transitive deps of with artifact deps' target platform ### What does this PR try to resolve? Fixes #12554. `download_accessible` will now download platform-specified deps of artifact deps with `target = ...`. It will also resolve the panic in `cargo tree -Z bindeps` in [#10593 (comment)](#10593 (comment)), where: - a dependency of an artifact dependency is platform specified - artifact dep itself is { target = } with the same platform as its own dependency - the platform is not activated. Essentially, `no entry found for key` was happening because for artifact deps with `{.., target = <target>}`, transitive deps that specified their platform as `<target>` were not downloaded. This is why adding `--target all` to `cargo tree -Z bindeps` made the bug dissapear. ### How should we test and review this PR? Tests included in this PR should be enough. ~~Test `artifact_dep::proc_macro_in_artifact_dep` still throws; this PR will be ready for review once the test does not panic.~~ ### Additional Information `used` set needs to be target-aware r? `@weihanglo`
2 parents 9abcaef + d125262 commit e09a5b8

File tree

2 files changed

+56
-22
lines changed

2 files changed

+56
-22
lines changed

src/cargo/core/package.rs

Lines changed: 39 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -499,17 +499,18 @@ impl<'gctx> PackageSet<'gctx> {
499499
force_all_targets: ForceAllTargets,
500500
) -> CargoResult<()> {
501501
fn collect_used_deps(
502-
used: &mut BTreeSet<PackageId>,
502+
used: &mut BTreeSet<(PackageId, CompileKind)>,
503503
resolve: &Resolve,
504504
pkg_id: PackageId,
505505
has_dev_units: HasDevUnits,
506-
requested_kinds: &[CompileKind],
506+
requested_kind: CompileKind,
507507
target_data: &RustcTargetData<'_>,
508508
force_all_targets: ForceAllTargets,
509509
) -> CargoResult<()> {
510-
if !used.insert(pkg_id) {
510+
if !used.insert((pkg_id, requested_kind)) {
511511
return Ok(());
512512
}
513+
let requested_kinds = &[requested_kind];
513514
let filtered_deps = PackageSet::filter_deps(
514515
pkg_id,
515516
resolve,
@@ -518,16 +519,34 @@ impl<'gctx> PackageSet<'gctx> {
518519
target_data,
519520
force_all_targets,
520521
);
521-
for (pkg_id, _dep) in filtered_deps {
522+
for (pkg_id, deps) in filtered_deps {
522523
collect_used_deps(
523524
used,
524525
resolve,
525526
pkg_id,
526527
has_dev_units,
527-
requested_kinds,
528+
requested_kind,
528529
target_data,
529530
force_all_targets,
530531
)?;
532+
let artifact_kinds = deps.iter().filter_map(|dep| {
533+
Some(
534+
dep.artifact()?
535+
.target()?
536+
.to_resolved_compile_kind(*requested_kinds.iter().next().unwrap()),
537+
)
538+
});
539+
for artifact_kind in artifact_kinds {
540+
collect_used_deps(
541+
used,
542+
resolve,
543+
pkg_id,
544+
has_dev_units,
545+
artifact_kind,
546+
target_data,
547+
force_all_targets,
548+
)?;
549+
}
531550
}
532551
Ok(())
533552
}
@@ -538,16 +557,22 @@ impl<'gctx> PackageSet<'gctx> {
538557
let mut to_download = BTreeSet::new();
539558

540559
for id in root_ids {
541-
collect_used_deps(
542-
&mut to_download,
543-
resolve,
544-
*id,
545-
has_dev_units,
546-
requested_kinds,
547-
target_data,
548-
force_all_targets,
549-
)?;
560+
for requested_kind in requested_kinds {
561+
collect_used_deps(
562+
&mut to_download,
563+
resolve,
564+
*id,
565+
has_dev_units,
566+
*requested_kind,
567+
target_data,
568+
force_all_targets,
569+
)?;
570+
}
550571
}
572+
let to_download = to_download
573+
.into_iter()
574+
.map(|(p, _)| p)
575+
.collect::<BTreeSet<_>>();
551576
self.get_many(to_download.into_iter())?;
552577
Ok(())
553578
}

tests/testsuite/artifact_dep.rs

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1646,16 +1646,16 @@ fn dep_of_artifact_dep_same_target_specified() {
16461646
.with_status(0)
16471647
.run();
16481648

1649-
// TODO This command currently fails due to a bug in cargo but it should be fixed so that it succeeds in the future.
16501649
p.cargo("tree -Z bindeps")
16511650
.masquerade_as_nightly_cargo(&["bindeps"])
1652-
.with_stderr_data(
1651+
.with_stdout_data(
16531652
r#"...
1654-
no entry found for key
1655-
...
1653+
foo v0.1.0 ([ROOT]/foo)
1654+
└── bar v0.1.0 ([ROOT]/foo/bar)
1655+
└── baz v0.1.0 ([ROOT]/foo/baz)
16561656
"#,
16571657
)
1658-
.with_status(101)
1658+
.with_status(0)
16591659
.run();
16601660
}
16611661

@@ -1777,9 +1777,7 @@ perhaps a crate was updated and forgotten to be re-vendored?
17771777
.run();
17781778
}
17791779

1780-
// FIXME: `download_accessible` should work properly for artifact dependencies
17811780
#[cargo_test]
1782-
#[ignore = "broken, needs download_accessible fix"]
17831781
fn proc_macro_in_artifact_dep() {
17841782
// Forcing FeatureResolver to check a proc-macro for a dependency behind a
17851783
// target dependency.
@@ -1829,7 +1827,18 @@ fn proc_macro_in_artifact_dep() {
18291827

18301828
p.cargo("check -Z bindeps")
18311829
.masquerade_as_nightly_cargo(&["bindeps"])
1832-
.with_stderr_data(str![[r#""#]])
1830+
.with_stderr_data(
1831+
r#"...
1832+
[UPDATING] `dummy-registry` index
1833+
[LOCKING] 2 packages to latest compatible versions
1834+
[DOWNLOADING] crates ...
1835+
[ERROR] failed to download from `[ROOTURL]/dl/pm/1.0.0/download`
1836+
1837+
Caused by:
1838+
[37] Could[..]t read a file:// file (Couldn't open file [ROOT]/dl/pm/1.0.0/download)
1839+
"#,
1840+
)
1841+
.with_status(101)
18331842
.run();
18341843
}
18351844

0 commit comments

Comments
 (0)