[stdlib] Implementation of rmtree

[artemiogr97]

#2430 must be merged first

[abduld]

Pretty cool. Please make sure about the Int types though

[laszlokindrat]

Thanks for the patch! This could be very nice to have, but there are a bunch of library design questions, as well as implementation details that we should iron out. My main question at this point: do we actually want this right now, given that Python interop already allows one to do this?

While we work on these, I have a couple of suggestions that could help moving this forward: 1) we could use an os.makedirs with an exist_ok argument; 2) we could explore (maybe in a design doc) if we could start a tempfile module to build some testing utilities. @artemiogr97 Would you be interested in working on either of these?

CC: @ConnorGray @rparolin @JoeLoser

[laszlokindrat]

@JoeLoser @ConnorGray Do you have any opinions if we should make ignore_errors keyword-only? Python doesn't do this, but it's not clear if that's just a result of backward compatibility. Otherwise, I always feel a bit strange about flags that can be specified positionally.

[laszlokindrat]

Idea: Once we have parametric raising, we could introduce overloads that have this as a parameter. In the meanwhile, would it be beneficial to have a separate function that ignores errors and never raises?

[artemiogr97]

I think that could make sense for now

[laszlokindrat]

I'm not so sure about this. It would be nice if we had some tempfile-like utilities to create these ephemeral paths. At the same time, tempfile should probably depend on shutil, not the other way around.
Idea 1: Maybe we could bring up some basic tempfile utilities so that we can at least have a platform-independent temporary path.
Idea 2: we could just use python interop in these tests while we get there.

[artemiogr97]

I guess python interop is the best option for now, one idea to prevent testing shutil with a potential tempfile module (which logically would depend on shutil) would be to use something like subprocess.run and call something like rm -rf my_dir, but it does not seem like a short-term solution

[laszlokindrat]

Testing these kinds of functions can actually be tricky: what if the test fails for some reason and the directory isn't deleted? The next time the test is run, mkdir can fail. One way to fix this is by doing this in a context manager, like tempfile.TemporaryDirectory. In lieu of that, some manual cleanup logic could also work.

[artemiogr97]

yes, it's tricky, but doing some manual cleanup is not that simple in this case, what if what is failing is rmdir/unlink ? obviously rmtree is relying on those functions so what else could be done?

we have the same "issue" in test_remove and test_mkdir_and_rmdir

[artemiogr97]

My main question at this point: do we actually want this right now, given that Python interop already allows one to do this?

I agree that maybe it is a bit soon to start implementing a new module such as shutil (or an equivalent).
At the beginning I wanted to implement #2018 but then I soon realized that a lot of the basic functionalities such as os.remove/os.unlink, os.mkdir, os.rmdir where missing so I started implementing them to finish #2352, and still there is a lot of stuff missing os.makedirs, os.join, os.abspath, ...

While we work on these, I have a couple of suggestions that could help moving this forward: 1) we could use an os.makedirs with an exist_ok argument; 2) we could explore (maybe in a design doc) if we could start a tempfile module to build some testing utilities. @artemiogr97 Would you be interested in working on either of these?

for your point 1, yes, I would like to work on that, everything that could be done to "complete" the os module makes a lot of sense, but I have some doubts:

• should I care about windows compatibility at some point?
• should we start creating the equivalents of posix/nt modules to start handling os specific stuff?
• if windows is to be taken into account it is going to be hard to make sure that everything is ok since mojo is not available in windows yet

for 2 I'm not sure what there is to explore/discuss about a tempfile module, the equivalent python module just contains a couple of functions/classes but nothing really out of the ordinary, most of the functionality is already in the PR that I mentioned above but it depends on rmtree for now, so if needed I could modify it to remove rmtree and use it as a testing module for now

[laszlokindrat]

While we work on these, I have a couple of suggestions that could help moving this forward: 1) we could use an os.makedirs with an exist_ok argument; 2) we could explore (maybe in a design doc) if we could start a tempfile module to build some testing utilities. @artemiogr97 Would you be interested in working on either of these?

for your point 1, yes, I would like to work on that, everything that could be done to "complete" the os module makes a lot of sense, ...

That's awesome, thank you! Please ping me if you have PRs for these, I'm happy to review. I'm assuming these os utilities shouldn't depend on shutil, right (i.e. the dependency is the other way around)?

... but I have some doubts:

• should I care about windows compatibility at some point?

For now, don't worry about Windows. The existing code in os would already break on native Windows. But during the implementation, feel free to leave TODOs in places where you think some special handling for Windows might be needed.

• should we start creating the equivalents of posix/nt modules to start handling os specific stuff?

I would be lazy about this stuff. If the complexity of handling multiple platforms within os/sys (as well as user code) would grow to the point where it makes sense, we can introduce these, but I don't think we're there yet.

• if windows is to be taken into account it is going to be hard to make sure that everything is ok since mojo is not available in windows yet

Yep, don't worry about this for now.

for 2 I'm not sure what there is to explore/discuss about a tempfile module, the equivalent python module just contains a couple of functions/classes but nothing really out of the ordinary, most of the functionality is already in the PR that I mentioned above but it depends on rmtree for now, so if needed I could modify it to remove rmtree and use it as a testing module for now

I see now that you already did quite a bit of work on tempfile, which is awesome. Let's discuss that on the other PR.