-Z rustdoc-scrape-examples panics with some profile overrides

[lqd]

Problem

The unstable scrape examples feature seems to somehow rely on some implicit reuse behavior that doesn't hold when users override settings in profiles causing dependencies to be built more than once.

I've noticed this with all the scrape-examples tests when changing these defaults to values users can already provide.

Steps

So for example, a simple project setup like the scrape_examples_basic test ("foo/src/lib.rs" & "foo/examples/ex.rs") can make cargo panic by changing the foo crate's profiles like:

1. changing the panic method:

[profile.dev]
panic = "abort"

2. opting into stripping:

[profile.dev]
strip = true

3. changing the build-dependencies profile (strange because that example doesn't seem to have build dependencies ?) with either of the following 3 settings:

[profile.dev.build-override]
debug = 0
strip = true
incremental = false

It's unlikely that this list is exhaustive, as it's more about the fact that the dependencies and deduplication are different when these settings are used, I guess ?

Any of the changes above will make cargo panic like so, on nightly or master:

> cargo doc -Zunstable-options -Z rustdoc-scrape-examples=examples -v
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', src/tools/cargo/src/cargo/ops/cargo_compile.rs:1594:36
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Backtrace

thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', src/cargo/ops/cargo_compile.rs:1596:28
stack backtrace:
   0: rust_begin_unwind
             at /rustc/f0c4da49983aa699f715caf681e3154b445fb60b/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/f0c4da49983aa699f715caf681e3154b445fb60b/library/core/src/panicking.rs:143:14
   2: core::panicking::panic
             at /rustc/f0c4da49983aa699f715caf681e3154b445fb60b/library/core/src/panicking.rs:48:5
   3: core::option::Option<T>::unwrap
             at /rustc/f0c4da49983aa699f715caf681e3154b445fb60b/library/core/src/option.rs:752:21
   4: cargo::ops::cargo_compile::rebuild_unit_graph_shared::{{closure}}
             at ./cargo/src/cargo/ops/cargo_compile.rs:1596:13
   5: core::iter::adapters::map::map_fold::{{closure}}
             at /rustc/f0c4da49983aa699f715caf681e3154b445fb60b/library/core/src/iter/adapters/map.rs:84:28
   6: core::iter::traits::iterator::Iterator::fold
             at /rustc/f0c4da49983aa699f715caf681e3154b445fb60b/library/core/src/iter/traits/iterator.rs:2285:21
   7: <core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::fold
             at /rustc/f0c4da49983aa699f715caf681e3154b445fb60b/library/core/src/iter/adapters/map.rs:124:9
   8: core::iter::traits::iterator::Iterator::for_each
             at /rustc/f0c4da49983aa699f715caf681e3154b445fb60b/library/core/src/iter/traits/iterator.rs:773:9
   9: <alloc::vec::Vec<T,A> as alloc::vec::spec_extend::SpecExtend<T,I>>::spec_extend
             at /rustc/f0c4da49983aa699f715caf681e3154b445fb60b/library/alloc/src/vec/spec_extend.rs:40:17
  10: <alloc::vec::Vec<T> as alloc::vec::spec_from_iter_nested::SpecFromIterNested<T,I>>::from_iter
             at /rustc/f0c4da49983aa699f715caf681e3154b445fb60b/library/alloc/src/vec/spec_from_iter_nested.rs:62:9
  11: <alloc::vec::Vec<T> as alloc::vec::spec_from_iter::SpecFromIter<T,I>>::from_iter
             at /rustc/f0c4da49983aa699f715caf681e3154b445fb60b/library/alloc/src/vec/spec_from_iter.rs:33:9
  12: <alloc::vec::Vec<T> as core::iter::traits::collect::FromIterator<T>>::from_iter
             at /rustc/f0c4da49983aa699f715caf681e3154b445fb60b/library/alloc/src/vec/mod.rs:2552:9
  13: core::iter::traits::iterator::Iterator::collect
             at /rustc/f0c4da49983aa699f715caf681e3154b445fb60b/library/core/src/iter/traits/iterator.rs:1778:9
  14: cargo::ops::cargo_compile::rebuild_unit_graph_shared
             at ./cargo/src/cargo/ops/cargo_compile.rs:1592:28
  15: cargo::ops::cargo_compile::create_bcx
             at ./cargo/src/cargo/ops/cargo_compile.rs:607:25
  16: cargo::ops::cargo_compile::compile_ws
             at ./cargo/src/cargo/ops/cargo_compile.rs:285:15
  17: cargo::ops::cargo_compile::compile_with_exec
             at ./cargo/src/cargo/ops/cargo_compile.rs:276:5
  18: cargo::ops::cargo_compile::compile
             at ./cargo/src/cargo/ops/cargo_compile.rs:265:5
  19: cargo::ops::cargo_doc::doc
             at ./cargo/src/cargo/ops/cargo_doc.rs:20:23
  20: cargo::commands::doc::exec
             at ./cargo/src/bin/cargo/commands/doc.rs:56:5
  21: cargo::cli::execute_subcommand
             at ./cargo/src/bin/cargo/cli.rs:369:16
  22: cargo::cli::main
             at ./cargo/src/bin/cargo/cli.rs:176:5
  23: cargo::main
             at ./cargo/src/bin/cargo/main.rs:37:13
  24: core::ops::function::FnOnce::call_once
             at /rustc/f0c4da49983aa699f715caf681e3154b445fb60b/library/core/src/ops/function.rs:227:5

Version

any recent nighty or master

[lqd]

cc #9910, and @willcrichton

[willcrichton]

I will investigate, thanks for the report @lqd.

[willcrichton]

I just tested and this should be fixed once #10343 gets merged. I'll leave it open until then.

[nnethercote]

Note that this is blocking progress on #10493.

[willcrichton]

Oh, I didn't realize this was blocking. After a quick check, it seems like this diff solves the issue: https://github.com/rust-lang/cargo/pull/10343/files#diff-a78dec059e787d4e9a1a70144c9d2f1824d374e9ab6ccc65ef7aba6bf5b21a32

I can pull it out into a separate PR if needed, or it could just be in @lqd's PR if that will get merged sooner. #10343 will probably wait until upstream rustc PRs get merged and land on stable.

[lqd]

Yesterday, I noticed your "properly memoize Docscrape units" commit, and tried it to move forward with #10493. It did fix the tests, so I started reducing it into a smaller change, and what I ended up with was bigger than just the new_unit_dep_with_profile call you linked. Just that does indeed fix the scrape-examples tests, thanks a bunch Will!

It's unknown which of the two PRs would merge sooner (but you may be right that #10343 could involve more rustc/rustdoc coordination), so I'll try to incorporate this small change as well and add a few of the use-cases in the OP as tests. It's probably going to be transparent whenever either of the two lands and fixes the issue (worst case also causing a small conflict).