From c7590dd752ab35d06a85f016e88921f10934e6aa Mon Sep 17 00:00:00 2001 From: mo8it Date: Thu, 1 Aug 2024 15:23:54 +0200 Subject: [PATCH] Improve the runner --- src/app_state.rs | 51 ++++-------------- src/cmd.rs | 126 +++++++++++++++++++++++++++++++++------------ src/dev.rs | 4 +- src/dev/check.rs | 34 ++++++------ src/dev/update.rs | 3 +- src/exercise.rs | 103 +++++++++++++----------------------- src/main.rs | 18 +------ src/run.rs | 2 +- src/watch/state.rs | 2 +- 9 files changed, 162 insertions(+), 181 deletions(-) diff --git a/src/app_state.rs b/src/app_state.rs index 537732bc..ea99746b 100644 --- a/src/app_state.rs +++ b/src/app_state.rs @@ -1,19 +1,18 @@ use anyhow::{bail, Context, Error, Result}; -use serde::Deserialize; use std::{ fs::{self, File}, io::{Read, StdoutLock, Write}, - path::{Path, PathBuf}, + path::Path, process::{Command, Stdio}, thread, }; use crate::{ clear_terminal, + cmd::CmdRunner, embedded::EMBEDDED_FILES, exercise::{Exercise, RunnableExercise}, info_file::ExerciseInfo, - DEBUG_PROFILE, }; const STATE_FILE_NAME: &str = ".rustlings-state.txt"; @@ -34,31 +33,6 @@ pub enum StateFileStatus { NotRead, } -// Parses parts of the output of `cargo metadata`. -#[derive(Deserialize)] -struct CargoMetadata { - target_directory: PathBuf, -} - -pub fn parse_target_dir() -> Result { - // Get the target directory from Cargo. - let metadata_output = Command::new("cargo") - .arg("metadata") - .arg("-q") - .arg("--format-version") - .arg("1") - .arg("--no-deps") - .stdin(Stdio::null()) - .stderr(Stdio::inherit()) - .output() - .context(CARGO_METADATA_ERR)? - .stdout; - - serde_json::de::from_slice::(&metadata_output) - .context("Failed to read the field `target_directory` from the `cargo metadata` output") - .map(|metadata| metadata.target_directory) -} - pub struct AppState { current_exercise_ind: usize, exercises: Vec, @@ -68,8 +42,7 @@ pub struct AppState { // Preallocated buffer for reading and writing the state file. file_buf: Vec, official_exercises: bool, - // Cargo's target directory. - target_dir: PathBuf, + cmd_runner: CmdRunner, } impl AppState { @@ -123,7 +96,7 @@ impl AppState { exercise_infos: Vec, final_message: String, ) -> Result<(Self, StateFileStatus)> { - let target_dir = parse_target_dir()?; + let cmd_runner = CmdRunner::build()?; let exercises = exercise_infos .into_iter() @@ -157,7 +130,7 @@ impl AppState { final_message, file_buf: Vec::with_capacity(2048), official_exercises: !Path::new("info.toml").exists(), - target_dir, + cmd_runner, }; let state_file_status = slf.update_from_file(); @@ -186,8 +159,8 @@ impl AppState { } #[inline] - pub fn target_dir(&self) -> &Path { - &self.target_dir + pub fn cmd_runner(&self) -> &CmdRunner { + &self.cmd_runner } // Write the state file. @@ -336,7 +309,7 @@ impl AppState { /// Official exercises: Dump the solution file form the binary and return its path. /// Third-party exercises: Check if a solution file exists and return its path in that case. pub fn current_solution_path(&self) -> Result> { - if DEBUG_PROFILE { + if cfg!(debug_assertions) { return Ok(None); } @@ -386,7 +359,7 @@ impl AppState { .iter_mut() .map(|exercise| { s.spawn(|| { - let success = exercise.run_exercise(None, &self.target_dir)?; + let success = exercise.run_exercise(None, &self.cmd_runner)?; exercise.done = success; Ok::<_, Error>(success) }) @@ -434,10 +407,6 @@ impl AppState { } } -const CARGO_METADATA_ERR: &str = "Failed to run the command `cargo metadata …` -Did you already install Rust? -Try running `cargo --version` to diagnose the problem."; - const RERUNNING_ALL_EXERCISES_MSG: &[u8] = b" All exercises seem to be done. Recompiling and running all exercises to make sure that all of them are actually done. @@ -490,7 +459,7 @@ mod tests { final_message: String::new(), file_buf: Vec::new(), official_exercises: true, - target_dir: PathBuf::new(), + cmd_runner: CmdRunner::build().unwrap(), }; let mut assert = |done: [bool; 3], expected: [Option; 3]| { diff --git a/src/cmd.rs b/src/cmd.rs index d158bfb9..1891a283 100644 --- a/src/cmd.rs +++ b/src/cmd.rs @@ -1,13 +1,14 @@ use anyhow::{Context, Result}; +use serde::Deserialize; use std::{ io::Read, - path::Path, + path::PathBuf, process::{Command, Stdio}, }; /// Run a command with a description for a possible error and append the merged stdout and stderr. /// The boolean in the returned `Result` is true if the command's exit status is success. -pub fn run_cmd(mut cmd: Command, description: &str, output: Option<&mut Vec>) -> Result { +fn run_cmd(mut cmd: Command, description: &str, output: Option<&mut Vec>) -> Result { let spawn = |mut cmd: Command| { // NOTE: The closure drops `cmd` which prevents a pipe deadlock. cmd.stdin(Stdio::null()) @@ -45,50 +46,107 @@ pub fn run_cmd(mut cmd: Command, description: &str, output: Option<&mut Vec> .map(|status| status.success()) } -pub struct CargoCmd<'a> { - pub subcommand: &'a str, - pub args: &'a [&'a str], - pub bin_name: &'a str, - pub description: &'a str, - /// RUSTFLAGS="-A warnings" - pub hide_warnings: bool, - /// Added as `--target-dir` if `Self::dev` is true. - pub target_dir: &'a Path, - /// The output buffer to append the merged stdout and stderr. - pub output: Option<&'a mut Vec>, - /// true while developing Rustlings. - pub dev: bool, +// Parses parts of the output of `cargo metadata`. +#[derive(Deserialize)] +struct CargoMetadata { + target_directory: PathBuf, } -impl<'a> CargoCmd<'a> { - /// Run `cargo SUBCOMMAND --bin EXERCISE_NAME … ARGS`. - pub fn run(self) -> Result { +pub struct CmdRunner { + target_dir: PathBuf, +} + +impl CmdRunner { + pub fn build() -> Result { + // Get the target directory from Cargo. + let metadata_output = Command::new("cargo") + .arg("metadata") + .arg("-q") + .arg("--format-version") + .arg("1") + .arg("--no-deps") + .stdin(Stdio::null()) + .stderr(Stdio::inherit()) + .output() + .context(CARGO_METADATA_ERR)? + .stdout; + + let target_dir = serde_json::de::from_slice::(&metadata_output) + .context("Failed to read the field `target_directory` from the `cargo metadata` output") + .map(|metadata| metadata.target_directory)?; + + Ok(Self { target_dir }) + } + + pub fn cargo<'out>( + &self, + subcommand: &str, + bin_name: &str, + output: Option<&'out mut Vec>, + ) -> CargoSubcommand<'out> { let mut cmd = Command::new("cargo"); - cmd.arg(self.subcommand); + cmd.arg(subcommand).arg("-q").arg("--bin").arg(bin_name); // A hack to make `cargo run` work when developing Rustlings. - if self.dev { - cmd.arg("--manifest-path") - .arg("dev/Cargo.toml") - .arg("--target-dir") - .arg(self.target_dir); + #[cfg(debug_assertions)] + cmd.arg("--manifest-path") + .arg("dev/Cargo.toml") + .arg("--target-dir") + .arg(&self.target_dir); + + if output.is_some() { + cmd.arg("--color").arg("always"); } - cmd.arg("--color") - .arg("always") - .arg("-q") - .arg("--bin") - .arg(self.bin_name) - .args(self.args); + CargoSubcommand { cmd, output } + } - if self.hide_warnings { - cmd.env("RUSTFLAGS", "-A warnings"); - } + /// The boolean in the returned `Result` is true if the command's exit status is success. + pub fn run_debug_bin(&self, bin_name: &str, output: Option<&mut Vec>) -> Result { + // 7 = "/debug/".len() + let mut bin_path = + PathBuf::with_capacity(self.target_dir.as_os_str().len() + 7 + bin_name.len()); + bin_path.push(&self.target_dir); + bin_path.push("debug"); + bin_path.push(bin_name); - run_cmd(cmd, self.description, self.output) + run_cmd(Command::new(&bin_path), &bin_path.to_string_lossy(), output) } } +pub struct CargoSubcommand<'out> { + cmd: Command, + output: Option<&'out mut Vec>, +} + +impl<'out> CargoSubcommand<'out> { + #[inline] + pub fn args<'arg, I>(&mut self, args: I) -> &mut Self + where + I: IntoIterator, + { + self.cmd.args(args); + self + } + + /// RUSTFLAGS="-A warnings" + #[inline] + pub fn hide_warnings(&mut self) -> &mut Self { + self.cmd.env("RUSTFLAGS", "-A warnings"); + self + } + + /// The boolean in the returned `Result` is true if the command's exit status is success. + #[inline] + pub fn run(self, description: &str) -> Result { + run_cmd(self.cmd, description, self.output) + } +} + +const CARGO_METADATA_ERR: &str = "Failed to run the command `cargo metadata …` +Did you already install Rust? +Try running `cargo --version` to diagnose the problem."; + #[cfg(test)] mod tests { use super::*; diff --git a/src/dev.rs b/src/dev.rs index 5f7e64c8..8af40d69 100644 --- a/src/dev.rs +++ b/src/dev.rs @@ -2,8 +2,6 @@ use anyhow::{bail, Context, Result}; use clap::Subcommand; use std::path::PathBuf; -use crate::DEBUG_PROFILE; - mod check; mod new; mod update; @@ -32,7 +30,7 @@ impl DevCommands { pub fn run(self) -> Result<()> { match self { Self::New { path, no_git } => { - if DEBUG_PROFILE { + if cfg!(debug_assertions) { bail!("Disabled in the debug build"); } diff --git a/src/dev/check.rs b/src/dev/check.rs index 1087138c..db5b21fc 100644 --- a/src/dev/check.rs +++ b/src/dev/check.rs @@ -12,11 +12,11 @@ use std::{ }; use crate::{ - app_state::parse_target_dir, cargo_toml::{append_bins, bins_start_end_ind, BINS_BUFFER_CAPACITY}, + cmd::CmdRunner, exercise::{RunnableExercise, OUTPUT_CAPACITY}, info_file::{ExerciseInfo, InfoFile}, - CURRENT_FORMAT_VERSION, DEBUG_PROFILE, + CURRENT_FORMAT_VERSION, }; // Find a char that isn't allowed in the exercise's `name` or `dir`. @@ -37,8 +37,8 @@ fn check_cargo_toml( append_bins(&mut new_bins, exercise_infos, exercise_path_prefix); if old_bins != new_bins { - if DEBUG_PROFILE { - bail!("The file `dev/Cargo.toml` is outdated. Please run `cargo run -- dev update` to update it"); + if cfg!(debug_assertions) { + bail!("The file `dev/Cargo.toml` is outdated. Please run `cargo run -- dev update` to update it. Then run `cargo run -- dev check` again"); } bail!("The file `Cargo.toml` is outdated. Please run `rustlings dev update` to update it. Then run `rustlings dev check` again"); @@ -162,7 +162,7 @@ fn check_unexpected_files( Ok(()) } -fn check_exercises_unsolved(info_file: &InfoFile, target_dir: &Path) -> Result<()> { +fn check_exercises_unsolved(info_file: &InfoFile, cmd_runner: &CmdRunner) -> Result<()> { let error_occurred = AtomicBool::new(false); println!( @@ -184,7 +184,7 @@ fn check_exercises_unsolved(info_file: &InfoFile, target_dir: &Path) -> Result<( error_occurred.store(true, atomic::Ordering::Relaxed); }; - match exercise_info.run_exercise(None, target_dir) { + match exercise_info.run_exercise(None, cmd_runner) { Ok(true) => error(b"Already solved!"), Ok(false) => (), Err(e) => error(e.to_string().as_bytes()), @@ -200,7 +200,7 @@ fn check_exercises_unsolved(info_file: &InfoFile, target_dir: &Path) -> Result<( Ok(()) } -fn check_exercises(info_file: &InfoFile, target_dir: &Path) -> Result<()> { +fn check_exercises(info_file: &InfoFile, cmd_runner: &CmdRunner) -> Result<()> { match info_file.format_version.cmp(&CURRENT_FORMAT_VERSION) { Ordering::Less => bail!("`format_version` < {CURRENT_FORMAT_VERSION} (supported version)\nPlease migrate to the latest format version"), Ordering::Greater => bail!("`format_version` > {CURRENT_FORMAT_VERSION} (supported version)\nTry updating the Rustlings program"), @@ -210,10 +210,14 @@ fn check_exercises(info_file: &InfoFile, target_dir: &Path) -> Result<()> { let info_file_paths = check_info_file_exercises(info_file)?; check_unexpected_files("exercises", &info_file_paths)?; - check_exercises_unsolved(info_file, target_dir) + check_exercises_unsolved(info_file, cmd_runner) } -fn check_solutions(require_solutions: bool, info_file: &InfoFile, target_dir: &Path) -> Result<()> { +fn check_solutions( + require_solutions: bool, + info_file: &InfoFile, + cmd_runner: &CmdRunner, +) -> Result<()> { let paths = Mutex::new(hashbrown::HashSet::with_capacity(info_file.exercises.len())); let error_occurred = AtomicBool::new(false); @@ -243,7 +247,7 @@ fn check_solutions(require_solutions: bool, info_file: &InfoFile, target_dir: &P } let mut output = Vec::with_capacity(OUTPUT_CAPACITY); - match exercise_info.run_solution(Some(&mut output), target_dir) { + match exercise_info.run_solution(Some(&mut output), cmd_runner) { Ok(true) => { paths.lock().unwrap().insert(PathBuf::from(path)); } @@ -266,8 +270,8 @@ fn check_solutions(require_solutions: bool, info_file: &InfoFile, target_dir: &P pub fn check(require_solutions: bool) -> Result<()> { let info_file = InfoFile::parse()?; - // A hack to make `cargo run -- dev check` work when developing Rustlings. - if DEBUG_PROFILE { + if cfg!(debug_assertions) { + // A hack to make `cargo run -- dev check` work when developing Rustlings. check_cargo_toml( &info_file.exercises, include_str!("../../dev-Cargo.toml"), @@ -279,9 +283,9 @@ pub fn check(require_solutions: bool) -> Result<()> { check_cargo_toml(&info_file.exercises, ¤t_cargo_toml, b"")?; } - let target_dir = parse_target_dir()?; - check_exercises(&info_file, &target_dir)?; - check_solutions(require_solutions, &info_file, &target_dir)?; + let cmd_runner = CmdRunner::build()?; + check_exercises(&info_file, &cmd_runner)?; + check_solutions(require_solutions, &info_file, &cmd_runner)?; println!("\nEverything looks fine!"); diff --git a/src/dev/update.rs b/src/dev/update.rs index 66efe3d0..680d302f 100644 --- a/src/dev/update.rs +++ b/src/dev/update.rs @@ -4,7 +4,6 @@ use std::fs; use crate::{ cargo_toml::updated_cargo_toml, info_file::{ExerciseInfo, InfoFile}, - DEBUG_PROFILE, }; // Update the `Cargo.toml` file. @@ -27,7 +26,7 @@ pub fn update() -> Result<()> { let info_file = InfoFile::parse()?; // A hack to make `cargo run -- dev update` work when developing Rustlings. - if DEBUG_PROFILE { + if cfg!(debug_assertions) { update_cargo_toml( &info_file.exercises, include_str!("../../dev-Cargo.toml"), diff --git a/src/exercise.rs b/src/exercise.rs index 8a040613..48b98896 100644 --- a/src/exercise.rs +++ b/src/exercise.rs @@ -3,38 +3,25 @@ use ratatui::crossterm::style::{style, StyledContent, Stylize}; use std::{ fmt::{self, Display, Formatter}, io::Write, - path::{Path, PathBuf}, - process::Command, }; -use crate::{ - cmd::{run_cmd, CargoCmd}, - in_official_repo, - terminal_link::TerminalFileLink, - DEBUG_PROFILE, -}; +use crate::{cmd::CmdRunner, terminal_link::TerminalFileLink}; /// The initial capacity of the output buffer. pub const OUTPUT_CAPACITY: usize = 1 << 14; // Run an exercise binary and append its output to the `output` buffer. // Compilation must be done before calling this method. -fn run_bin(bin_name: &str, mut output: Option<&mut Vec>, target_dir: &Path) -> Result { +fn run_bin( + bin_name: &str, + mut output: Option<&mut Vec>, + cmd_runner: &CmdRunner, +) -> Result { if let Some(output) = output.as_deref_mut() { writeln!(output, "{}", "Output".underlined())?; } - // 7 = "/debug/".len() - let mut bin_path = PathBuf::with_capacity(target_dir.as_os_str().len() + 7 + bin_name.len()); - bin_path.push(target_dir); - bin_path.push("debug"); - bin_path.push(bin_name); - - let success = run_cmd( - Command::new(&bin_path), - &bin_path.to_string_lossy(), - output.as_deref_mut(), - )?; + let success = cmd_runner.run_debug_bin(bin_name, output.as_deref_mut())?; if let Some(output) = output { if !success { @@ -89,26 +76,20 @@ pub trait RunnableExercise { &self, bin_name: &str, mut output: Option<&mut Vec>, - target_dir: &Path, + cmd_runner: &CmdRunner, ) -> Result { - if let Some(output) = output.as_deref_mut() { + let output_is_none = if let Some(output) = output.as_deref_mut() { output.clear(); - } + false + } else { + true + }; - // Developing the official Rustlings. - let dev = DEBUG_PROFILE && in_official_repo(); - - let build_success = CargoCmd { - subcommand: "build", - args: &[], - bin_name, - description: "cargo build …", - hide_warnings: output.is_none(), - target_dir, - output: output.as_deref_mut(), - dev, + let mut build_cmd = cmd_runner.cargo("build", bin_name, output.as_deref_mut()); + if output_is_none { + build_cmd.hide_warnings(); } - .run()?; + let build_success = build_cmd.run("cargo build …")?; if !build_success { return Ok(false); } @@ -118,45 +99,33 @@ pub trait RunnableExercise { output.clear(); } + let mut clippy_cmd = cmd_runner.cargo("clippy", bin_name, output.as_deref_mut()); + // `--profile test` is required to also check code with `[cfg(test)]`. - let clippy_args: &[&str] = if self.strict_clippy() { - &["--profile", "test", "--", "-D", "warnings"] + if self.strict_clippy() { + clippy_cmd.args(["--profile", "test", "--", "-D", "warnings"]); } else { - &["--profile", "test"] - }; - let clippy_success = CargoCmd { - subcommand: "clippy", - args: clippy_args, - bin_name, - description: "cargo clippy …", - hide_warnings: false, - target_dir, - output: output.as_deref_mut(), - dev, + clippy_cmd.args(["--profile", "test"]); } - .run()?; + + let clippy_success = clippy_cmd.run("cargo clippy …")?; if !clippy_success { return Ok(false); } if !self.test() { - return run_bin(bin_name, output.as_deref_mut(), target_dir); + return run_bin(bin_name, output.as_deref_mut(), cmd_runner); } - let test_success = CargoCmd { - subcommand: "test", - args: &["--", "--color", "always", "--show-output"], - bin_name, - description: "cargo test …", - // Hide warnings because they are shown by Clippy. - hide_warnings: true, - target_dir, - output: output.as_deref_mut(), - dev, + let mut test_cmd = cmd_runner.cargo("test", bin_name, output.as_deref_mut()); + if !output_is_none { + test_cmd.args(["--", "--color", "always", "--show-output"]); } - .run()?; + // Hide warnings because they are shown by Clippy. + test_cmd.hide_warnings(); + let test_success = test_cmd.run("cargo test …")?; - let run_success = run_bin(bin_name, output.as_deref_mut(), target_dir)?; + let run_success = run_bin(bin_name, output, cmd_runner)?; Ok(test_success && run_success) } @@ -164,19 +133,19 @@ pub trait RunnableExercise { /// Compile, check and run the exercise. /// The output is written to the `output` buffer after clearing it. #[inline] - fn run_exercise(&self, output: Option<&mut Vec>, target_dir: &Path) -> Result { - self.run(self.name(), output, target_dir) + fn run_exercise(&self, output: Option<&mut Vec>, cmd_runner: &CmdRunner) -> Result { + self.run(self.name(), output, cmd_runner) } /// Compile, check and run the exercise's solution. /// The output is written to the `output` buffer after clearing it. - fn run_solution(&self, output: Option<&mut Vec>, target_dir: &Path) -> Result { + fn run_solution(&self, output: Option<&mut Vec>, cmd_runner: &CmdRunner) -> Result { let name = self.name(); let mut bin_name = String::with_capacity(name.len() + 4); bin_name.push_str(name); bin_name.push_str("_sol"); - self.run(&bin_name, output, target_dir) + self.run(&bin_name, output, cmd_runner) } } diff --git a/src/main.rs b/src/main.rs index 658d551d..1f0afdec 100644 --- a/src/main.rs +++ b/src/main.rs @@ -24,22 +24,6 @@ mod terminal_link; mod watch; const CURRENT_FORMAT_VERSION: u8 = 1; -const DEBUG_PROFILE: bool = { - #[allow(unused_assignments, unused_mut)] - let mut debug_profile = false; - - #[cfg(debug_assertions)] - { - debug_profile = true; - } - - debug_profile -}; - -// The current directory is the official Rustligns repository. -fn in_official_repo() -> bool { - Path::new("dev/rustlings-repo.txt").exists() -} fn clear_terminal(stdout: &mut StdoutLock) -> io::Result<()> { stdout.write_all(b"\x1b[H\x1b[2J\x1b[3J") @@ -89,7 +73,7 @@ enum Subcommands { fn main() -> Result<()> { let args = Args::parse(); - if !DEBUG_PROFILE && in_official_repo() { + if cfg!(not(debug_assertions)) && Path::new("dev/rustlings-repo.txt").exists() { bail!("{OLD_METHOD_ERR}"); } diff --git a/src/run.rs b/src/run.rs index 606f0a43..964e13b8 100644 --- a/src/run.rs +++ b/src/run.rs @@ -11,7 +11,7 @@ use crate::{ pub fn run(app_state: &mut AppState) -> Result<()> { let exercise = app_state.current_exercise(); let mut output = Vec::with_capacity(OUTPUT_CAPACITY); - let success = exercise.run_exercise(Some(&mut output), app_state.target_dir())?; + let success = exercise.run_exercise(Some(&mut output), app_state.cmd_runner())?; let mut stdout = io::stdout().lock(); stdout.write_all(&output)?; diff --git a/src/watch/state.rs b/src/watch/state.rs index 8f01db7d..46f48d9f 100644 --- a/src/watch/state.rs +++ b/src/watch/state.rs @@ -54,7 +54,7 @@ impl<'a> WatchState<'a> { let success = self .app_state .current_exercise() - .run_exercise(Some(&mut self.output), self.app_state.target_dir())?; + .run_exercise(Some(&mut self.output), self.app_state.cmd_runner())?; if success { self.done_status = if let Some(solution_path) = self.app_state.current_solution_path()? {