From bf37c27a3873d5eb2bcbafd8e6325f9b3f8a2b95 Mon Sep 17 00:00:00 2001 From: Thayne McCombs Date: Mon, 9 Aug 2021 01:02:30 -0600 Subject: [PATCH] Address feedback for removing buffering when running on a single thread --- src/exec/command.rs | 18 ++++++++++-------- src/exec/job.rs | 10 +++++----- src/exec/mod.rs | 13 +++++++++---- src/walk.rs | 19 +++++++++++-------- 4 files changed, 35 insertions(+), 25 deletions(-) diff --git a/src/exec/command.rs b/src/exec/command.rs index 5d546f2..9c2c5e2 100644 --- a/src/exec/command.rs +++ b/src/exec/command.rs @@ -7,18 +7,20 @@ use crate::error::print_error; use crate::exit_codes::ExitCode; /// Executes a command. -pub fn execute_command(mut cmd: Command, out_perm: &Mutex<()>, is_multithread: bool) -> ExitCode { - let output; +pub fn execute_command( + mut cmd: Command, + out_perm: &Mutex<()>, + enable_output_buffering: bool, +) -> ExitCode { // Spawn the supplied command. - if is_multithread { - output = cmd.output(); + let output = if enable_output_buffering { + cmd.output() } else { // If running on only one thread, don't buffer output // Allows for viewing and interacting with intermediate command output - let child = cmd.spawn().expect("Failed to execute command"); - output = child.wait_with_output(); - } - + cmd.spawn().and_then(|c| c.wait_with_output()) + }; + // Then wait for the command to exit, if it was spawned. match output { Ok(output) => { diff --git a/src/exec/job.rs b/src/exec/job.rs index 9b27516..c43be5b 100644 --- a/src/exec/job.rs +++ b/src/exec/job.rs @@ -16,7 +16,7 @@ pub fn job( cmd: Arc, out_perm: Arc>, show_filesystem_errors: bool, - is_multithread: bool, + buffer_output: bool, ) -> ExitCode { let mut results: Vec = Vec::new(); loop { @@ -35,11 +35,11 @@ pub fn job( } Err(_) => break, }; - + // Drop the lock so that other threads can read from the receiver. drop(lock); // Generate a command, execute it and store its exit code. - results.push(cmd.generate_and_execute(&value, Arc::clone(&out_perm), is_multithread)) + results.push(cmd.generate_and_execute(&value, Arc::clone(&out_perm), buffer_output)) } // Returns error in case of any error. merge_exitcodes(&results) @@ -49,7 +49,7 @@ pub fn batch( rx: Receiver, cmd: &CommandTemplate, show_filesystem_errors: bool, - is_multithread: bool, + buffer_output: bool, ) -> ExitCode { let paths = rx.iter().filter_map(|value| match value { WorkerResult::Entry(val) => Some(val), @@ -60,5 +60,5 @@ pub fn batch( None } }); - cmd.generate_and_execute_batch(paths, is_multithread) + cmd.generate_and_execute_batch(paths, buffer_output) } diff --git a/src/exec/mod.rs b/src/exec/mod.rs index 9a3d6df..d1f7d74 100644 --- a/src/exec/mod.rs +++ b/src/exec/mod.rs @@ -139,7 +139,12 @@ impl CommandTemplate { /// /// Using the internal `args` field, and a supplied `input` variable, a `Command` will be /// build. Once all arguments have been processed, the command is executed. - pub fn generate_and_execute(&self, input: &Path, out_perm: Arc>, is_multithread: bool) -> ExitCode { + pub fn generate_and_execute( + &self, + input: &Path, + out_perm: Arc>, + buffer_output: bool, + ) -> ExitCode { let input = strip_current_dir(input); let mut cmd = Command::new(self.args[0].generate(&input, self.path_separator.as_deref())); @@ -147,14 +152,14 @@ impl CommandTemplate { cmd.arg(arg.generate(&input, self.path_separator.as_deref())); } - execute_command(cmd, &out_perm, is_multithread) + execute_command(cmd, &out_perm, buffer_output) } pub fn in_batch_mode(&self) -> bool { self.mode == ExecutionMode::Batch } - pub fn generate_and_execute_batch(&self, paths: I, is_multithread: bool) -> ExitCode + pub fn generate_and_execute_batch(&self, paths: I, buffer_output: bool) -> ExitCode where I: Iterator, { @@ -182,7 +187,7 @@ impl CommandTemplate { } if has_path { - execute_command(cmd, &Mutex::new(()), is_multithread) + execute_command(cmd, &Mutex::new(()), buffer_output) } else { ExitCode::Success } diff --git a/src/walk.rs b/src/walk.rs index 6edb9b2..3e1e5e4 100644 --- a/src/walk.rs +++ b/src/walk.rs @@ -171,16 +171,12 @@ fn spawn_receiver( let show_filesystem_errors = config.show_filesystem_errors; let threads = config.threads; // This will be used to check if output should be buffered when only running a single thread - let is_multithread: bool = if threads > 1 { - true - } else { - false - }; + let enable_output_buffering: bool = threads > 1; thread::spawn(move || { // This will be set to `Some` if the `--exec` argument was supplied. if let Some(ref cmd) = config.command { if cmd.in_batch_mode() { - exec::batch(rx, cmd, show_filesystem_errors, is_multithread) + exec::batch(rx, cmd, show_filesystem_errors, enable_output_buffering) } else { let shared_rx = Arc::new(Mutex::new(rx)); @@ -194,8 +190,15 @@ fn spawn_receiver( let out_perm = Arc::clone(&out_perm); // Spawn a job thread that will listen for and execute inputs. - let handle = - thread::spawn(move || exec::job(rx, cmd, out_perm, show_filesystem_errors, is_multithread)); + let handle = thread::spawn(move || { + exec::job( + rx, + cmd, + out_perm, + show_filesystem_errors, + enable_output_buffering, + ) + }); // Push the handle of the spawned thread into the vector for later joining. handles.push(handle);