From 67fa01774223b08833c21baeb13bdec9e4a298a0 Mon Sep 17 00:00:00 2001 From: mo8it Date: Thu, 25 Apr 2024 01:56:01 +0200 Subject: [PATCH] Use os_pipe --- Cargo.lock | 11 +++++ Cargo.toml | 1 + src/app_state.rs | 13 +++++- src/exercise.rs | 110 +++++++++++++++++++++++++++++++++++++++------ src/main.rs | 6 ++- src/run.rs | 11 +++-- src/watch/state.rs | 26 +++-------- 7 files changed, 135 insertions(+), 43 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5f0b5978..823fec36 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -519,6 +519,16 @@ version = "1.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3fdb12b2476b595f9358c5161aa467c2438859caa136dec86c26fdd2efe17b92" +[[package]] +name = "os_pipe" +version = "1.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "57119c3b893986491ec9aa85056780d3a0f3cf4da7cc09dd3650dbd6c6738fb9" +dependencies = [ + "libc", + "windows-sys 0.52.0", +] + [[package]] name = "parking_lot" version = "0.12.1" @@ -677,6 +687,7 @@ dependencies = [ "crossterm", "hashbrown", "notify-debouncer-mini", + "os_pipe", "predicates", "ratatui", "rustlings-macros", diff --git a/Cargo.toml b/Cargo.toml index f4615b13..31e74568 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,6 +50,7 @@ clap = { version = "4.5.4", features = ["derive"] } crossterm = "0.27.0" hashbrown = "0.14.3" notify-debouncer-mini = "0.4.1" +os_pipe = "1.1.5" ratatui = "0.26.2" rustlings-macros = { path = "rustlings-macros", version = "6.0.0-beta.0" } serde.workspace = true diff --git a/src/app_state.rs b/src/app_state.rs index 33d3de2b..4160f6e5 100644 --- a/src/app_state.rs +++ b/src/app_state.rs @@ -11,7 +11,12 @@ use std::{ process::{Command, Stdio}, }; -use crate::{embedded::EMBEDDED_FILES, exercise::Exercise, info_file::ExerciseInfo, DEBUG_PROFILE}; +use crate::{ + embedded::EMBEDDED_FILES, + exercise::{Exercise, OUTPUT_CAPACITY}, + info_file::ExerciseInfo, + DEBUG_PROFILE, +}; const STATE_FILE_NAME: &str = ".rustlings-state.txt"; const BAD_INDEX_ERR: &str = "The current exercise index is higher than the number of exercises"; @@ -302,11 +307,13 @@ impl AppState { let Some(ind) = self.next_pending_exercise_ind() else { writer.write_all(RERUNNING_ALL_EXERCISES_MSG)?; + let mut output = Vec::with_capacity(OUTPUT_CAPACITY); for (exercise_ind, exercise) in self.exercises().iter().enumerate() { writer.write_fmt(format_args!("Running {exercise} ... "))?; writer.flush()?; - if !exercise.run()?.status.success() { + let success = exercise.run(&mut output)?; + if !success { writer.write_fmt(format_args!("{}\n\n", "FAILED".red()))?; self.current_exercise_ind = exercise_ind; @@ -322,6 +329,8 @@ impl AppState { } writer.write_fmt(format_args!("{}\n", "ok".green()))?; + + output.clear(); } writer.execute(Clear(ClearType::All))?; diff --git a/src/exercise.rs b/src/exercise.rs index 45ac2086..17cc8d77 100644 --- a/src/exercise.rs +++ b/src/exercise.rs @@ -2,11 +2,42 @@ use anyhow::{Context, Result}; use crossterm::style::{style, StyledContent, Stylize}; use std::{ fmt::{self, Display, Formatter}, - path::Path, - process::{Command, Output}, + io::{Read, Write}, + process::Command, }; -use crate::{info_file::Mode, terminal_link::TerminalFileLink, DEBUG_PROFILE}; +use crate::{in_official_repo, info_file::Mode, terminal_link::TerminalFileLink, DEBUG_PROFILE}; + +// TODO +pub const OUTPUT_CAPACITY: usize = 1 << 12; + +fn run_command(mut cmd: Command, cmd_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 `{cmd_description}``") + })?; + + let mut handle = cmd + .stdout(writer.try_clone().with_context(|| { + format!("Failed to clone the pipe writer for the command `{cmd_description}`") + })?) + .stderr(writer) + .spawn() + .with_context(|| format!("Failed to run the command `{cmd_description}`"))?; + + // Prevent pipe deadlock. + drop(cmd); + + reader + .read_to_end(output) + .with_context(|| format!("Failed to read the output of the command `{cmd_description}`"))?; + + output.push(b'\n'); + + handle + .wait() + .with_context(|| format!("Failed to wait on the command `{cmd_description}` to exit")) + .map(|status| status.success()) +} pub struct Exercise { pub dir: Option<&'static str>, @@ -22,13 +53,30 @@ pub struct Exercise { } impl Exercise { - fn cargo_cmd(&self, command: &str, args: &[&str]) -> Result { + fn run_bin(&self, output: &mut Vec) -> Result { + writeln!(output, "{}", "Output".bold().magenta().underlined())?; + + let bin_path = format!("target/debug/{}", self.name); + run_command(Command::new(&bin_path), &bin_path, output) + } + + fn cargo_cmd( + &self, + command: &str, + args: &[&str], + cmd_description: &str, + output: &mut Vec, + dev: bool, + ) -> Result { let mut cmd = Command::new("cargo"); cmd.arg(command); // A hack to make `cargo run` work when developing Rustlings. - if DEBUG_PROFILE && Path::new("tests").exists() { - cmd.arg("--manifest-path").arg("dev/Cargo.toml"); + if dev { + cmd.arg("--manifest-path") + .arg("dev/Cargo.toml") + .arg("--target-dir") + .arg("target"); } cmd.arg("--color") @@ -36,15 +84,43 @@ impl Exercise { .arg("-q") .arg("--bin") .arg(self.name) - .args(args) - .output() - .context("Failed to run Cargo") + .args(args); + + run_command(cmd, cmd_description, output) } - pub fn run(&self) -> Result { + fn cargo_cmd_with_bin_output( + &self, + command: &str, + args: &[&str], + cmd_description: &str, + output: &mut Vec, + dev: bool, + ) -> Result { + // Discard the output of `cargo build` because it will be shown again by the Cargo command. + output.clear(); + + let cargo_cmd_success = self.cargo_cmd(command, args, cmd_description, output, dev)?; + + let run_success = self.run_bin(output)?; + + Ok(cargo_cmd_success && run_success) + } + + pub fn run(&self, output: &mut Vec) -> Result { + output.clear(); + + // Developing the official Rustlings. + let dev = DEBUG_PROFILE && in_official_repo(); + + let build_success = self.cargo_cmd("build", &[], "cargo build …", output, dev)?; + if !build_success { + return Ok(false); + } + match self.mode { - Mode::Run => self.cargo_cmd("run", &[]), - Mode::Test => self.cargo_cmd( + Mode::Run => self.run_bin(output), + Mode::Test => self.cargo_cmd_with_bin_output( "test", &[ "--", @@ -54,10 +130,16 @@ impl Exercise { "--format", "pretty", ], + "cargo test …", + output, + dev, ), - Mode::Clippy => self.cargo_cmd( + Mode::Clippy => self.cargo_cmd_with_bin_output( "clippy", - &["--", "-D", "warnings", "-D", "clippy::float_cmp"], + &["--", "-D", "warnings"], + "cargo clippy …", + output, + dev, ), } } diff --git a/src/main.rs b/src/main.rs index 790fff69..a9285044 100644 --- a/src/main.rs +++ b/src/main.rs @@ -75,10 +75,14 @@ enum Subcommands { Dev(DevCommands), } +fn in_official_repo() -> bool { + Path::new("dev/rustlings-repo.txt").exists() +} + fn main() -> Result<()> { let args = Args::parse(); - if !DEBUG_PROFILE && Path::new("dev/rustlings-repo.txt").exists() { + if !DEBUG_PROFILE && in_official_repo() { bail!("{OLD_METHOD_ERR}"); } diff --git a/src/run.rs b/src/run.rs index a2b69720..1db8dcbb 100644 --- a/src/run.rs +++ b/src/run.rs @@ -4,20 +4,19 @@ use std::io::{self, Write}; use crate::{ app_state::{AppState, ExercisesProgress}, + exercise::OUTPUT_CAPACITY, terminal_link::TerminalFileLink, }; pub fn run(app_state: &mut AppState) -> Result<()> { let exercise = app_state.current_exercise(); - let output = exercise.run()?; + let mut output = Vec::with_capacity(OUTPUT_CAPACITY); + let success = exercise.run(&mut output)?; let mut stdout = io::stdout().lock(); - stdout.write_all(&output.stdout)?; - stdout.write_all(b"\n")?; - stdout.write_all(&output.stderr)?; - stdout.flush()?; + stdout.write_all(&output)?; - if !output.status.success() { + if !success { app_state.set_pending(app_state.current_exercise_ind())?; bail!( diff --git a/src/watch/state.rs b/src/watch/state.rs index 5f4abf3c..df492dc8 100644 --- a/src/watch/state.rs +++ b/src/watch/state.rs @@ -8,6 +8,7 @@ use std::io::{self, StdoutLock, Write}; use crate::{ app_state::{AppState, ExercisesProgress}, + exercise::OUTPUT_CAPACITY, progress_bar::progress_bar, terminal_link::TerminalFileLink, }; @@ -21,8 +22,7 @@ enum DoneStatus { pub struct WatchState<'a> { writer: StdoutLock<'a>, app_state: &'a mut AppState, - stdout: Option>, - stderr: Option>, + output: Vec, show_hint: bool, done_status: DoneStatus, manual_run: bool, @@ -35,8 +35,7 @@ impl<'a> WatchState<'a> { Self { writer, app_state, - stdout: None, - stderr: None, + output: Vec::with_capacity(OUTPUT_CAPACITY), show_hint: false, done_status: DoneStatus::Pending, manual_run, @@ -51,11 +50,8 @@ impl<'a> WatchState<'a> { pub fn run_current_exercise(&mut self) -> Result<()> { self.show_hint = false; - let output = self.app_state.current_exercise().run()?; - self.stdout = Some(output.stdout); - - if output.status.success() { - self.stderr = None; + let success = self.app_state.current_exercise().run(&mut self.output)?; + if success { self.done_status = if let Some(solution_path) = self.app_state.current_solution_path()? { DoneStatus::DoneWithSolution(solution_path) @@ -66,7 +62,6 @@ impl<'a> WatchState<'a> { self.app_state .set_pending(self.app_state.current_exercise_ind())?; - self.stderr = Some(output.stderr); self.done_status = DoneStatus::Pending; } @@ -116,16 +111,7 @@ impl<'a> WatchState<'a> { self.writer.execute(Clear(ClearType::All))?; - if let Some(stdout) = &self.stdout { - self.writer.write_all(stdout)?; - self.writer.write_all(b"\n")?; - } - - if let Some(stderr) = &self.stderr { - self.writer.write_all(stderr)?; - self.writer.write_all(b"\n")?; - } - + self.writer.write_all(&self.output)?; self.writer.write_all(b"\n")?; if self.show_hint {