Skip to content

Commit

Permalink
Auto merge of #14593 - elchukc:fix_cargo_tree_bindep_crosscompile, r=…
Browse files Browse the repository at this point in the history
…weihanglo

Fix panic when running cargo tree on a package with a cross compiled bindep

### What does this PR try to resolve?

This is an attempt to close out `@rukai's` [PR](#13207 (comment)) for #12358 and #10593 by adjusting the new integration test and resolving merge conflicts.

I have also separated the changes into atomic commits as per [previous review](#13207 (comment)).

### How should we test and review this PR?

The integration test that has been edited here is sufficient, plus the new integration test that confirms a more specific case where `cargo tree` throws an error.

### Additional information

I have confirmed the test `artifact_dep_target_specified` fails on master branch and succeeds on this branch.

The first commit fixes the panic and the integration test. Commits 2 and 3 add other tests that confirm behaviour mentioned in related issues.

Commits:
1. [Fix panic when running cargo tree on a package with a cross compiled bindep](5c5ea78) - fixes some panics and changes the integration test to succeed
2. [test: cargo tree panic on artifact dep target deactivated](ed294ab) - adds test to confirm the behaviour for the specific panic from [#10539 (comment)](#10593 (comment))
  • Loading branch information
bors committed Oct 11, 2024
2 parents 1ea02ff + ed294ab commit 1e5bad3
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 9 deletions.
30 changes: 26 additions & 4 deletions src/cargo/ops/tree/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -391,10 +391,32 @@ fn add_pkg(
let dep_pkg = graph.package_map[&dep_id];

for dep in deps {
let dep_features_for = if dep.is_build() || dep_pkg.proc_macro() {
FeaturesFor::HostDep
} else {
features_for
let dep_features_for = match dep
.artifact()
.and_then(|artifact| artifact.target())
.and_then(|target| target.to_resolved_compile_target(requested_kind))
{
// Dependency has a `{ …, target = <triple> }`
Some(target) => FeaturesFor::ArtifactDep(target),
// Get the information of the dependent crate from `features_for`.
// If a dependent crate is
//
// * specified as an artifact dep with a `target`, or
// * a host dep,
//
// its transitive deps, including build-deps, need to be built on that target.
None if features_for != FeaturesFor::default() => features_for,
// Dependent crate is a normal dep, then back to old rules:
//
// * normal deps, dev-deps -> inherited target
// * build-deps -> host
None => {
if dep.is_build() || dep_pkg.proc_macro() {
FeaturesFor::HostDep
} else {
features_for
}
}
};
let dep_index = add_pkg(
graph,
Expand Down
89 changes: 84 additions & 5 deletions tests/testsuite/artifact_dep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1568,14 +1568,93 @@ fn artifact_dep_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_stdout_data("")
.with_stderr_data(r#"...
[..]did not find features for (PackageId { name: "bindep", version: "0.0.0", source: "[..]" }, NormalOrDev) within activated_features:[..]
.with_stdout_data(str![[r#"
foo v0.0.0 ([ROOT]/foo)
└── bindep v0.0.0 ([ROOT]/foo/bindep)
"#]])
.with_status(0)
.run();
}

/// From issue #10593
/// The case where:
/// * artifact dep is { target = <specified> }
/// * dependency of that artifact dependency specifies the same target
/// * the target is not activated.
#[cargo_test]
fn dep_of_artifact_dep_same_target_specified() {
if cross_compile::disabled() {
return;
}
let target = cross_compile::alternate();
let p = project()
.file(
"Cargo.toml",
&format!(
r#"
[package]
name = "foo"
version = "0.1.0"
edition = "2015"
resolver = "2"
[dependencies]
bar = {{ path = "bar", artifact = "bin", target = "{target}" }}
"#,
),
)
.file("src/lib.rs", "")
.file(
"bar/Cargo.toml",
&format!(
r#"
[package]
name = "bar"
version = "0.1.0"
[target.{target}.dependencies]
baz = {{ path = "../baz" }}
"#,
),
)
.file("bar/src/main.rs", "fn main() {}")
.file(
"baz/Cargo.toml",
r#"
[package]
name = "baz"
version = "0.1.0"
"#,
)
.file("baz/src/lib.rs", "")
.build();

p.cargo("check -Z bindeps")
.masquerade_as_nightly_cargo(&["bindeps"])
.with_stderr_data(str![[r#"
[LOCKING] 2 packages to latest compatible versions
[COMPILING] baz v0.1.0 ([ROOT]/foo/baz)
[COMPILING] bar v0.1.0 ([ROOT]/foo/bar)
[CHECKING] foo v0.1.0 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
"#]])
.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(
r#"...
no entry found for key
...
"#)
"#,
)
.with_status(101)
.run();
}
Expand Down

0 comments on commit 1e5bad3

Please sign in to comment.