[PATCH] D85690: [split-file] Fix sys::fs::remove() on Solaris after D83834

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/tools/split-file/split-file.cpp:158-159
 
   // Delete output if it is a file or an empty directory, so that we can create
   // a directory.
   sys::fs::file_status status;
----------------
dblaikie wrote:
> Perhaps it should (at least, this was my original idea/suggestion when I proposed this behavior - not sure if I was clear/specified that, though) delete the contents even if it is non-empty. This would help make tests more hermetic/less likely to have the weird state of "oh, I accidentally made a test that changes behavior when run a second time/with files left around" (we see this sometimes and need to explicitly rm things in the test - sometimes then cleaning up the test later once the bad files are no longer present/problematic)
I'd like to avoid recursive remove, which can easily cause damage to the system (e.g. `split-file %s .` will delete the current directory)

I have thought about hermetic issues, but in the end I don't think leaving files with split-file can more easily trap the user.

If the user wants to remove the directory, make it clear with an explicit `rm -fr %t`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85690/new/

https://reviews.llvm.org/D85690