diff --git a/src/uu/cp/src/copydir.rs b/src/uu/cp/src/copydir.rs index c482c7fde..a4901903d 100644 --- a/src/uu/cp/src/copydir.rs +++ b/src/uu/cp/src/copydir.rs @@ -99,6 +99,9 @@ struct Context<'a> { /// The target path to which the directory will be copied. target: &'a Path, + /// Whether the target is an existing file. Cached to avoid repeated `stat` calls. + target_is_file: bool, + /// The source path from which the directory will be copied. root: &'a Path, } @@ -107,6 +110,7 @@ impl<'a> Context<'a> { fn new(root: &'a Path, target: &'a Path) -> io::Result { let current_dir = env::current_dir()?; let root_path = current_dir.join(root); + let target_is_file = target.is_file(); let root_parent = if target.exists() && !root.to_str().unwrap().ends_with("/.") { root_path.parent().map(|p| p.to_path_buf()) } else if root == Path::new(".") && target.is_dir() { @@ -122,6 +126,7 @@ impl<'a> Context<'a> { current_dir, root_parent, target, + target_is_file, root, }) } @@ -213,7 +218,7 @@ impl Entry { } let local_to_target = context.target.join(descendant); - let target_is_file = context.target.is_file(); + let target_is_file = context.target_is_file; Ok(Self { source_absolute, source_relative, @@ -227,54 +232,73 @@ impl Entry { /// Copy a single entry during a directory traversal. fn copy_direntry( progress_bar: Option<&ProgressBar>, - entry: Entry, + entry: &Entry, + entry_is_symlink: bool, + entry_is_dir_no_follow: bool, options: &Options, symlinked_files: &mut HashSet, preserve_hard_links: bool, copied_destinations: &HashSet, copied_files: &mut HashMap, + created_parent_dirs: &mut HashSet, ) -> CopyResult<()> { - let Entry { - source_absolute, - source_relative, - local_to_target, - target_is_file, - } = entry; + let source_is_symlink = entry_is_symlink; + let source_is_dir = if source_is_symlink && !options.dereference { + false + } else if source_is_symlink { + entry.source_absolute.is_dir() + } else { + entry_is_dir_no_follow + }; // If the source is a symbolic link and the options tell us not to // dereference the link, then copy the link object itself. - if source_absolute.is_symlink() && !options.dereference { - return copy_link(&source_absolute, &local_to_target, symlinked_files, options); + if source_is_symlink && !options.dereference { + return copy_link( + &entry.source_absolute, + &entry.local_to_target, + symlinked_files, + options, + ); } // If the source is a directory and the destination does not // exist, ... - if source_absolute.is_dir() && !local_to_target.exists() { - return if target_is_file { + if source_is_dir && !entry.local_to_target.exists() { + return if entry.target_is_file { Err(translate!("cp-error-cannot-overwrite-non-directory-with-directory").into()) } else { - build_dir(&local_to_target, false, options, Some(&source_absolute))?; + build_dir( + &entry.local_to_target, + false, + options, + Some(&entry.source_absolute), + )?; if options.verbose { - println!("{}", context_for(&source_relative, &local_to_target)); + println!( + "{}", + context_for(&entry.source_relative, &entry.local_to_target) + ); } Ok(()) }; } // If the source is not a directory, then we need to copy the file. - if !source_absolute.is_dir() { + if !source_is_dir { if let Err(err) = copy_file( progress_bar, - &source_absolute, - local_to_target.as_path(), + &entry.source_absolute, + entry.local_to_target.as_path(), options, symlinked_files, copied_destinations, copied_files, + created_parent_dirs, false, ) { if preserve_hard_links { - if !source_absolute.is_symlink() { + if !source_is_symlink { return Err(err); } // silent the error with a symlink @@ -292,7 +316,10 @@ fn copy_direntry( show!(uio_error!( e, "{}", - translate!("cp-error-cannot-open-for-reading", "source" => source_relative.quote()), + translate!( + "cp-error-cannot-open-for-reading", + "source" => entry.source_relative.quote() + ), )); } e => return Err(e), @@ -320,6 +347,7 @@ pub(crate) fn copy_directory( symlinked_files: &mut HashSet, copied_destinations: &HashSet, copied_files: &mut HashMap, + created_parent_dirs: &mut HashSet, source_in_command_line: bool, ) -> CopyResult<()> { // if no-dereference is enabled and this is a symlink, copy it as a file @@ -332,6 +360,7 @@ pub(crate) fn copy_directory( symlinked_files, copied_destinations, copied_files, + created_parent_dirs, source_in_command_line, ); } @@ -405,16 +434,29 @@ pub(crate) fn copy_directory( { match direntry_result { Ok(direntry) => { - let entry = Entry::new(&context, direntry.path(), options.no_target_dir)?; + let direntry_type = direntry.file_type(); + let direntry_path = direntry.path(); + let (entry_is_symlink, entry_is_dir_no_follow) = + match direntry_path.symlink_metadata() { + Ok(metadata) => { + let file_type = metadata.file_type(); + (file_type.is_symlink(), file_type.is_dir()) + } + Err(_) => (direntry_type.is_symlink(), direntry_type.is_dir()), + }; + let entry = Entry::new(&context, direntry_path, options.no_target_dir)?; copy_direntry( progress_bar, - entry, + &entry, + entry_is_symlink, + entry_is_dir_no_follow, options, symlinked_files, preserve_hard_links, copied_destinations, copied_files, + created_parent_dirs, )?; // We omit certain permissions when creating directories @@ -428,19 +470,17 @@ pub(crate) fn copy_directory( // (Note that there can be more than one! We might step out of // `./a/b/c` into `./a/`, in which case we'll need to fix the // permissions of both `./a/b/c` and `./a/b`, in that order.) - if direntry.file_type().is_dir() { + let is_dir_for_permissions = + entry_is_dir_no_follow || (options.dereference && direntry_path.is_dir()); + if is_dir_for_permissions { // Add this directory to our list for permission fixing later - let entry_for_tracking = - Entry::new(&context, direntry.path(), options.no_target_dir)?; - dirs_needing_permissions.push(( - entry_for_tracking.source_absolute, - entry_for_tracking.local_to_target, - )); + dirs_needing_permissions + .push((entry.source_absolute.clone(), entry.local_to_target.clone())); // If true, last_iter is not a parent of this iter. // The means we just exited a directory. let went_up = if let Some(last_iter) = &last_iter { - last_iter.path().strip_prefix(direntry.path()).is_ok() + last_iter.path().strip_prefix(direntry_path).is_ok() } else { false }; @@ -454,14 +494,14 @@ pub(crate) fn copy_directory( // // All the unwraps() here are unreachable. let last_iter = last_iter.as_ref().unwrap(); - let diff = last_iter.path().strip_prefix(direntry.path()).unwrap(); + let diff = last_iter.path().strip_prefix(direntry_path).unwrap(); // Fix permissions for every entry in `diff`, inside-out. // We skip the last directory (which will be `.`) because // its permissions will be fixed when we walk _out_ of it. // (at this point, we might not be done copying `.`!) for p in skip_last(diff.ancestors()) { - let src = direntry.path().join(p); + let src = direntry_path.join(p); let entry = Entry::new(&context, &src, options.no_target_dir)?; copy_attributes( diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 6c544310c..1754fbb0a 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1335,6 +1335,7 @@ pub fn copy(sources: &[PathBuf], target: &Path, options: &Options) -> CopyResult // remember the copied destinations for further usage. // we can't use copied_files as it is because the key is the source file's information. let mut copied_destinations: HashSet = HashSet::with_capacity(sources.len()); + let mut created_parent_dirs: HashSet = HashSet::new(); let progress_bar = if options.progress_bar { let pb = ProgressBar::new(disk_usage(sources, options.recursive)?) @@ -1390,6 +1391,7 @@ pub fn copy(sources: &[PathBuf], target: &Path, options: &Options) -> CopyResult &mut symlinked_files, &copied_destinations, &mut copied_files, + &mut created_parent_dirs, ) { show_error_if_needed(&error); if !matches!(error, CpError::Skipped(false)) { @@ -1464,6 +1466,7 @@ fn copy_source( symlinked_files: &mut HashSet, copied_destinations: &HashSet, copied_files: &mut HashMap, + created_parent_dirs: &mut HashSet, ) -> CopyResult<()> { let source_path = Path::new(&source); if source_path.is_dir() && (options.dereference || !source_path.is_symlink()) { @@ -1476,6 +1479,7 @@ fn copy_source( symlinked_files, copied_destinations, copied_files, + created_parent_dirs, true, ) } else { @@ -1489,6 +1493,7 @@ fn copy_source( symlinked_files, copied_destinations, copied_files, + created_parent_dirs, true, ); if options.parents { @@ -1970,21 +1975,18 @@ fn delete_dest_if_needed_and_allowed( }; if delete_dest { - fs::remove_file(dest)?; + match fs::remove_file(dest) { + Ok(()) => {} + Err(err) if err.kind() == io::ErrorKind::NotFound => { + // target could have been deleted earlier (e.g. same-file with --remove-destination) + } + Err(err) => return Err(err.into()), + } } Ok(()) } -/// Decide whether the given path exists. -fn file_or_link_exists(path: &Path) -> bool { - // Using `Path.exists()` or `Path.try_exists()` is not sufficient, - // because if `path` is a symbolic link and there are too many - // levels of symbolic link, then those methods will return false - // or an OS error. - path.symlink_metadata().is_ok() -} - /// Zip the ancestors of a source path and destination path. /// /// # Examples @@ -2082,6 +2084,7 @@ fn handle_copy_mode( source_in_command_line: bool, source_is_fifo: bool, source_is_socket: bool, + created_parent_dirs: &mut HashSet, #[cfg(unix)] source_is_stream: bool, ) -> CopyResult { let source_is_symlink = source_metadata.is_symlink(); @@ -2123,6 +2126,7 @@ fn handle_copy_mode( source_is_fifo, source_is_socket, symlinked_files, + created_parent_dirs, #[cfg(unix)] source_is_stream, )?; @@ -2146,6 +2150,7 @@ fn handle_copy_mode( source_is_fifo, source_is_socket, symlinked_files, + created_parent_dirs, #[cfg(unix)] source_is_stream, )?; @@ -2182,6 +2187,7 @@ fn handle_copy_mode( source_is_fifo, source_is_socket, symlinked_files, + created_parent_dirs, #[cfg(unix)] source_is_stream, )?; @@ -2197,6 +2203,7 @@ fn handle_copy_mode( source_is_fifo, source_is_socket, symlinked_files, + created_parent_dirs, #[cfg(unix)] source_is_stream, )?; @@ -2228,16 +2235,14 @@ fn handle_copy_mode( // Allow unused variables for Windows (on options) #[allow(unused_variables)] fn calculate_dest_permissions( + dest_metadata: Option<&Metadata>, dest: &Path, source_metadata: &Metadata, options: &Options, context: &str, ) -> CopyResult { - if dest.exists() { - Ok(dest - .symlink_metadata() - .map_err(|e| CpError::IoErrContext(e, context.to_owned()))? - .permissions()) + if let Some(metadata) = dest_metadata { + Ok(metadata.permissions()) } else { #[cfg(unix)] { @@ -2277,10 +2282,16 @@ fn copy_file( symlinked_files: &mut HashSet, copied_destinations: &HashSet, copied_files: &mut HashMap, + created_parent_dirs: &mut HashSet, source_in_command_line: bool, ) -> CopyResult<()> { let source_is_symlink = source.is_symlink(); - let dest_is_symlink = dest.is_symlink(); + let initial_dest_metadata = dest.symlink_metadata().ok(); + let dest_is_symlink = initial_dest_metadata + .as_ref() + .map(|md| md.file_type().is_symlink()) + .unwrap_or(false); + let dest_target_exists = dest.try_exists().unwrap_or(false); // Fail if dest is a dangling symlink or a symlink this program created previously if dest_is_symlink { if FileInformation::from_path(dest, false) @@ -2302,7 +2313,7 @@ fn copy_file( let copy_contents = options.dereference(source_in_command_line) || !source_is_symlink; if copy_contents - && !dest.exists() + && !dest_target_exists && !matches!( options.overwrite, OverwriteMode::Clobber(ClobberMode::RemoveDestination) @@ -2335,7 +2346,7 @@ fn copy_file( fs::remove_file(dest)?; } - if file_or_link_exists(dest) + if initial_dest_metadata.is_some() && (!options.attributes_only || matches!( options.overwrite, @@ -2418,7 +2429,15 @@ fn copy_file( })? }; - let dest_permissions = calculate_dest_permissions(dest, &source_metadata, options, context)?; + let dest_metadata = dest.symlink_metadata().ok(); + + let dest_permissions = calculate_dest_permissions( + dest_metadata.as_ref(), + dest, + &source_metadata, + options, + context, + )?; #[cfg(unix)] let source_is_fifo = source_metadata.file_type().is_fifo(); @@ -2441,6 +2460,7 @@ fn copy_file( source_in_command_line, source_is_fifo, source_is_socket, + created_parent_dirs, #[cfg(unix)] source_is_stream, )?; @@ -2493,7 +2513,7 @@ fn copy_file( ); if let Some(progress_bar) = progress_bar { - progress_bar.inc(fs::metadata(source)?.len()); + progress_bar.inc(source_metadata.len()); } Ok(()) @@ -2569,11 +2589,14 @@ fn copy_helper( source_is_fifo: bool, source_is_socket: bool, symlinked_files: &mut HashSet, + created_parent_dirs: &mut HashSet, #[cfg(unix)] source_is_stream: bool, ) -> CopyResult<()> { if options.parents { let parent = dest.parent().unwrap_or(dest); - fs::create_dir_all(parent)?; + if created_parent_dirs.insert(parent.to_path_buf()) { + fs::create_dir_all(parent)?; + } } if path_ends_with_terminator(dest) && !dest.is_dir() {