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: correct error message on copying dir to itself #3981

Merged
merged 1 commit into from
Oct 6, 2022

Conversation

jfinkels
Copy link
Collaborator

Correct the error message produced when attempting to copy a directory into itself with cp. Before this commit, the error message was

$ cp -R d d
cp: cannot copy a directory, 'd', into itself, 'd'

After this commit, the error message is

$ cp -R d d
cp: cannot copy a directory, 'd', into itself, 'd/d'

This makes GNU test suite file tests/cp/into-self.sh pass.

@jfinkels jfinkels force-pushed the cp-directory-into-itself branch from c6797e2 to 7e0399e Compare September 26, 2022 00:55
Copy link
Member

@tertsdiepraam tertsdiepraam left a comment

Choose a reason for hiding this comment

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

I think this is good enough to merge, but if you're up for a challenge, I found a case where the output still differs from GNU :)

❯ mkdir -p foo/bar/baz

❯ cp -r foo/bar foo/bar/baz
cp: cannot copy a directory, 'foo/bar', into itself, 'foo/bar/baz/bar'

❯ cargo run -- cp -r foo/bar foo/bar/baz
cp: cannot copy a directory, 'foo/bar', into itself, 'foo/bar/foo/bar/baz'

@jfinkels
Copy link
Collaborator Author

Another good catch. I added a fixup! commit that corrects the behavior to satisfy both the simple and nested test cases

@sylvestre
Copy link
Contributor

I guess you saw



---- test_cp::test_copy_nested_directory_to_itself_disallowed stdout ----
current_directory_resolved: 
mkdir: C:\Users\RUNNER~1\AppData\Local\Temp\.tmpeJUUl8\a
mkdir: C:\Users\RUNNER~1\AppData\Local\Temp\.tmpeJUUl8\a/b
mkdir: C:\Users\RUNNER~1\AppData\Local\Temp\.tmpeJUUl8\a/b/c
run: D:\a\coreutils\coreutils\target\debug\coreutils.exe cp -R a/b a/b/c
thread 'test_cp::test_copy_nested_directory_to_itself_disallowed' panicked at 'assertion failed: `(left == right)`

Diff < left / right > :
<cp: cannot copy a directory, 'a/b', into itself, 'a/b/c\b'
>cp: cannot copy a directory, 'a\b', into itself, 'a\b\c\b'

', tests\common\util.rs:328:9


failures:
    test_cp::test_copy_nested_directory_to_itself_disallowed

@sylvestre sylvestre force-pushed the cp-directory-into-itself branch from 2ae9cdb to 986135d Compare October 5, 2022 11:30
Correct the error message produced when attempting to copy a directory
into itself with `cp`. Before this commit, the error message was

    $ cp -R d d
    cp: cannot copy a directory, 'd', into itself, 'd'

After this commit, the error message is

    $ cp -R d d
    cp: cannot copy a directory, 'd', into itself, 'd/d'
@jfinkels jfinkels force-pushed the cp-directory-into-itself branch from 986135d to 8bfd96f Compare October 6, 2022 01:38
@sylvestre
Copy link
Contributor

Cool:
Congrats! The gnu test tests/cp/into-self is no longer failing!

@sylvestre sylvestre merged commit 6b36a26 into uutils:main Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants