diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index b8062b8dafe..bd4d166fb0f 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1666,7 +1666,7 @@ impl OverwriteMode { match self { Self::NoClobber => { if debug { - println!("{}", translate!("cp-debug-skipped", "path" => path.quote())); + print_stdout_line(translate!("cp-debug-skipped", "path" => path.quote()))?; } Err(CpError::Skipped(false)) } @@ -2100,7 +2100,7 @@ fn handle_existing_dest( if options.update == UpdateMode::None { if options.debug { - println!("skipped {}", dest.quote()); + print_stdout_line(format!("skipped {}", dest.quote()))?; } return Err(CpError::Skipped(false)); } @@ -2205,10 +2205,7 @@ fn delete_path(path: &Path, options: &Options) -> CopyResult<()> { match fs::remove_file(path) { Ok(()) => { if options.verbose { - println!( - "{}", - translate!("cp-verbose-removed", "path" => path.quote()) - ); + print_stdout_line(translate!("cp-verbose-removed", "path" => path.quote()))?; } } Err(err) if err.kind() == io::ErrorKind::NotFound => { @@ -2299,6 +2296,21 @@ fn print_paths(parents: bool, source: &Path, dest: &Path) -> CopyResult<()> { Ok(()) } +/// Print a single `--verbose`/`--debug` line to stdout, surfacing a write +/// failure (e.g. stdout redirected to a full disk, or a closed pipe) as a +/// normal error instead of panicking inside `println!`. Mirrors the buffered +/// write approach of [`print_paths`] so every user-facing verbose/debug line +/// shares the same non-panicking behaviour. See #10554. +fn print_stdout_line(line: impl Display) -> CopyResult<()> { + use std::io::Write; + + let mut out = io::BufWriter::new(io::stdout().lock()); + let write_err = |e| CpError::IoErrContext(e, translate!("cp-error-write")); + writeln!(out, "{line}").map_err(write_err)?; + out.flush().map_err(write_err)?; + Ok(()) +} + /// Handles the copy mode for a file copy operation. /// /// This function determines how to copy a file based on the provided options. @@ -2382,7 +2394,7 @@ fn handle_copy_mode( } UpdateMode::None => { if options.debug { - println!("skipped {}", dest.quote()); + print_stdout_line(format!("skipped {}", dest.quote()))?; } return Ok(PerformedAction::Skipped); diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 9ad4ef75c4d..8d59f14b489 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -120,6 +120,46 @@ fn test_cp_verbose_write_error_is_reported() { assert!(at.file_exists("dest_file")); } +// https://github.com/uutils/coreutils/issues/10554 +// #13310 made the `'src' -> 'dest'` verbose line non-panicking, but the +// remaining `--verbose`/`--debug` lines still used `println!` and panicked on a +// stdout write error (e.g. `> /dev/full`). Verify each of those paths now +// reports the write error instead of crashing, matching the `'src' -> 'dest'` +// behaviour fixed by `test_cp_verbose_write_error_is_reported`. +#[test] +#[cfg(target_os = "linux")] +fn test_cp_verbose_removed_write_error_is_reported_issue10554() { + let (at, mut ucmd) = at_and_ucmd!(); + at.touch("source_file"); + at.touch("dest_file"); + // `--remove-destination` always unlinks the existing dest first, emitting + // the `removed 'dest_file'` verbose line before the copy begins. + ucmd.arg("--remove-destination") + .arg("--verbose") + .arg("source_file") + .arg("dest_file") + .set_stdout(std::fs::File::create("/dev/full").unwrap()) + .fails() + .stderr_is("cp: write error: No space left on device\n"); +} + +#[test] +#[cfg(target_os = "linux")] +fn test_cp_debug_skipped_write_error_is_reported_issue10554() { + let (at, mut ucmd) = at_and_ucmd!(); + at.touch("source_file"); + at.touch("dest_file"); + // `--no-clobber --debug` against an existing dest emits the debug + // `skipped 'dest_file'` line via the same `print_stdout_line` helper. + ucmd.arg("--debug") + .arg("--no-clobber") + .arg("source_file") + .arg("dest_file") + .set_stdout(std::fs::File::create("/dev/full").unwrap()) + .fails() + .stderr_is("cp: write error: No space left on device\n"); +} + #[test] fn test_cp_cp() { let (at, mut ucmd) = at_and_ucmd!();