y test: improve verbose output formatting (#8)

xizheyin · web-flow · commit 2f477f05f760 · 2025-02-08T03:17:08.000-06:00
* ./y Test: improve verbose output formatting
* Build: improve verbose output in manifest
* refactor: unify verbose flag handling

- Refactor Log trait to handle command status and details consistently
- Make verbose flag shared between main command and subcommands
- Improve log output format for test and build commands

This change ensures that `y -v test` produce verbose outputs.

Signed-off-by: xizheyin &lt;xizheyin@smail.nju.edu.cn&gt;
diff --git a/bootstrap/src/clean.rs b/bootstrap/src/clean.rs
@@ -5,11 +5,23 @@ use crate::Run;
 
 /// Clean the build directory
 #[derive(Args, Debug)]
-pub struct CleanCommand {}
+pub struct CleanCommand {
+    #[arg(short, long)]
+    pub verbose: bool,
+}
 
 impl Run for CleanCommand {
+    const STEP_DISPLAY_NAME: &'static str = "clean";
+
     fn run(&self, manifest: &Manifest) {
+        self.log_action_start("cleaning", "build directory");
         let _ = std::fs::remove_dir_all("crates/target");
+        self.log_action_context("rm", "crates/target");
         let _ = std::fs::remove_dir_all(&manifest.out_dir);
+        self.log_action_context("rm", &manifest.out_dir.display());
+    }
+
+    fn verbose(&self) -> bool {
+        self.verbose
     }
 }
diff --git a/bootstrap/src/fmt.rs b/bootstrap/src/fmt.rs
@@ -10,9 +10,14 @@ use crate::Run;
 pub struct FmtCommand {
     #[arg(short, long)]
     pub check: bool,
+
+    #[arg(short, long)]
+    pub verbose: bool,
 }
 
 impl Run for FmtCommand {
+    const STEP_DISPLAY_NAME: &'static str = "FMT";
+
     fn run(&self, _manifest: &crate::manifest::Manifest) {
         self.perform(
             Command::new("cargo").arg("fmt").args(["--manifest-path", "bootstrap/Cargo.toml"]),
@@ -30,14 +35,18 @@ impl Run for FmtCommand {
             self.perform(Command::new("rustfmt").args(["--edition", "2021"]).arg(file.unwrap()));
         }
     }
+
+    fn verbose(&self) -> bool {
+        self.verbose
+    }
 }
 
 impl FmtCommand {
     pub fn perform(&self, command: &mut Command) {
         if self.check {
             command.arg("--check");
         }
-        log::debug!("running {:?}", command);
-        assert!(command.status().unwrap().success(), "failed to run {:?}", command);
+
+        self.command_status("format code", command);
     }
 }
diff --git a/bootstrap/src/main.rs b/bootstrap/src/main.rs
@@ -1,4 +1,8 @@
+use std::fmt::Display;
+use std::process::{self, ExitStatus};
+
 use clap::{Parser, Subcommand};
+use color_print::cprintln;
 
 use crate::manifest::Manifest;
 
@@ -37,7 +41,65 @@ pub enum Command {
 }
 
 trait Run {
+    // The name like "BUILD" or "TEST" for logs.
+    const STEP_DISPLAY_NAME: &'static str;
     fn run(&self, manifest: &Manifest);
+
+    /// True if verbose output should be enabled.
+    fn verbose(&self) -> bool;
+
+    /// Record that the step has started a new action.
+    fn log_action_start(&self, action: &str, item: impl Display) {
+        let name = Self::STEP_DISPLAY_NAME;
+        cprintln!("<b>[{name}]</b> {action} <cyan>{item}</cyan>");
+    }
+
+    /// Record context associated with the current action. Only use if there has been a preceding
+    /// call to `log_action_start`.
+    fn log_action_context(&self, key: impl Display, value: impl Display) {
+        if self.verbose() {
+            cprintln!("       {key}: {value}");
+        }
+    }
+
+    /// Run a command and ensure it succeeds, capturing output.
+    fn command_output(&self, action: &str, command: &mut process::Command) -> process::Output {
+        if self.verbose() {
+            cprintln!("       {action}: {command:?}");
+        }
+
+        match command.output() {
+            // Command ran and completed successfully
+            Ok(output) if output.status.success() => {
+                if self.verbose() {
+                    cprintln!("       <g>success</g>");
+                }
+                output
+            }
+            // Command ran but did not complete
+            Ok(output) => panic!("command failed: {output:?}"),
+            Err(e) => panic!("command failed: {e:?}"),
+        }
+    }
+
+    /// Run a command and ensure it succeeds.
+    fn command_status(&self, action: &str, command: &mut process::Command) -> ExitStatus {
+        if self.verbose() {
+            cprintln!("       {}: {}", action, format!("{:?}", command).replace('"', ""));
+        }
+        match command.status() {
+            // Command ran and completed successfully
+            Ok(status) if status.success() => {
+                if self.verbose() {
+                    cprintln!("       <g>success</g>");
+                }
+                status
+            }
+            // Command ran but did not complete
+            Ok(status) => panic!("command failed: {status:?}"),
+            Err(e) => panic!("command failed: {e:?}"),
+        }
+    }
 }
 
 fn main() {
@@ -49,10 +111,23 @@ fn main() {
         release: cli.release,
         out_dir: cli.out_dir.unwrap_or("build".to_string()).into(),
     };
+
     match cli.command {
-        Command::Test(test) => test.run(&manifest),
-        Command::Clean(clean) => clean.run(&manifest),
-        Command::Rustc(rustc) => rustc.run(&manifest),
-        Command::Fmt(fmt) => fmt.run(&manifest),
+        Command::Test(mut test) => {
+            test.verbose |= cli.verbose;
+            test.run(&manifest)
+        }
+        Command::Clean(mut clean) => {
+            clean.verbose |= cli.verbose;
+            clean.run(&manifest)
+        }
+        Command::Rustc(mut rustc) => {
+            rustc.verbose |= cli.verbose;
+            rustc.run(&manifest)
+        }
+        Command::Fmt(mut fmt) => {
+            fmt.verbose |= cli.verbose;
+            fmt.run(&manifest)
+        }
     }
 }
diff --git a/bootstrap/src/manifest.rs b/bootstrap/src/manifest.rs
@@ -1,8 +1,12 @@
-use anstream::eprintln as println;
-use color_print::cprintln;
 use std::path::{Path, PathBuf};
 use std::process::Command;
 
+use anstream::eprintln as println;
+use color_print::cprintln;
+
+use crate::Run;
+
+#[derive(Debug)]
 pub struct Manifest {
     pub verbose: bool,
     pub release: bool,
@@ -12,36 +16,8 @@ pub struct Manifest {
 impl Manifest {
     /// Builds the rustc codegen c library
     pub fn prepare(&self) {
-        cprintln!("<b>[BUILD]</b> codegen backend");
-        let mut command = Command::new("cargo");
-        command.arg("build").args(["--manifest-path", "crates/Cargo.toml"]);
-        if self.verbose {
-            command.args(["-F", "debug"]);
-        }
-        if self.release {
-            command.arg("--release");
-        }
-        log::debug!("running {:?}", command);
-        command.status().unwrap();
-
-        cprintln!("<b>[BUILD]</b> librust_runtime");
-        std::fs::create_dir_all(&self.out_dir).unwrap();
-        let cc = std::env::var("CC").unwrap_or("clang".to_string());
-        let mut command = Command::new(&cc);
-        command
-            .arg("rust_runtime/rust_runtime.c")
-            .arg("-o")
-            .arg(self.out_dir.join("rust_runtime.o"))
-            .arg("-c");
-        log::debug!("running {:?}", command);
-        command.status().unwrap();
-        let mut command = Command::new("ar");
-        command
-            .arg("rcs")
-            .arg(self.out_dir.join("librust_runtime.a"))
-            .arg(self.out_dir.join("rust_runtime.o"));
-        log::debug!("running {:?}", command);
-        command.status().unwrap();
+        let prepare = PrepareAction { verbose: self.verbose };
+        prepare.run(&self);
     }
 
     /// The path to the rustc codegen c library
@@ -72,3 +48,60 @@ impl Manifest {
         command
     }
 }
+
+struct PrepareAction {
+    verbose: bool,
+}
+
+impl Run for PrepareAction {
+    const STEP_DISPLAY_NAME: &'static str = "prepare";
+
+    fn run(&self, manifest: &Manifest) {
+        // action: Build codegen backend
+        self.log_action_start("building", "codegen backend");
+        self.log_action_context("target", manifest.codegen_backend().display());
+
+        let mut command = Command::new("cargo");
+        command.arg("build").args(["--manifest-path", "crates/Cargo.toml"]);
+        if manifest.verbose {
+            command.args(["-v"]);
+        }
+        if manifest.release {
+            command.arg("--release");
+        }
+        self.command_status("build", &mut command);
+
+        // action: Build runtime library
+        self.log_action_start("building", "librust_runtime");
+        self.log_action_context("output dir", &manifest.out_dir.to_path_buf().display());
+
+        // cmd: Create output directory
+        if let Err(e) = std::fs::create_dir_all(&manifest.out_dir) {
+            cprintln!("       <r>failed</r> to create output directory: {}", e);
+            std::process::exit(1);
+        }
+
+        let cc = std::env::var("CC").unwrap_or("clang".to_string());
+
+        // cmd: Compile runtime.c
+        let mut command = Command::new(&cc);
+        command
+            .arg("rust_runtime/rust_runtime.c")
+            .arg("-o")
+            .arg(manifest.out_dir.join("rust_runtime.o"))
+            .arg("-c");
+        self.command_status("build", &mut command);
+
+        // cmd: Create static library
+        let mut command = Command::new("ar");
+        command
+            .arg("rcs")
+            .arg(manifest.out_dir.join("librust_runtime.a"))
+            .arg(manifest.out_dir.join("rust_runtime.o"));
+        self.command_status("archive", &mut command);
+    }
+
+    fn verbose(&self) -> bool {
+        self.verbose
+    }
+}
diff --git a/bootstrap/src/rustc.rs b/bootstrap/src/rustc.rs
@@ -12,9 +12,14 @@ pub struct RustcCommand {
 
     #[arg(last = true)]
     slop: Vec<String>,
+
+    #[arg(short, long)]
+    pub verbose: bool,
 }
 
 impl Run for RustcCommand {
+    const STEP_DISPLAY_NAME: &'static str = "RUSTC";
+
     fn run(&self, manifest: &Manifest) {
         manifest.prepare();
 
@@ -25,7 +30,14 @@ impl Run for RustcCommand {
             .arg("--out-dir")
             .arg(&manifest.out_dir)
             .args(&self.slop);
-        log::debug!("running {:?}", command);
-        command.status().unwrap();
+        if self.verbose {
+            command.env("RUST_BACKTRACE", "full");
+        }
+
+        self.command_status("rustc", &mut command);
+    }
+
+    fn verbose(&self) -> bool {
+        self.verbose
     }
 }
diff --git a/bootstrap/src/test.rs b/bootstrap/src/test.rs

Original file line number	Diff line number	Diff line change
`@@ -10,9 +10,14 @@ use crate::Run;`
`10`	`10`	`pub struct FmtCommand {`
`11`	`11`	`#[arg(short, long)]`
`12`	`12`	`pub check: bool,`
	`13`	`+`
	`14`	`+ #[arg(short, long)]`
	`15`	`+ pub verbose: bool,`
`13`	`16`	`}`
`14`	`17`
`15`	`18`	`impl Run for FmtCommand {`
	`19`	`+ const STEP_DISPLAY_NAME: &'static str = "FMT";`
	`20`	`+`
`16`	`21`	`fn run(&self, _manifest: &crate::manifest::Manifest) {`
`17`	`22`	`self.perform(`
`18`	`23`	`Command::new("cargo").arg("fmt").args(["--manifest-path", "bootstrap/Cargo.toml"]),`
`@@ -30,14 +35,18 @@ impl Run for FmtCommand {`
`30`	`35`	`self.perform(Command::new("rustfmt").args(["--edition", "2021"]).arg(file.unwrap()));`
`31`	`36`	`}`
`32`	`37`	`}`
	`38`	`+`
	`39`	`+ fn verbose(&self) -> bool {`
	`40`	`+ self.verbose`
	`41`	`+ }`
`33`	`42`	`}`
`34`	`43`
`35`	`44`	`impl FmtCommand {`
`36`	`45`	`pub fn perform(&self, command: &mut Command) {`
`37`	`46`	`if self.check {`
`38`	`47`	`command.arg("--check");`
`39`	`48`	`}`
`40`		`- log::debug!("running {:?}", command);`
`41`		`- assert!(command.status().unwrap().success(), "failed to run {:?}", command);`
	`49`	`+`
	`50`	`+ self.command_status("format code", command);`
`42`	`51`	`}`
`43`	`52`	`}`