From 4ffc34956f9a00d3d5279758feabb5862f5dc9c4 Mon Sep 17 00:00:00 2001 From: Tavian Barnes Date: Tue, 27 Sep 2022 16:09:21 -0400 Subject: [PATCH] Make --strip-cwd-prefix apply to -x/-X Fixes #898. --- src/app.rs | 6 ++++-- src/dir_entry.rs | 21 +++++++++++++++++++++ src/exec/job.rs | 28 +++++++++++++++------------- src/main.rs | 7 +++++-- src/output.rs | 17 +++-------------- src/walk.rs | 16 +++------------- tests/tests.rs | 15 +++++++++++++++ 7 files changed, 66 insertions(+), 44 deletions(-) diff --git a/src/app.rs b/src/app.rs index 1a904d1..fa9416a 100644 --- a/src/app.rs +++ b/src/app.rs @@ -724,8 +724,10 @@ pub fn build_app() -> Command<'static> { .hide_short_help(true) .help("strip './' prefix from -0/--print0 output") .long_help( - "By default, relative paths are prefixed with './' when -0/--print0 is given, to \ - make them safer for use with xargs. Use this flag to disable this behaviour." + "By default, relative paths are prefixed with './' when -x/--exec, \ + -X/--exec-batch, or -0/--print0 are given, to reduce the risk of a \ + path starting with '-' being treated as a command line option. Use \ + this flag to disable this behaviour." ) ); diff --git a/src/dir_entry.rs b/src/dir_entry.rs index 5def5de..7c34be5 100644 --- a/src/dir_entry.rs +++ b/src/dir_entry.rs @@ -5,6 +5,9 @@ use std::{ use once_cell::unsync::OnceCell; +use crate::config::Config; +use crate::filesystem::strip_current_dir; + enum DirEntryInner { Normal(ignore::DirEntry), BrokenSymlink(PathBuf), @@ -45,6 +48,24 @@ impl DirEntry { } } + /// Returns the path as it should be presented to the user. + pub fn stripped_path(&self, config: &Config) -> &Path { + if config.strip_cwd_prefix { + strip_current_dir(self.path()) + } else { + self.path() + } + } + + /// Returns the path as it should be presented to the user. + pub fn into_stripped_path(self, config: &Config) -> PathBuf { + if config.strip_cwd_prefix { + self.stripped_path(config).to_path_buf() + } else { + self.into_path() + } + } + pub fn file_type(&self) -> Option { match &self.inner { DirEntryInner::Normal(e) => e.file_type(), diff --git a/src/exec/job.rs b/src/exec/job.rs index 9b95ac2..b803f79 100644 --- a/src/exec/job.rs +++ b/src/exec/job.rs @@ -1,6 +1,7 @@ use std::sync::mpsc::Receiver; use std::sync::{Arc, Mutex}; +use crate::config::Config; use crate::dir_entry::DirEntry; use crate::error::print_error; use crate::exit_codes::{merge_exitcodes, ExitCode}; @@ -15,9 +16,11 @@ pub fn job( rx: Arc>>, cmd: Arc, out_perm: Arc>, - show_filesystem_errors: bool, - buffer_output: bool, + config: &Config, ) -> ExitCode { + // Output should be buffered when only running a single thread + let buffer_output: bool = config.threads > 1; + let mut results: Vec = Vec::new(); loop { // Create a lock on the shared receiver for this thread. @@ -28,7 +31,7 @@ pub fn job( let dir_entry: DirEntry = match lock.recv() { Ok(WorkerResult::Entry(dir_entry)) => dir_entry, Ok(WorkerResult::Error(err)) => { - if show_filesystem_errors { + if config.show_filesystem_errors { print_error(err.to_string()); } continue; @@ -39,29 +42,28 @@ pub fn job( // 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.execute(dir_entry.path(), Arc::clone(&out_perm), buffer_output)) + results.push(cmd.execute( + dir_entry.stripped_path(config), + Arc::clone(&out_perm), + buffer_output, + )) } // Returns error in case of any error. merge_exitcodes(results) } -pub fn batch( - rx: Receiver, - cmd: &CommandSet, - show_filesystem_errors: bool, - limit: usize, -) -> ExitCode { +pub fn batch(rx: Receiver, cmd: &CommandSet, config: &Config) -> ExitCode { let paths = rx .into_iter() .filter_map(|worker_result| match worker_result { - WorkerResult::Entry(dir_entry) => Some(dir_entry.into_path()), + WorkerResult::Entry(dir_entry) => Some(dir_entry.into_stripped_path(config)), WorkerResult::Error(err) => { - if show_filesystem_errors { + if config.show_filesystem_errors { print_error(err.to_string()); } None } }); - cmd.execute_batch(paths, limit) + cmd.execute_batch(paths, config.batch_size) } diff --git a/src/main.rs b/src/main.rs index dba6d4b..ece5b4d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -383,9 +383,12 @@ fn construct_config(matches: clap::ArgMatches, pattern_regex: &str) -> Result String { path.replace(std::path::MAIN_SEPARATOR, new_path_separator) } -fn stripped_path<'a>(entry: &'a DirEntry, config: &Config) -> &'a Path { - let path = entry.path(); - if config.strip_cwd_prefix { - strip_current_dir(path) - } else { - path - } -} - // TODO: this function is performance critical and can probably be optimized pub fn print_entry(stdout: &mut W, entry: &DirEntry, config: &Config) { let r = if let Some(ref ls_colors) = config.ls_colors { @@ -74,7 +63,7 @@ fn print_entry_colorized( ) -> io::Result<()> { // Split the path between the parent and the last component let mut offset = 0; - let path = stripped_path(entry, config); + let path = entry.stripped_path(config); let path_str = path.to_string_lossy(); if let Some(parent) = path.parent() { @@ -130,7 +119,7 @@ fn print_entry_uncolorized_base( config: &Config, ) -> io::Result<()> { let separator = if config.null_separator { "\0" } else { "\n" }; - let path = stripped_path(entry, config); + let path = entry.stripped_path(config); let mut path_string = path.to_string_lossy(); if let Some(ref separator) = config.path_separator { @@ -164,7 +153,7 @@ fn print_entry_uncolorized( } else { // Print path as raw bytes, allowing invalid UTF-8 filenames to be passed to other processes let separator = if config.null_separator { b"\0" } else { b"\n" }; - stdout.write_all(stripped_path(entry, config).as_os_str().as_bytes())?; + stdout.write_all(entry.stripped_path(config).as_os_str().as_bytes())?; print_trailing_slash(stdout, entry, config, None)?; stdout.write_all(separator) } diff --git a/src/walk.rs b/src/walk.rs index 463417e..62dcc78 100644 --- a/src/walk.rs +++ b/src/walk.rs @@ -341,15 +341,12 @@ fn spawn_receiver( let quit_flag = Arc::clone(quit_flag); let interrupt_flag = Arc::clone(interrupt_flag); - 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 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, config.batch_size) + exec::batch(rx, cmd, &config) } else { let shared_rx = Arc::new(Mutex::new(rx)); @@ -358,20 +355,13 @@ fn spawn_receiver( // Each spawned job will store it's thread handle in here. let mut handles = Vec::with_capacity(threads); for _ in 0..threads { + let config = Arc::clone(&config); let rx = Arc::clone(&shared_rx); let cmd = Arc::clone(cmd); 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, - enable_output_buffering, - ) - }); + let handle = thread::spawn(move || exec::job(rx, cmd, out_perm, &config)); // Push the handle of the spawned thread into the vector for later joining. handles.push(handle); diff --git a/tests/tests.rs b/tests/tests.rs index 0bed407..50157c4 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -1327,6 +1327,16 @@ fn test_exec() { ./one/two/three/directory_foo", ); + te.assert_output( + &["foo", "--strip-cwd-prefix", "--exec", "echo", "{}"], + "a.foo + one/b.foo + one/two/C.Foo2 + one/two/c.foo + one/two/three/d.foo + one/two/three/directory_foo", + ); + te.assert_output( &["foo", "--exec", "echo", "{.}"], "a @@ -1452,6 +1462,11 @@ fn test_exec_batch() { "./a.foo ./one/b.foo ./one/two/C.Foo2 ./one/two/c.foo ./one/two/three/d.foo ./one/two/three/directory_foo", ); + te.assert_output( + &["foo", "--strip-cwd-prefix", "--exec-batch", "echo", "{}"], + "a.foo one/b.foo one/two/C.Foo2 one/two/c.foo one/two/three/d.foo one/two/three/directory_foo", + ); + te.assert_output( &["foo", "--exec-batch", "echo", "{/}"], "a.foo b.foo C.Foo2 c.foo d.foo directory_foo",