x build -h -v panics since #104952

[jyn514]

Code

I tried this code:

x build --help --verbose

I expected to see this happen: A list of all crates that can be built.

Instead, this happened:

running: /home/jyn/src/rust/rust/build/bootstrap/debug/bootstrap b -h -v
thread 'main' panicked at 'metadata missing for test: {}', lib.rs:1399:67
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Version it worked on

It most recently worked on: 8ad447c

Version with regression

88c58e3 / #104952

Backtrace

Backtrace

stack backtrace:
   0: rust_begin_unwind
             at /rustc/e080cc5a659fb760c0bc561b722a790dad35b5e1/library/std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at /rustc/e080cc5a659fb760c0bc561b722a790dad35b5e1/library/core/src/panicking.rs:65:14
   2: bootstrap::Build::in_tree_crates::{{closure}}
             at ./src/bootstrap/lib.rs:1399:67
   3: core::option::Option<T>::unwrap_or_else
             at /rustc/e080cc5a659fb760c0bc561b722a790dad35b5e1/library/core/src/option.rs:828:21
   4: bootstrap::Build::in_tree_crates
             at ./src/bootstrap/lib.rs:1399:25
   5: bootstrap::builder::ShouldRun::crate_or_deps
             at ./src/bootstrap/builder.rs:432:22
   6: <bootstrap::compile::Std as bootstrap::builder::Step>::should_run
             at ./src/bootstrap/compile.rs:57:9
   7: bootstrap::builder::Builder::get_help
             at ./src/bootstrap/builder.rs:798:26
   8: bootstrap::flags::Flags::parse::{{closure}}
             at ./src/bootstrap/flags.rs:357:25
   9: bootstrap::flags::Flags::parse
             at ./src/bootstrap/flags.rs:558:13
  10: bootstrap::config::Config::parse_inner
             at ./src/bootstrap/config.rs:849:21
  11: bootstrap::config::Config::parse
             at ./src/bootstrap/config.rs:845:9
  12: bootstrap::main
             at ./src/bootstrap/bin/main.rs:14:18
  13: core::ops::function::FnOnce::call_once
             at /rustc/e080cc5a659fb760c0bc561b722a790dad35b5e1/library/core/src/ops/function.rs:251:5

[jyn514]

oh, also I changed the error message locally a bit:

diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs
index f84fcd21cfc..5aff33a2fb8 100644
--- a/src/bootstrap/lib.rs
+++ b/src/bootstrap/lib.rs
@@ -1400,7 +1396,7 @@ fn in_tree_crates(&self, root: &str, target: Option<TargetSelection>) -> Vec<&Cr
         let mut list = vec![INTERNER.intern_str(root)];
         let mut visited = HashSet::new();
         while let Some(krate) = list.pop() {
-            let krate = &self.crates[&krate];
+            let krate = self.crates.get(&krate).unwrap_or_else(|| panic!("metadata missing for {krate}: {:?}", self.crates));
             ret.push(krate);
             for dep in &krate.deps {
                 if !self.crates.contains_key(dep) {

before that change it was

thread 'main' panicked at 'no entry found for key', lib.rs:1400:26

[jyn514]

the problem is that I changed Build::new to avoid running cargo metadata for the setup command, but in_tree_crates needs that info to print the list of paths:

rust/src/bootstrap/lib.rs

Lines 566 to 591 in 5e656ba

	// When running `setup`, the profile is about to change, so any requirements we have now may
	// be different on the next invocation. Don't check for them until the next time x.py is
	// run. This is ok because `setup` never runs any build commands, so it won't fail if commands are missing.
	//
	// Similarly, for `setup` we don't actually need submodules or cargo metadata.
	if !matches!(build.config.cmd, Subcommand::Setup { .. }) {
	build.verbose("running sanity check");
	sanity::check(&mut build);
	// Make sure we update these before gathering metadata so we don't get an error about missing
	// Cargo.toml files.
	let rust_submodules = [
	"src/tools/rust-installer",
	"src/tools/cargo",
	"library/backtrace",
	"library/stdarch",
	];
	for s in rust_submodules {
	build.update_submodule(Path::new(s));
	}
	// Now, update all existing submodules.
	build.update_existing_submodules();
	build.verbose("learning about cargo");
	metadata::build(&mut build);
	}

[jyn514]

I don't like the options here :/ either we start updating submodules on usage:

rust/src/bootstrap/flags.rs

Lines 354 to 356 in fca8290

	let config = Config::parse(&["setup".to_string()]);
	let build = Build::new(config);
	let paths = Builder::get_help(&build, subcommand);

or we stop showing a list of all crates on -h -v. both of those seem unfortunate.

once again it would be really nice if cargo didn't require all submodules to be checked out to have even the slightest bit of metadata about the workspace :/

[jyn514]

I think updating the submodules is the least hacky fix here, as much as I dislike it.

[albertlarsan68]

@jyn514

I got the panic you added in #106166 on the current master, b38a6d3, with the command x build --help, after having done x check. Is this related?

Here is the panic message: thread 'main' panicked at 'metadata missing for test: {}', lib.rs:1402:36

[jyn514]

ah yes, it needs to delay calling Builder::get_help unless verbose is true.

[jyn514]

Can you open a new issue?

[albertlarsan68]

Opened at #106313