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

implement rm -rf in terms of pathlib #4061

Closed
RonnyPfannschmidt opened this issue Oct 2, 2018 · 7 comments
Closed

implement rm -rf in terms of pathlib #4061

RonnyPfannschmidt opened this issue Oct 2, 2018 · 7 comments
Assignees
Labels
plugin: tmpdir related to the tmpdir builtin plugin type: enhancement new feature or API change, should be merged into features branch

Comments

@RonnyPfannschmidt
Copy link
Member

followup to #3988 (comment)

as it turns out shutil.rmtree is unable to be harsh
its fine if we aren't for a while, but we need eventual parity

@nicoddemus nicoddemus added type: enhancement new feature or API change, should be merged into features branch plugin: tmpdir related to the tmpdir builtin plugin labels Oct 2, 2018
@Zac-HD
Copy link
Member

Zac-HD commented Oct 19, 2018

That is, implement the following using pathlib rather than shutil:

def rmtree(path, force=False):
if force:
# ignore_errors leaves dead folders around
# python needs a rm -rf as a followup
# the trick with _shutil_rmtree_remove_writable is unreliable
shutil.rmtree(str(path), ignore_errors=True)
else:
shutil.rmtree(str(path))

@RonnyPfannschmidt
Copy link
Member Author

also ref #4222

@gnikonorov gnikonorov self-assigned this Jun 30, 2020
@gnikonorov
Copy link
Member

I'm going to take a look at this one as I'm freeing up a bit.
@RonnyPfannschmidt - can you elaborate what you mean by shutil.rmtree is unable to be harsh? If I understand correctly, all we need to do is what @Zac-HD has suggested but I am curious about your comment

@RonnyPfannschmidt
Copy link
Member Author

@gnikonorov shutil.rmtree doesn't try as hard as py.path.local does

It's basically a rm -r and we need the -f on top as well

@gnikonorov
Copy link
Member

Thanks for confirming @RonnyPfannschmidt

@RonnyPfannschmidt
Copy link
Member Author

@nicoddemus i think we can consider this fixed with #5588 merged?

@nicoddemus
Copy link
Member

Yes, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin: tmpdir related to the tmpdir builtin plugin type: enhancement new feature or API change, should be merged into features branch
Projects
None yet
Development

No branches or pull requests

4 participants