extra::tempfile: replace mkdtemp with an RAII wrapper (Closes #9763) #9802
Conversation
|
Not sure about the right interface for the whole RAII thing, I just wrote something up as a first draft and to see how it would work at call sites. I took the name of the |
|
tagging @catamorphism |
|
NIce job! This is pretty much exactly what I would expect from an RAII-style tempdir. The only request that I would have for this is some tests ensuring that the directory is indeed removed upon running out of scope and on a failure. Could you add a task or two to ensure that happens? Otherwise r+ from me. |
|
Added some tests for the destructor behavior, also involving tasks. |
|
r+ from me, nice job, and thanks! Needs a rebase though :( |
|
Working on ( |
this incidentally stops `make check` from leaving directories in `/tmp`
this incidentally stops `make check` from leaving directories in `/tmp` (Closes #9764)
|
@ben0x539 I just rebased one of my branches off this, and I wanted to note that it's a great change. I don't know why I/we didn't do it this way in the first place (well, maybe destructors didn't work yet then :-) Kudos! |
| /// Unwrap the wrapped `std::path::Path` from the `TempDir` wrapper. | ||
| /// This discards the wrapper so that the automatic deletion of the | ||
| /// temporary directory is prevented. | ||
| pub fn unwrap(self) -> Path { |
There was a problem hiding this comment.
Out of curiosity, why is self passed by value, instead of by reference?
There was a problem hiding this comment.
@hatahet I figured it was easier to have unwrap discard the TempDir rather than keeping the "disarmed" TempDir around. So it is passed by value so that it is consumed by call to unwrap and you get compile-time errors if you try to use your TempDir object after unwrapping the path from it, instead of runtime errors when it turns out the Option value has been set to None.
I hope that this way it is easier to use than if for example TempDir just had a Path field and a delete_on_drop: bool field that could be set to false instead of using unwrap, and it felt rather usable when I was working on the tests. Of course it might be different in "real" code that wants to keep temporary directories around for longer, and maybe there is a better way. :)
this incidentally stops
make checkfrom leaving directories in/tmp(Closes #9764)