Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cp: fix the result of inodes are not the same when preserve links is flagged #5064

Merged
merged 57 commits into from
Sep 24, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
5a94557
fix-issue-5031
tommady Jul 10, 2023
ea8794c
Merge branch 'main' into fix-issue-5031
tommady Jul 10, 2023
31c1ebf
fix spelling
tommady Jul 10, 2023
1dfa23f
add explanation
tommady Jul 11, 2023
cde4af8
Merge branch 'main' into fix-issue-5031
tommady Jul 12, 2023
efbdf51
adding testcase for 5031
tommady Jul 13, 2023
a3803af
adding new testcase for 5031
tommady Jul 13, 2023
37e3a7c
Fix a typo
sylvestre Jul 13, 2023
0cd08e7
add testcase 3
tommady Jul 13, 2023
fa4ea63
Merge branch 'main' into fix-issue-5031
tommady Jul 13, 2023
930137c
remove unused code
tommady Jul 13, 2023
86a96fa
Merge branch 'main' into fix-issue-5031
tommady Jul 17, 2023
8820a04
by pass case 3 unit test on android
tommady Jul 19, 2023
bc9a054
Merge branch 'main' into fix-issue-5031
tommady Jul 19, 2023
b94be4c
by pass case 1 and 2 unit tests on android
tommady Jul 19, 2023
5ede19a
Merge branch 'main' into fix-issue-5031
tommady Jul 19, 2023
94fb212
add 5031 ut case 4 and fix it
tommady Jul 25, 2023
f49b86b
add 5031 ut case 5
tommady Jul 25, 2023
28278e2
fix linter issues
tommady Jul 25, 2023
bdaa394
Merge branch 'main' into fix-issue-5031
tommady Jul 25, 2023
395cee4
fix unit test failed
tommady Jul 25, 2023
48e7f2e
Merge branch 'main' into fix-issue-5031
tommady Jul 27, 2023
87bae22
align preserve links unit tests with gnu cp tests
tommady Aug 2, 2023
bf87a44
correct preserve links unit test case 5
tommady Aug 2, 2023
340df6b
using single copied FileInformation map to do the job
tommady Aug 3, 2023
35000cf
Merge branch 'fix-issue-5031' of github.com:tommady/coreutils into fi…
tommady Aug 3, 2023
cb65994
Merge branch 'main' into fix-issue-5031
tommady Aug 3, 2023
5bf83d0
remove unused clippy too many arg allowance
tommady Aug 3, 2023
26b5727
adding some comments to explain the copied_file hashmap and the hard_…
tommady Aug 3, 2023
36487a9
fix ERROR: Unknown word (hashmap's)
tommady Aug 3, 2023
d77e2a5
fix ERROR: Unknown word (files's)
tommady Aug 3, 2023
49edf6b
Merge branch 'main' into fix-issue-5031
tommady Aug 10, 2023
ba69841
let the unit tests run on android and freebsd
tommady Aug 11, 2023
57718f8
Merge branch 'main' into fix-issue-5031
tommady Aug 11, 2023
2b7e3c4
let the unit tests run on unix platform while the inode assertion
tommady Aug 11, 2023
844d99b
Merge branch 'main' into fix-issue-5031
tommady Aug 11, 2023
3554e62
let MetadataExt can be used while testing on freebsd platform
tommady Aug 12, 2023
2743c11
Merge branch 'main' into fix-issue-5031
tommady Aug 12, 2023
c592a0a
address comments
tommady Aug 13, 2023
f65fd74
Merge branch 'main' into fix-issue-5031
tommady Aug 13, 2023
8e9c2a3
Merge branch 'main' into fix-issue-5031
tommady Aug 14, 2023
be274d3
Merge branch 'main' into fix-issue-5031
tommady Aug 15, 2023
926db5e
Merge branch 'main' into fix-issue-5031
tommady Aug 20, 2023
5e59458
address comment move the copied_files hashmap insertion from each cop…
tommady Aug 22, 2023
abb2c0f
restore the dest variable back to it's original place of definition
tommady Aug 22, 2023
39ad651
fix clippy linter issue
tommady Aug 22, 2023
4eb2a27
Merge branch 'main' into fix-issue-5031
tommady Aug 22, 2023
04f4abc
address comment, merge preserve_hardlinks function
tommady Aug 27, 2023
4af43c4
Merge branch 'fix-issue-5031' of github.com:tommady/coreutils into fi…
tommady Aug 27, 2023
1377e6f
Merge branch 'main' into fix-issue-5031
tommady Aug 27, 2023
8dc3deb
fix clippy linter issue
tommady Aug 27, 2023
8adcafb
Merge branch 'fix-issue-5031' of github.com:tommady/coreutils into fi…
tommady Aug 27, 2023
0abb096
address comments
tommady Aug 29, 2023
fd4b7cf
address comments
tommady Aug 30, 2023
f81145e
address comments
tommady Aug 30, 2023
d7d58b0
Merge branch 'main' into fix-issue-5031
tommady Sep 6, 2023
5827f91
Merge branch 'main' into fix-issue-5031
tommady Sep 14, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 46 additions & 5 deletions src/uu/cp/src/cp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

use quick_error::quick_error;
use std::borrow::Cow;
use std::collections::HashSet;
use std::collections::{HashMap, HashSet};
use std::env;
#[cfg(not(windows))]
use std::ffi::CString;
Expand Down Expand Up @@ -1134,11 +1134,13 @@
verify_target_type(target, &target_type)?;

let preserve_hard_links = options.preserve_hard_links();
let cli_dereference = options.cli_dereference;

let mut hard_links: Vec<(String, u64)> = vec![];

let mut non_fatal_errors = false;
let mut seen_sources = HashSet::with_capacity(sources.len());
let mut seen_sources = HashMap::with_capacity(sources.len());
let mut should_hard_linked_files = HashMap::with_capacity(sources.len());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seen_sources is a name I understand immediately, but I don't really understand should_hard_linked_files? Can we give it a more descriptive name?

let mut symlinked_files = HashSet::new();

let progress_bar = if options.progress_bar {
Expand All @@ -1157,12 +1159,51 @@
};

for source in sources.iter() {
if seen_sources.contains(source) {
let dest = construct_dest_path(source, target, &target_type, options)?;
if seen_sources.contains_key(source) && !should_hard_linked_files.contains_key(source) {
// 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 if let Some(new_source) = should_hard_linked_files.get(source) {
std::fs::hard_link(new_source, &dest)?;
should_hard_linked_files.remove(source);
} else if preserve_hard_links && source.is_symlink() && cli_dereference {
// issue 5031 case
// touch a && ln -s a b && mkdir c
// cp --preserve=links -R -H a b c

let mut original_source = source.read_link()?;
while original_source.is_symlink() {
original_source = original_source.read_link()?;
}
// unwrap it because the read_link method is successed

Check failure on line 1178 in src/uu/cp/src/cp.rs

View workflow job for this annotation

GitHub Actions / Style/spelling (ubuntu-latest, feat_os_unix)

ERROR: Unknown word (successed) (file:'src/uu/cp/src/cp.rs', line:1178)
sylvestre marked this conversation as resolved.
Show resolved Hide resolved
// so file_name won't be failed
original_source = original_source.file_name().unwrap().into();

match seen_sources.get(&original_source) {
Some(new_source) => {
std::fs::hard_link(new_source, &dest)?;
}
None => {
// TODO: GNU cp can deal with this case
// for example:
// touch a && ln -s a b && mkdir c
// cp --preserve=links -R -H b a c
if let Err(error) = copy_source(
&progress_bar,
source,
target,
&target_type,
options,
&mut symlinked_files,
) {
show_error_if_needed(&error);
non_fatal_errors = true;

Check warning on line 1200 in src/uu/cp/src/cp.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/cp/src/cp.rs#L1199-L1200

Added lines #L1199 - L1200 were not covered by tests
}
should_hard_linked_files.insert(original_source, dest.clone());
}
}
} else {
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)?
} else {
false
Expand All @@ -1180,8 +1221,8 @@
non_fatal_errors = true;
}
}
seen_sources.insert(source);
}
seen_sources.insert(source, dest);
}
if non_fatal_errors {
Err(Error::NotAllFilesCopied)
Expand Down
66 changes: 66 additions & 0 deletions tests/by-util/test_cp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1349,6 +1349,72 @@ fn test_cp_preserve_xattr_fails_on_android() {
.fails();
}

#[test]
fn test_cp_issue_5031_case_1() {
let (at, mut ucmd) = at_and_ucmd!();

at.touch("a");
at.symlink_file("a", "b");
at.mkdir("c");

assert!(at.file_exists("a"));
assert!(at.symlink_exists("b"));
assert!(at.dir_exists("c"));

ucmd.arg("--preserve=links")
.arg("-R")
.arg("-H")
.arg("a")
.arg("b")
.arg("c")
.succeeds();

#[cfg(not(any(target_os = "freebsd", target_os = "macos")))]
{
assert!(at.dir_exists("c"));
assert!(at.plus("c").join("a").exists());
assert!(at.plus("c").join("b").exists());

let metadata_a = std_fs::metadata(at.subdir.join("c").join("a")).unwrap();
let metadata_b = std_fs::metadata(at.subdir.join("c").join("b")).unwrap();

assert_eq!(metadata_a.ino(), metadata_b.ino());
}
}

#[test]
fn test_cp_issue_5031_case_2() {
let (at, mut ucmd) = at_and_ucmd!();

at.touch("a");
at.symlink_file("a", "b");
at.mkdir("c");

assert!(at.file_exists("a"));
assert!(at.symlink_exists("b"));
assert!(at.dir_exists("c"));

ucmd.arg("--preserve=links")
.arg("-R")
.arg("-H")
.arg("b")
.arg("a")
.arg("c")
.succeeds();

#[cfg(not(any(target_os = "freebsd", target_os = "macos")))]
{
assert!(at.dir_exists("c"));
assert!(at.plus("c").join("a").exists());
assert!(at.plus("c").join("b").exists());

let metadata_a = std_fs::metadata(at.subdir.join("c").join("a")).unwrap();
let metadata_b = std_fs::metadata(at.subdir.join("c").join("b")).unwrap();

assert_eq!(metadata_a.ino(), metadata_b.ino());
}
}

#[test]
// For now, disable the test on Windows. Symlinks aren't well support on Windows.
// It works on Unix for now and it works locally when run from a powershell
Expand Down
Loading