Skip to content

Commit

Permalink
Auto merge of #14723 - elchukc:enhance_download_accessible, r=epage
Browse files Browse the repository at this point in the history
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`
  • Loading branch information
bors committed Oct 30, 2024
2 parents 9abcaef + d125262 commit e09a5b8
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 22 deletions.
53 changes: 39 additions & 14 deletions src/cargo/core/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -499,17 +499,18 @@ impl<'gctx> PackageSet<'gctx> {
force_all_targets: ForceAllTargets,
) -> CargoResult<()> {
fn collect_used_deps(
used: &mut BTreeSet<PackageId>,
used: &mut BTreeSet<(PackageId, CompileKind)>,
resolve: &Resolve,
pkg_id: PackageId,
has_dev_units: HasDevUnits,
requested_kinds: &[CompileKind],
requested_kind: CompileKind,
target_data: &RustcTargetData<'_>,
force_all_targets: ForceAllTargets,
) -> CargoResult<()> {
if !used.insert(pkg_id) {
if !used.insert((pkg_id, requested_kind)) {
return Ok(());
}
let requested_kinds = &[requested_kind];
let filtered_deps = PackageSet::filter_deps(
pkg_id,
resolve,
Expand All @@ -518,16 +519,34 @@ impl<'gctx> PackageSet<'gctx> {
target_data,
force_all_targets,
);
for (pkg_id, _dep) in filtered_deps {
for (pkg_id, deps) in filtered_deps {
collect_used_deps(
used,
resolve,
pkg_id,
has_dev_units,
requested_kinds,
requested_kind,
target_data,
force_all_targets,
)?;
let artifact_kinds = deps.iter().filter_map(|dep| {
Some(
dep.artifact()?
.target()?
.to_resolved_compile_kind(*requested_kinds.iter().next().unwrap()),
)
});
for artifact_kind in artifact_kinds {
collect_used_deps(
used,
resolve,
pkg_id,
has_dev_units,
artifact_kind,
target_data,
force_all_targets,
)?;
}
}
Ok(())
}
Expand All @@ -538,16 +557,22 @@ impl<'gctx> PackageSet<'gctx> {
let mut to_download = BTreeSet::new();

for id in root_ids {
collect_used_deps(
&mut to_download,
resolve,
*id,
has_dev_units,
requested_kinds,
target_data,
force_all_targets,
)?;
for requested_kind in requested_kinds {
collect_used_deps(
&mut to_download,
resolve,
*id,
has_dev_units,
*requested_kind,
target_data,
force_all_targets,
)?;
}
}
let to_download = to_download
.into_iter()
.map(|(p, _)| p)
.collect::<BTreeSet<_>>();
self.get_many(to_download.into_iter())?;
Ok(())
}
Expand Down
25 changes: 17 additions & 8 deletions tests/testsuite/artifact_dep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1646,16 +1646,16 @@ fn dep_of_artifact_dep_same_target_specified() {
.with_status(0)
.run();

// TODO This command currently fails due to a bug in cargo but it should be fixed so that it succeeds in the future.
p.cargo("tree -Z bindeps")
.masquerade_as_nightly_cargo(&["bindeps"])
.with_stderr_data(
.with_stdout_data(
r#"...
no entry found for key
...
foo v0.1.0 ([ROOT]/foo)
└── bar v0.1.0 ([ROOT]/foo/bar)
└── baz v0.1.0 ([ROOT]/foo/baz)
"#,
)
.with_status(101)
.with_status(0)
.run();
}

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

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

p.cargo("check -Z bindeps")
.masquerade_as_nightly_cargo(&["bindeps"])
.with_stderr_data(str![[r#""#]])
.with_stderr_data(
r#"...
[UPDATING] `dummy-registry` index
[LOCKING] 2 packages to latest compatible versions
[DOWNLOADING] crates ...
[ERROR] failed to download from `[ROOTURL]/dl/pm/1.0.0/download`
Caused by:
[37] Could[..]t read a file:// file (Couldn't open file [ROOT]/dl/pm/1.0.0/download)
"#,
)
.with_status(101)
.run();
}

Expand Down

0 comments on commit e09a5b8

Please sign in to comment.