Skip to content

Commit 1dec35a

Browse files
committed
Auto merge of #96003 - aswild:pr/bootstrap-subcommands-cleanup, r=jyn514
bootstrap: consolidate subcommand parsing and matching There's several places where the x.py command names are matched as strings, leading to some inconsistencies and opportunities for cleanup. * Add Format, Clean, and Setup variants to builder::Kind. * Use Kind to parse the x.py subcommand name (including aliases) * Match on the subcommand Kind rather than strings when handling options and help text. * Several subcommands don't display any paths when run with `-h -v` even though the help text indicates that they should. Fix this and refactor so that manually keeping matches in sync isn't necessary. Fixes #95937
2 parents 3d3dafb + 870cb8e commit 1dec35a

File tree

2 files changed

+85
-103
lines changed

2 files changed

+85
-103
lines changed

src/bootstrap/builder.rs

+35-27
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::cell::{Cell, RefCell};
33
use std::collections::BTreeSet;
44
use std::env;
55
use std::ffi::OsStr;
6-
use std::fmt::Debug;
6+
use std::fmt::{Debug, Write};
77
use std::fs;
88
use std::hash::Hash;
99
use std::ops::Deref;
@@ -125,7 +125,8 @@ impl TaskPath {
125125
if found_kind.is_empty() {
126126
panic!("empty kind in task path {}", path.display());
127127
}
128-
kind = Some(Kind::parse(found_kind));
128+
kind = Kind::parse(found_kind);
129+
assert!(kind.is_some());
129130
path = Path::new(found_prefix).join(components.as_path());
130131
}
131132
}
@@ -431,43 +432,53 @@ pub enum Kind {
431432
Check,
432433
Clippy,
433434
Fix,
435+
Format,
434436
Test,
435437
Bench,
436-
Dist,
437438
Doc,
439+
Clean,
440+
Dist,
438441
Install,
439442
Run,
443+
Setup,
440444
}
441445

442446
impl Kind {
443-
fn parse(string: &str) -> Kind {
444-
match string {
445-
"build" => Kind::Build,
446-
"check" => Kind::Check,
447+
pub fn parse(string: &str) -> Option<Kind> {
448+
// these strings, including the one-letter aliases, must match the x.py help text
449+
Some(match string {
450+
"build" | "b" => Kind::Build,
451+
"check" | "c" => Kind::Check,
447452
"clippy" => Kind::Clippy,
448453
"fix" => Kind::Fix,
449-
"test" => Kind::Test,
454+
"fmt" => Kind::Format,
455+
"test" | "t" => Kind::Test,
450456
"bench" => Kind::Bench,
457+
"doc" | "d" => Kind::Doc,
458+
"clean" => Kind::Clean,
451459
"dist" => Kind::Dist,
452-
"doc" => Kind::Doc,
453460
"install" => Kind::Install,
454-
"run" => Kind::Run,
455-
other => panic!("unknown kind: {}", other),
456-
}
461+
"run" | "r" => Kind::Run,
462+
"setup" => Kind::Setup,
463+
_ => return None,
464+
})
457465
}
458466

459-
fn as_str(&self) -> &'static str {
467+
pub fn as_str(&self) -> &'static str {
460468
match self {
461469
Kind::Build => "build",
462470
Kind::Check => "check",
463471
Kind::Clippy => "clippy",
464472
Kind::Fix => "fix",
473+
Kind::Format => "fmt",
465474
Kind::Test => "test",
466475
Kind::Bench => "bench",
467-
Kind::Dist => "dist",
468476
Kind::Doc => "doc",
477+
Kind::Clean => "clean",
478+
Kind::Dist => "dist",
469479
Kind::Install => "install",
470480
Kind::Run => "run",
481+
Kind::Setup => "setup",
471482
}
472483
}
473484
}
@@ -511,7 +522,7 @@ impl<'a> Builder<'a> {
511522
native::Lld,
512523
native::CrtBeginEnd
513524
),
514-
Kind::Check | Kind::Clippy { .. } | Kind::Fix => describe!(
525+
Kind::Check => describe!(
515526
check::Std,
516527
check::Rustc,
517528
check::Rustdoc,
@@ -641,32 +652,29 @@ impl<'a> Builder<'a> {
641652
install::Rustc
642653
),
643654
Kind::Run => describe!(run::ExpandYamlAnchors, run::BuildManifest, run::BumpStage0),
655+
// These commands either don't use paths, or they're special-cased in Build::build()
656+
Kind::Clean | Kind::Clippy | Kind::Fix | Kind::Format | Kind::Setup => vec![],
644657
}
645658
}
646659

647-
pub fn get_help(build: &Build, subcommand: &str) -> Option<String> {
648-
let kind = match subcommand {
649-
"build" | "b" => Kind::Build,
650-
"doc" | "d" => Kind::Doc,
651-
"test" | "t" => Kind::Test,
652-
"bench" => Kind::Bench,
653-
"dist" => Kind::Dist,
654-
"install" => Kind::Install,
655-
_ => return None,
656-
};
660+
pub fn get_help(build: &Build, kind: Kind) -> Option<String> {
661+
let step_descriptions = Builder::get_step_descriptions(kind);
662+
if step_descriptions.is_empty() {
663+
return None;
664+
}
657665

658666
let builder = Self::new_internal(build, kind, vec![]);
659667
let builder = &builder;
660668
// The "build" kind here is just a placeholder, it will be replaced with something else in
661669
// the following statement.
662670
let mut should_run = ShouldRun::new(builder, Kind::Build);
663-
for desc in Builder::get_step_descriptions(builder.kind) {
671+
for desc in step_descriptions {
664672
should_run.kind = desc.kind;
665673
should_run = (desc.should_run)(should_run);
666674
}
667675
let mut help = String::from("Available paths:\n");
668676
let mut add_path = |path: &Path| {
669-
help.push_str(&format!(" ./x.py {} {}\n", subcommand, path.display()));
677+
t!(write!(help, " ./x.py {} {}\n", kind.as_str(), path.display()));
670678
};
671679
for pathset in should_run.paths {
672680
match pathset {

0 commit comments

Comments
 (0)