-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
implement posix delete on windows, if supported #4318
base: v1.x
Are you sure you want to change the base?
Conversation
90d014f
to
9e5bbac
Compare
9e5bbac
to
d29d4db
Compare
563917b
to
759217e
Compare
LGTM, but will leave open for a bit in case anyone else wants to comment |
Of the two tests that are failing, I don't see a relation to this PR and: This one is failing on the tip of v1.x #4325
I take it this one might be flaky?
|
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
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.
Only a partial review due to time constraints.
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.
LGTM insofar I'm able to judge that.
I note there's a TOCTOU issue in that function (checking file type before applying the operation) but that doesn't look like it's new.
Gentle bump on this. |
Fixes #3839
Implements posix delete for files and dirs, with fallback to the old method if not supported (e.g. Fat32)