diff --git a/src/uu/cp/src/copydir.rs b/src/uu/cp/src/copydir.rs index 86a4081ee..e243782a9 100644 --- a/src/uu/cp/src/copydir.rs +++ b/src/uu/cp/src/copydir.rs @@ -213,9 +213,8 @@ fn copy_direntry( // If the source is not a directory, then we need to copy the file. if !source_absolute.is_dir() { if preserve_hard_links { - let mut found_hard_link = false; let dest = local_to_target.as_path().to_path_buf(); - preserve_hardlinks(hard_links, &source_absolute, &dest, &mut found_hard_link)?; + let found_hard_link = preserve_hardlinks(hard_links, &source_absolute, &dest)?; if !found_hard_link { match copy_file( progress_bar, diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index bb1ca8679..2e4a4c2b4 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -891,6 +891,16 @@ fn parse_path_args(path_args: &[String], options: &Options) -> CopyResult<(Vec u64 { + #[cfg(unix)] + let result = file_info.inode(); + #[cfg(windows)] + let result = file_info.file_index(); + result +} + +#[cfg(target_os = "redox")] fn preserve_hardlinks( hard_links: &mut Vec<(String, u64)>, source: &std::path::Path, @@ -898,53 +908,47 @@ fn preserve_hardlinks( found_hard_link: &mut bool, ) -> CopyResult<()> { // Redox does not currently support hard links - #[cfg(not(target_os = "redox"))] - { - if !source.is_dir() { - let info = match FileInformation::from_path(source, false) { - Ok(info) => info, - Err(e) => { - return Err(format!("cannot stat {}: {}", source.quote(), e,).into()); - } - }; + Ok(()) +} - #[cfg(unix)] - let inode = info.inode(); - - #[cfg(windows)] - let inode = info.file_index(); - - let nlinks = info.number_of_links(); - - for hard_link in hard_links.iter() { - if hard_link.1 == inode { - // Consider the following files: - // - // * `src/f` - a regular file - // * `src/link` - a hard link to `src/f` - // * `dest/src/f` - a different regular file - // - // In this scenario, if we do `cp -a src/ dest/`, it is - // possible that the order of traversal causes `src/link` - // to get copied first (to `dest/src/link`). In that case, - // in order to make sure `dest/src/link` is a hard link to - // `dest/src/f` and `dest/src/f` has the contents of - // `src/f`, we delete the existing file to allow the hard - // linking. - if file_or_link_exists(dest) && file_or_link_exists(Path::new(&hard_link.0)) { - std::fs::remove_file(dest)?; - } - - std::fs::hard_link(hard_link.0.clone(), dest).unwrap(); - *found_hard_link = true; - } - } - if !(*found_hard_link) && nlinks > 1 { - hard_links.push((dest.to_str().unwrap().to_string(), inode)); +/// Hard link a pair of files if needed _and_ record if this pair is a new hard link. +#[cfg(not(target_os = "redox"))] +fn preserve_hardlinks( + hard_links: &mut Vec<(String, u64)>, + source: &std::path::Path, + dest: &std::path::Path, +) -> CopyResult { + let info = FileInformation::from_path(source, false) + .context(format!("cannot stat {}", source.quote()))?; + let inode = get_inode(&info); + let nlinks = info.number_of_links(); + let mut found_hard_link = false; + for (link, link_inode) in hard_links.iter() { + if *link_inode == inode { + // Consider the following files: + // + // * `src/f` - a regular file + // * `src/link` - a hard link to `src/f` + // * `dest/src/f` - a different regular file + // + // In this scenario, if we do `cp -a src/ dest/`, it is + // possible that the order of traversal causes `src/link` + // to get copied first (to `dest/src/link`). In that case, + // in order to make sure `dest/src/link` is a hard link to + // `dest/src/f` and `dest/src/f` has the contents of + // `src/f`, we delete the existing file to allow the hard + // linking. + if file_or_link_exists(dest) && file_or_link_exists(Path::new(link)) { + std::fs::remove_file(dest)?; } + std::fs::hard_link(link, dest).unwrap(); + found_hard_link = true; } } - Ok(()) + if !found_hard_link && nlinks > 1 { + hard_links.push((dest.to_str().unwrap().to_string(), inode)); + } + Ok(found_hard_link) } /// Copy all `sources` to `target`. Returns an @@ -986,11 +990,12 @@ fn copy(sources: &[Source], target: &TargetSlice, options: &Options) -> CopyResu // FIXME: compare sources by the actual file they point to, not their path. (e.g. dir/file == dir/../dir/file in most cases) show_warning!("source {} specified more than once", source.quote()); } else { - let mut found_hard_link = false; - if preserve_hard_links { + let found_hard_link = if preserve_hard_links && !source.is_dir() { let dest = construct_dest_path(source, target, &target_type, options)?; - preserve_hardlinks(&mut hard_links, source, &dest, &mut found_hard_link)?; - } + preserve_hardlinks(&mut hard_links, source, &dest)? + } else { + false + }; if !found_hard_link { if let Err(error) = copy_source( &progress_bar,