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: handle edge cases when dest is a symlink #2610

Merged
merged 3 commits into from
Jan 10, 2022

Conversation

miDeb
Copy link
Contributor

@miDeb miDeb commented Aug 28, 2021

  • Refactor identical file detection into uucore
  • Use refactored file identity detection in cat (fixes Cat Windows 10 does not work  #2601)
  • Fix edge cases in cp when the destination is a symlink.
    I also added some FIXME comments to cp about issues I found but didn't fix in this PR.

@sylvestre
Copy link
Sponsor Contributor

Sorry, it is conflicting, could you please fix that? thanks

- Fail if dest is a dangling symlink
- Fail if dest is a symlink that was previously created by the same
invocation of cp
@miDeb
Copy link
Contributor Author

miDeb commented Nov 1, 2021

Should be fixed now! Sorry for being so slow, but since school started again I have much less time.

@tertsdiepraam
Copy link
Member

The tests doesn't seem to have been run. Has the "rerun checks" button disappeared from GitHub? @miDeb could you check whether you can trigger them?

@miDeb
Copy link
Contributor Author

miDeb commented Dec 5, 2021

I also can't find it. I'll try closing and reopening the PR, I think that should trigger a rerun.

@miDeb miDeb closed this Dec 5, 2021
@miDeb miDeb reopened this Dec 5, 2021
@miDeb
Copy link
Contributor Author

miDeb commented Dec 8, 2021

A test on windows seems to be failing because of a bug in sort; I'll fix that in another PR.

@miDeb
Copy link
Contributor Author

miDeb commented Dec 29, 2021

Not sure why all CI jobs got cancelled, I'll rerun them once more...

@tertsdiepraam
Copy link
Member

Interesting, GitHub retriggered the CI because I made another PR with the same commits. I'll close that one and merge this, because it's all green.

@@ -32,6 +35,114 @@ macro_rules! has {
};
}

/// Information to uniquely identify a file
pub struct FileInformation(
Copy link
Member

Choose a reason for hiding this comment

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

I just wanted to comment that this API is really nice :)

@tertsdiepraam tertsdiepraam merged commit f60c36f into uutils:master Jan 10, 2022
@miDeb
Copy link
Contributor Author

miDeb commented Jan 11, 2022

Thanks!

@miDeb miDeb deleted the cp/abuse branch January 11, 2022 15:07
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.

Cat Windows 10 does not work
3 participants