Skip to content

Commit

Permalink
fix(bindeps): target field specified and optional = true coexist
Browse files Browse the repository at this point in the history
> Adapted from rust-lang#11183

Previously, `is_dep_activated` depends on `activated_dependencies`,
which is a map of `PackageFeaturesKey` to its own activated `DepFeature`s.
`PackageFeaturesKey` in feature resolver is always comprised of

* A `PackageId` of a given dependency, and
* A enum value that helps decoupling activated features for that dependency.
  Currently depending on the type of given dependency.

Looking into issue 10526, it has an `activated_dependencies` of

```
{
    (mycrate, NormalOrDev) -> [dep:mybindep]
}
```

However, this [line][1] accidentally use a parent's `pkgid`
and a dependency's `FeaturesFor` to compose a key:

```
(mycrate, ArtifactDep("x86_64-unknown-linux-gnu"))
```

That is never a valid key to query features for artifacts dependency.
A valid key should be comprised of components both from the parent:

```
(mycrate, NormalOrDev)
```

Or both from the dependency itself:

```
(mybindep, ArtifactDep("x86_64-unknown-linux-gnu"))
```

This `unit_for` should already contain its own artifact dep
information inside `artifact_target_for_features`,
so we don't need to map any `dep.artifact()` here.

[1]: https://github.com/rust-lang/cargo/blob/a2ea66bea6fe8156444144e98911d073d56c2c0c/src/cargo/core/compiler/unit_dependencies.rs#L1106-L1107
  • Loading branch information
weihanglo committed Nov 28, 2022
1 parent 0668547 commit ce69111
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 6 deletions.
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/unit_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1103,7 +1103,7 @@ impl<'a, 'cfg> State<'a, 'cfg> {
// If this is an optional dependency, and the new feature resolver
// did not enable it, don't include it.
if dep.is_optional() {
let features_for = unit_for.map_to_features_for(dep.artifact());
let features_for = unit_for.map_to_features_for(None);
if !self.is_dep_activated(pkg_id, features_for, dep.name_in_toml()) {
return false;
}
Expand Down
12 changes: 7 additions & 5 deletions tests/testsuite/artifact_dep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2345,7 +2345,6 @@ fn calc_bin_artifact_fingerprint() {

#[cargo_test]
fn build_with_target_and_optional() {
// This is a incorrect behaviour got to be fixed.
// See rust-lang/cargo#10526
if cross_compile::disabled() {
return;
Expand Down Expand Up @@ -2386,11 +2385,14 @@ fn build_with_target_and_optional() {

p.cargo("build -Z bindeps -F d1 -v")
.masquerade_as_nightly_cargo(&["bindeps"])
.with_stderr_contains(
.with_stderr(
"\
[ERROR] environment variable `CARGO_BIN_FILE_D1` not defined
[COMPILING] d1 v0.0.1 [..]
[RUNNING] `rustc --crate-name d1 [..]--crate-type bin[..]
[COMPILING] foo v0.0.1 [..]
[RUNNING] `rustc --crate-name foo [..]--cfg[..]d1[..]
[FINISHED] dev [..]
",
)
.with_status(101)
)
.run();
}

0 comments on commit ce69111

Please sign in to comment.