Panic when combining -Zbuild-std with artifact dependencies

[phil-opp]

Problem

The following panic happens when trying to check a project with an artifact dependency that uses build-std:

thread 'main' panicked at 'no entry found for key', src/tools/cargo/src/cargo/core/compiler/unit_dependencies.rs:169:25
stack backtrace:
   0: rust_begin_unwind
             at /rustc/4ce3749235fc31d15ebd444b038a9877e8c700d7/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/4ce3749235fc31d15ebd444b038a9877e8c700d7/library/core/src/panicking.rs:143:14
   2: core::panicking::panic_display
             at /rustc/4ce3749235fc31d15ebd444b038a9877e8c700d7/library/core/src/panicking.rs:72:5
   3: core::panicking::panic_str
             at /rustc/4ce3749235fc31d15ebd444b038a9877e8c700d7/library/core/src/panicking.rs:56:5
   4: core::option::expect_failed
             at /rustc/4ce3749235fc31d15ebd444b038a9877e8c700d7/library/core/src/option.rs:1852:5
   5: cargo::core::compiler::unit_dependencies::build_unit_dependencies
   6: cargo::ops::cargo_compile::create_bcx
   7: cargo::ops::cargo_compile::compile_ws
   8: cargo::ops::cargo_compile::compile
   9: cargo::commands::check::exec
  10: cargo::cli::main
  11: cargo::main

Steps

1. cargo new --lib artifact-dep-test
2. cd artifact-dep-test
3. cargo new --bin sub
4. In the top-level Cargo.toml, add the following dependency:

   sub = { path = "sub", artifact = ["bin"], target = "x86_64-unknown-none" } 

5. Run:

   cargo +nightly check -Zbindeps -Zbuild-std --target x86_64-unknown-linux-gnu

   Instead of x86_64-unknown-linux-gnu, use your host architecture. (Specifying it explicitly is currently required for build-std.)

Possible Solution(s)

No response

Notes

The issue also happens on the latest master (commit 5f611af). The backtrace in debug mode is:

thread 'main' panicked at 'no entry found for key', src/cargo/core/compiler/unit_dependencies.rs:169: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_display
             at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/panicking.rs:72:5
   3: core::panicking::panic_str
             at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/panicking.rs:56:5
   4: core::option::expect_failed
             at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/option.rs:1817:5
   5: core::option::Option<T>::expect
             at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/option.rs:692:21
   6: <std::collections::hash::map::HashMap<K,V,S> as core::ops::index::Index<&Q>>::index
             at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/std/src/collections/hash/map.rs:1164:9
   7: cargo::core::compiler::unit_dependencies::attach_std_deps
             at ./src/cargo/core/compiler/unit_dependencies.rs:169:25
   8: cargo::core::compiler::unit_dependencies::build_unit_dependencies
             at ./src/cargo/core/compiler/unit_dependencies.rs:125:9
   9: cargo::ops::cargo_compile::create_bcx
             at ./src/cargo/ops/cargo_compile.rs:578:26
  10: cargo::ops::cargo_compile::compile_ws
             at ./src/cargo/ops/cargo_compile.rs:285:15
  11: cargo::ops::cargo_compile::compile_with_exec
             at ./src/cargo/ops/cargo_compile.rs:276:5
  12: cargo::ops::cargo_compile::compile
             at ./src/cargo/ops/cargo_compile.rs:265:5
  13: cargo::commands::check::exec
             at ./src/bin/cargo/commands/check.rs:51:5
  14: cargo::cli::execute_subcommand
             at ./src/bin/cargo/cli.rs:369:16
  15: cargo::cli::main
             at ./src/bin/cargo/cli.rs:176:5
  16: cargo::main
             at ./src/bin/cargo/main.rs:39:13
  17: core::ops::function::FnOnce::call_once
             at /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/ops/function.rs:227:5

Version

cargo 1.59.0 (49d8809dc 2022-02-10)
release: 1.59.0
commit-hash: 49d8809dc2d3e6e0d5ec634fcf26d8e2aab67130
commit-date: 2022-02-10
host: x86_64-unknown-linux-gnu
libgit2: 1.3.0 (sys:0.13.23 vendored)
libcurl: 7.80.0-DEV (sys:0.4.51+curl-7.80.0 vendored ssl:OpenSSL/1.1.1l)
os: Pop!_OS 21.10 (impish) [64-bit]

[ehuss]

Thanks for the report! I forgot to open this issue.

Handling build-std will likely take a little bit of refactoring, but shouldn't be too hard.

An outline of the problem:

• generate_std_roots generates the root units of std to compile. However, this is only based on the --target list of targets. Instead, it needs to build the set of std to build based on all targets that will be used. I'm not sure of the best way to collect that list of targets, but it shouldn't be too hard. One option is to walk the dependency graph. Another option is to shift the building of std roots into build_unit_dependencies and lazily calculate the roots as needed. I'm not sure which approach will be better.
• attach_std_deps will need to be smarter about how it attaches std roots into the graph. It should only attach roots that match the target kind. For example, package foo built for x86_64-unknown-none should only have the x86_64-unknown-none std roots attached to it.

cc @Byron

[ehuss]

Oh, I should also link to #9451 which has essentially the same problem. It would be good to fix it for both scenarios at the same time. That also just needs to collect the list of additional targets that need to be built.

[Byron]

Thanks a lot for the outline, this should help a lot when attempting a fix.

Collecting a list of targets reliably is something that is also required for dependency and feature resolution, which I think is related to #10405 as it should mean that all targets to build for would be known as part of the crate metadata from the index.

One option is to walk the dependency graph.

My feeling is that collecting all targets there (and maybe even once) should be enough instead of having to collect them multiple times. Now, there probably is a reason that isn't the case (yet), but I thought I'd mention it here in case it's a possibility at all to avoid multiple dependency walks and repetition.

[bstrie]

I'm kind of unclear on how build-std should interact with bindeps. If you just wanted to build-std for the bindep, and wanted to use the shipped std for everything else, how would you express that? Alternatively if you wanted to build-std the top-level project but use the shipped std for the bindep, how would you express that?

[phil-opp]

@bstrie Yeah, that is something that we still need to figure out. I proposed two possible solutions:

1. Allow to set build-std and build-std-features per package in the Cargo.toml.
   • Proposed as part of https://internals.rust-lang.org/t/proposal-move-some-cargo-config-settings-to-cargo-toml/13336
   • There is already a draft PR by @fee1-dead: Move build-std to Cargo.toml #10308
   • The cargo team discussed this PR in: https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/build-std.20in.20Cargo.2Etoml
   • Thinking about this more, maybe it makes sense to put this into the [[bin]] section instead, since different executables could have different requirements?
2. Add target-specific config options for build-std/build-std-features via the [target] table in .cargo/config.toml files.
   • Might be simpler to implement and we would keep the unstable options out of the Cargo.toml files.
   • Discussion: https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/build-std.20per.20target.3F

Do you have other ideas?

(Maybe it makes sense to discuss this somewhere else to keep this issue thread clean?)

[bstrie]

Great, I just wanted to confirm that this is something that people are thinking about.

(Maybe it makes sense to discuss this somewhere else to keep this issue thread clean?)

Sure, is there a tracking issue for that?

WRT this issue, my main question is whether or not this is something that would need to be resolved before the stabilization of artifact dependencies, which I hope in my eternally boundless optimism might be thinkable in the near-ish term (or at least nearer than build-std seems to be).

[devsnek]

I have a project where I have a root package which has a [build-dependencies] bindep entry for another package (fwiw, only because cargo lacks post-build scripts ☹️). this other package is no_std and x86_64-unknown-none. If I try to use a package that needs std in the sub-package's build.rs/[build-dependencies], i get errors like:

error[E0405]: cannot find trait `Drop` in this scope
   --> /home/snek/.cargo/registry/src/gitpro.ttaallkk.top-1ecc6299db9ec823/crossbeam-utils-0.8.14/src/sync/sharded_lock.rs:606:6
    |
606 | impl Drop for Registration {
    |      ^^^^ not found in this scope
    |
help: consider importing this trait
    |
1   | use core::ops::Drop;
    |

It seems like whatever resolved the dependencies for the sub package's build deps are being inferred to need -Zbuild-std because of the x86_64-unknown-none, even though it should just be using the host target?

[JonasKruckenberg]

Had some time this morning and attempted a fix here #14480 which makes this work at least for usage in dependencies and build-dependencies (target.<cfg>.dependencies works fine too, but target.<cfg>.build-dependencies seems to still crash with a different error this one #10593, work for another PR I guess 🤷🏻‍♂️)