From 74fab994e2133cc40718abe923645922785c2a57 Mon Sep 17 00:00:00 2001 From: mo8it Date: Sun, 28 Jul 2024 20:30:23 +0200 Subject: [PATCH] Make the output optional --- Cargo.toml | 4 +++ src/app_state.rs | 5 ++-- src/cmd.rs | 54 +++++++++++++++++++++++--------------- src/dev/check.rs | 5 ++-- src/exercise.rs | 65 +++++++++++++++++++++++++++++----------------- src/run.rs | 2 +- src/watch/state.rs | 2 +- 7 files changed, 84 insertions(+), 53 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 4aad110f..0577a6e8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -63,3 +63,7 @@ panic = "abort" [package.metadata.release] pre-release-hook = ["./release-hook.sh"] + +# TODO: Remove after the following fix is released: https://github.com/rust-lang/rust-clippy/pull/13102 +[lints.clippy] +needless_option_as_deref = "allow" diff --git a/src/app_state.rs b/src/app_state.rs index 40c91308..537732bc 100644 --- a/src/app_state.rs +++ b/src/app_state.rs @@ -11,7 +11,7 @@ use std::{ use crate::{ clear_terminal, embedded::EMBEDDED_FILES, - exercise::{Exercise, RunnableExercise, OUTPUT_CAPACITY}, + exercise::{Exercise, RunnableExercise}, info_file::ExerciseInfo, DEBUG_PROFILE, }; @@ -386,8 +386,7 @@ impl AppState { .iter_mut() .map(|exercise| { s.spawn(|| { - let mut output = Vec::with_capacity(OUTPUT_CAPACITY); - let success = exercise.run_exercise(&mut output, &self.target_dir)?; + let success = exercise.run_exercise(None, &self.target_dir)?; exercise.done = success; Ok::<_, Error>(success) }) diff --git a/src/cmd.rs b/src/cmd.rs index 6092f531..efeb5988 100644 --- a/src/cmd.rs +++ b/src/cmd.rs @@ -1,30 +1,42 @@ use anyhow::{Context, Result}; -use std::{io::Read, path::Path, process::Command}; +use std::{ + io::Read, + path::Path, + 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: &mut Vec) -> Result { - let (mut reader, writer) = os_pipe::pipe() - .with_context(|| format!("Failed to create a pipe to run the command `{description}``"))?; +pub 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.spawn() + .with_context(|| format!("Failed to run the command `{description}`")) + }; - let writer_clone = writer.try_clone().with_context(|| { - format!("Failed to clone the pipe writer for the command `{description}`") - })?; + let mut handle = if let Some(output) = output { + let (mut reader, writer) = os_pipe::pipe().with_context(|| { + format!("Failed to create a pipe to run the command `{description}``") + })?; - let mut handle = cmd - .stdout(writer_clone) - .stderr(writer) - .spawn() - .with_context(|| format!("Failed to run the command `{description}`"))?; + let writer_clone = writer.try_clone().with_context(|| { + format!("Failed to clone the pipe writer for the command `{description}`") + })?; - // Prevent pipe deadlock. - drop(cmd); + cmd.stdout(writer_clone).stderr(writer); + let handle = spawn(cmd)?; - reader - .read_to_end(output) - .with_context(|| format!("Failed to read the output of the command `{description}`"))?; + reader + .read_to_end(output) + .with_context(|| format!("Failed to read the output of the command `{description}`"))?; - output.push(b'\n'); + output.push(b'\n'); + + handle + } else { + cmd.stdout(Stdio::null()).stderr(Stdio::null()); + spawn(cmd)? + }; handle .wait() @@ -42,14 +54,14 @@ pub struct CargoCmd<'a> { /// 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: &'a mut Vec, + pub output: Option<&'a mut Vec>, /// true while developing Rustlings. pub dev: bool, } impl<'a> CargoCmd<'a> { /// Run `cargo SUBCOMMAND --bin EXERCISE_NAME … ARGS`. - pub fn run(&mut self) -> Result { + pub fn run(self) -> Result { let mut cmd = Command::new("cargo"); cmd.arg(self.subcommand); @@ -86,7 +98,7 @@ mod tests { cmd.arg("Hello"); let mut output = Vec::with_capacity(8); - run_cmd(cmd, "echo …", &mut output).unwrap(); + run_cmd(cmd, "echo …", Some(&mut output)).unwrap(); assert_eq!(output, b"Hello\n\n"); } diff --git a/src/dev/check.rs b/src/dev/check.rs index 5c35462c..2090dab8 100644 --- a/src/dev/check.rs +++ b/src/dev/check.rs @@ -184,8 +184,7 @@ fn check_exercises_unsolved(info_file: &InfoFile, target_dir: &Path) -> Result<( error_occurred.store(true, atomic::Ordering::Relaxed); }; - let mut output = Vec::with_capacity(OUTPUT_CAPACITY); - match exercise_info.run_exercise(&mut output, target_dir) { + match exercise_info.run_exercise(None, target_dir) { Ok(true) => error(b"Already solved!"), Ok(false) => (), Err(e) => error(e.to_string().as_bytes()), @@ -244,7 +243,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(&mut output, target_dir) { + match exercise_info.run_solution(Some(&mut output), target_dir) { Ok(true) => { paths.lock().unwrap().insert(PathBuf::from(path)); } diff --git a/src/exercise.rs b/src/exercise.rs index 960eec0e..5cb434bf 100644 --- a/src/exercise.rs +++ b/src/exercise.rs @@ -19,8 +19,10 @@ 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, output: &mut Vec, target_dir: &Path) -> Result { - writeln!(output, "{}", "Output".underlined())?; +fn run_bin(bin_name: &str, mut output: Option<&mut Vec>, target_dir: &Path) -> 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()); @@ -28,19 +30,25 @@ fn run_bin(bin_name: &str, output: &mut Vec, target_dir: &Path) -> Result, target_dir: &Path) -> Result { - output.clear(); + fn run( + &self, + bin_name: &str, + mut output: Option<&mut Vec>, + target_dir: &Path, + ) -> Result { + if let Some(output) = output.as_deref_mut() { + output.clear(); + } // Developing the official Rustlings. let dev = DEBUG_PROFILE && in_official_repo(); @@ -90,7 +105,7 @@ pub trait RunnableExercise { description: "cargo build …", hide_warnings: false, target_dir, - output, + output: output.as_deref_mut(), dev, } .run()?; @@ -99,7 +114,9 @@ pub trait RunnableExercise { } // Discard the output of `cargo build` because it will be shown again by Clippy. - output.clear(); + if let Some(output) = output.as_deref_mut() { + output.clear(); + } // `--profile test` is required to also check code with `[cfg(test)]`. let clippy_args: &[&str] = if self.strict_clippy() { @@ -114,7 +131,7 @@ pub trait RunnableExercise { description: "cargo clippy …", hide_warnings: false, target_dir, - output, + output: output.as_deref_mut(), dev, } .run()?; @@ -123,7 +140,7 @@ pub trait RunnableExercise { } if !self.test() { - return run_bin(bin_name, output, target_dir); + return run_bin(bin_name, output.as_deref_mut(), target_dir); } let test_success = CargoCmd { @@ -134,12 +151,12 @@ pub trait RunnableExercise { // Hide warnings because they are shown by Clippy. hide_warnings: true, target_dir, - output, + output: output.as_deref_mut(), dev, } .run()?; - let run_success = run_bin(bin_name, output, target_dir)?; + let run_success = run_bin(bin_name, output.as_deref_mut(), target_dir)?; Ok(test_success && run_success) } @@ -147,13 +164,13 @@ 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: &mut Vec, target_dir: &Path) -> Result { + fn run_exercise(&self, output: Option<&mut Vec>, target_dir: &Path) -> Result { self.run(self.name(), output, target_dir) } /// Compile, check and run the exercise's solution. /// The output is written to the `output` buffer after clearing it. - fn run_solution(&self, output: &mut Vec, target_dir: &Path) -> Result { + fn run_solution(&self, output: Option<&mut Vec>, target_dir: &Path) -> Result { let name = self.name(); let mut bin_name = String::with_capacity(name.len()); bin_name.push_str(name); diff --git a/src/run.rs b/src/run.rs index 3965a0d0..606f0a43 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(&mut output, app_state.target_dir())?; + let success = exercise.run_exercise(Some(&mut output), app_state.target_dir())?; let mut stdout = io::stdout().lock(); stdout.write_all(&output)?; diff --git a/src/watch/state.rs b/src/watch/state.rs index 26ff4118..8f01db7d 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(&mut self.output, self.app_state.target_dir())?; + .run_exercise(Some(&mut self.output), self.app_state.target_dir())?; if success { self.done_status = if let Some(solution_path) = self.app_state.current_solution_path()? {