-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Don't let remove_dir_all
recursively remove a symlink
#31468
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
} else { | ||
remove_dir_all_recursive(path) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic looks pretty similar between here and windows, perhaps it could be lifted to std::fs
and leave the recursive bits to the platform-specific modules?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Windows version is similar but a bit tricky. It uses rmdir()
so it can only remove directory symlinks and not file symlinks. Unix just uses unlink()
. But I don't mind if we make the distinction here or in std::fs
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right, yeah, good point!
Looks good to me, thanks @pitdicker! |
⌛ Testing commit d47036c with merge 227f399... |
O, I am very sorry. I only tested with symlinking permissions, not also without. The build will fail |
See #29412