Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bindeps: wrong features in a build script for a shared dependency #10452

Closed
ehuss opened this issue Mar 3, 2022 · 1 comment
Closed

bindeps: wrong features in a build script for a shared dependency #10452

ehuss opened this issue Mar 3, 2022 · 1 comment
Labels
Z-bindeps Nightly: binary artifact dependencies

Comments

@ehuss
Copy link
Contributor

ehuss commented Mar 3, 2022

With -Zbindeps, if there is a dependency that is used as a binary artifact, and also as a normal dependency, the build script for that shared dependency is receiving the wrong features.

In the following example, common gets built twice, once as the target bindep with feature f2, and once as a normal dependency with feature f1. However, the build script only has CARGO_FEATURE_F1 set in both cases.

#[cargo_test]
fn build_script_features_for_shared_dependency() {
    // When a build script is built and run, its features should match. Here:
    //
    // foo
    //   -> artifact on d1 with target
    //   -> common with features f1
    //
    // d1
    //   -> common with features f2
    //
    // common has features f1 and f2, with a build script.
    //
    // When common is built as a dependency of d1, it should have features
    // `f2` (for the library and the build script).
    //
    // When common is built as a dependency of foo, it should have features
    // `f1` (for the library and the build script).
    if cross_compile::disabled() {
        return;
    }
    let target = cross_compile::alternate();
    let p = project()
        .file(
            "Cargo.toml",
            &r#"
                [project]
                name = "foo"
                version = "0.0.1"
                resolver = "2"

                [dependencies]
                d1 = { path = "d1", artifact = "bin", target = "$TARGET" }
                common = { path = "common", features = ["f1"] }
            "#
            .replace("$TARGET", target),
        )
        .file(
            "src/main.rs",
            r#"
                fn main() {
                    let _b = include_bytes!(env!("CARGO_BIN_FILE_D1"));
                    common::f1();
                }
            "#,
        )
        .file(
            "d1/Cargo.toml",
            r#"
                [package]
                name = "d1"
                version = "0.0.1"

                [dependencies]
                common = { path = "../common", features = ["f2"] }
            "#,
        )
        .file(
            "d1/src/main.rs",
            r#"fn main() {
                common::f2();
            }"#,
        )
        .file(
            "common/Cargo.toml",
            r#"
                [package]
                name = "common"
                version = "0.0.1"

                [features]
                f1 = []
                f2 = []
            "#,
        )
        .file(
            "common/src/lib.rs",
            r#"
                #[cfg(feature = "f1")]
                pub fn f1() {}

                #[cfg(feature = "f2")]
                pub fn f2() {}
            "#,
        )
        .file(
            "common/build.rs",
            &r#"
                use std::env::var_os;
                fn main() {
                    if std::env::var("TARGET").unwrap() == "$TARGET" {
                        assert!(var_os("CARGO_FEATURE_F1").is_none());
                        assert!(var_os("CARGO_FEATURE_F2").is_some());
                    } else {
                        assert!(var_os("CARGO_FEATURE_F1").is_some());
                        assert!(var_os("CARGO_FEATURE_F2").is_none());
                    }
                    assert_eq!(var_os("CARGO_FEATURE_F1").is_some(), cfg!(feature="f1"));
                    assert_eq!(var_os("CARGO_FEATURE_F2").is_some(), cfg!(feature="f2"));
                }
            "#
            .replace("$TARGET", target),
        )
        .build();

    p.cargo("build -Z bindeps")
        .masquerade_as_nightly_cargo()
        .run();
}

cc @Byron

@ehuss ehuss added the Z-bindeps Nightly: binary artifact dependencies label Mar 3, 2022
@Byron
Copy link
Member

Byron commented Mar 5, 2022

That's interesting, wouldn't this mean that the unit-dependencies are built correctly (as common does indeed get built twice with the desired features set), but when the build script is invoked it sees the wrong features nonetheless.

After applying this patch…

diff --git a/tests/testsuite/artifact_dep.rs b/tests/testsuite/artifact_dep.rs
index 12090a245..aa67e085e 100644
--- a/tests/testsuite/artifact_dep.rs
+++ b/tests/testsuite/artifact_dep.rs
@@ -2245,6 +2245,8 @@ fn build_script_features_for_shared_dependency() {
             &r#"
                 use std::env::var_os;
                 fn main() {
+                    assert_eq!(var_os("CARGO_FEATURE_F1").is_some(), cfg!(feature="f1"));
+                    assert_eq!(var_os("CARGO_FEATURE_F2").is_some(), cfg!(feature="f2"));
                     if std::env::var("TARGET").unwrap() == "$TARGET" {
                         assert!(var_os("CARGO_FEATURE_F1").is_none());
                         assert!(var_os("CARGO_FEATURE_F2").is_some());
@@ -2252,15 +2254,13 @@ fn build_script_features_for_shared_dependency() {
                         assert!(var_os("CARGO_FEATURE_F1").is_some());
                         assert!(var_os("CARGO_FEATURE_F2").is_none());
                     }
-                    assert_eq!(var_os("CARGO_FEATURE_F1").is_some(), cfg!(feature="f1"));
-                    assert_eq!(var_os("CARGO_FEATURE_F2").is_some(), cfg!(feature="f2"));
                 }
             "#
             .replace("$TARGET", target),
         )
         .build();
 
-    p.cargo("build -Z bindeps")
+    p.cargo("build -Z bindeps -v")
         .masquerade_as_nightly_cargo()
         .run();
 }

…and running I get this output:

assertion failed: var_os(\"CARGO_FEATURE_F2\").is_none()', common/build.rs:11:25
  stack backtrace:
     0: rust_begin_unwind
               at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/std/src/panicking.rs:498:5
     1: core::panicking::panic_fmt
               at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/panicking.rs:116:14
     2: core::panicking::panic
               at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/panicking.rs:48:5
     3: build_script_build::main
               at ./build.rs:11:25
     4: core::ops::function::FnOnce::call_once
               at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/ops/function.rs:227:5
  note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
warning: build failed, waiting for other jobs to finish...
error: build failed

thread 'artifact_dep::build_script_features_for_shared_dependency' panicked at '
test failed running `/cargo/target/debug/cargo build -Z bindeps -v`
error: process exited with code 101 (expected 0)
--- stdout

--- stderr
   Compiling common v0.0.1 (/cargo/target/tmp/cit/t0/foo/common)
     Running `rustc --crate-name build_script_build common/build.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type bin --emit=dep-info,link -C embed-bitcode=no -C split-debuginfo=unpacked -C debuginfo=2 --cfg 'feature="f1"' --cfg 'feature="f2"' -C metadata=680f222246bd571e -C extra-filename=-680f222246bd571e --out-dir /cargo/target/tmp/cit/t0/foo/target/debug/build/common-680f222246bd571e -L dependency=/cargo/target/tmp/cit/t0/foo/target/debug/deps`
     Running `/cargo/target/tmp/cit/t0/foo/target/debug/build/common-680f222246bd571e/build-script-build`
     Running `/cargo/target/tmp/cit/t0/foo/target/debug/build/common-680f222246bd571e/build-script-build`
error: failed to run custom build command for `common v0.0.1 (/cargo/target/tmp/cit/t0/foo/common)`

Caused by:
  process didn't exit successfully: `/cargo/target/tmp/cit/t0/foo/target/debug/build/common-680f222246bd571e/build-script-build` (exit status: 101)
  --- stderr
  thread 'main' panicked at 'assertion failed: var_os(\"CARGO_FEATURE_F2\").is_none()', common/build.rs:11:25
  stack backtrace:
     0: rust_begin_unwind
               at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/std/src/panicking.rs:498:5
     1: core::panicking::panic_fmt
               at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/panicking.rs:116:14
     2: core::panicking::panic
               at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/panicking.rs:48:5
     3: build_script_build::main
               at ./build.rs:11:25
     4: core::ops::function::FnOnce::call_once
               at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/ops/function.rs:227:5
  note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
warning: build failed, waiting for other jobs to finish...
error: build failed
', tests/testsuite/artifact_dep.rs:2265:10
stack backtrace:
   0: rust_begin_unwind
             at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/std/src/panicking.rs:498:5
   1: core::panicking::panic_fmt
             at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/panicking.rs:116:14
   2: cargo_test_support::panic_error::pe
             at ./crates/cargo-test-support/src/lib.rs:48:9
   3: cargo_test_support::panic_error
             at ./crates/cargo-test-support/src/lib.rs:40:5
   4: cargo_test_support::Execs::run
             at ./crates/cargo-test-support/src/lib.rs:802:13
   5: testsuite::artifact_dep::build_script_features_for_shared_dependency
             at ./tests/testsuite/artifact_dep.rs:2263:5
   6: testsuite::artifact_dep::build_script_features_for_shared_dependency::{{closure}}
             at ./tests/testsuite/artifact_dep.rs:2158:1
   7: core::ops::function::FnOnce::call_once
             at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/ops/function.rs:227:5
   8: core::ops::function::FnOnce::call_once
             at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/ops/function.rs:227:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

What I see from the above is that

  • it runs the build script twice
  • it fails for the second run

That doesn't make much sense to me as if the unit dependencies are configured correctly, they would be built and invoked correctly. Maybe the issue is indeed caused by the way features are resolved, there is a bunch of special cases about placing features into a different key so later queries will succeed, those features might be wrong.

bors added a commit that referenced this issue Mar 16, 2022
Fix panic when artifact target is used for `[target.'cfg(<target>)'.dependencies`

With an artifact dependency like this in package `a`…

```toml
[dependencies.a]
path = "b"
artifact = "bin"
target = "$TARGET"
```

…and when using `$TARGET` like this in another package `b`…

```toml
[target.'cfg(target_arch = "$ARCHOF_$TARGET")'.dependencies]
c = { path = "../c" }
```

…it panics with `thread 'main' panicked at 'activated_features for invalid package: features did not find PackageId <dbg-info>`, but we would expect this to work normally.

### Tasks

- [x] reproduce issue in new test
   - [x] figure out why the test is fixed but the real-world example isn't
- [x] find a fix

Fixes #10431 and  #10452.
@bors bors closed this as completed in 0b53b1b Mar 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Z-bindeps Nightly: binary artifact dependencies
Projects
None yet
Development

No branches or pull requests

2 participants