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

[MaskRay]

Summary

where stdio.h ::remove() may set errno to EEXIST instead of ENOTEMPTY.

POSIX.1-2017 allows EEXIST for unlink() (which is called by remove()):

[EEXIST] or [ENOTEMPTY]
The flag parameter has the AT_REMOVEDIR bit set and the path argument names a directory that is not an empty directory, or there are hard links to the directory other than dot or a single entry in dot-dot.

[dblaikie]

llvm/tools/split-file/split-file.cpp

158–159

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)

[MaskRay]

llvm/tools/split-file/split-file.cpp

158–159

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.

[dblaikie]

llvm/tools/split-file/split-file.cpp

158–159

Sure - though given the fairly narrow usage of the tool, predominantly in LLVM test cases where the directory given is a unique subdirectory for just this test files tests, it seemed like it'd be a chance to make tests more reliable by not having lots of leftover files. (I wouldn't be totally averse to that directory deleting having some heuristic about there not being too many files in the directory - since it's only expected to be cleaning up a small test directory).

I don't think split-file will make the situation worse - but seemed like an opportunity for it to make the situation better.

[MaskRay]

llvm/tools/split-file/split-file.cpp

158–159

I'd still want to avoid recursive removal. There is not many precedent for such feature for utilities.

I actually run tests in the local source tree a lot. When I type rm -r, I am very cautious about the next characters I will type and I think many others will do as well. For other utilities like split-file, they might be less careful. I don't want split-file %s ~ or split-file %s / causes disastrous effect.

[dblaikie]

llvm/tools/split-file/split-file.cpp

158–159

Fair enough

[ro]

I've tested the patched version of split-file on sparcv9-sun-solaris2.11 running the tools/llvm-strings and tools/split-file tests which had failed before. They now PASS reliably.

So LGTM. Thanks.

llvm/tools/split-file/split-file.cpp

158–159

FWIW I agree: in the Unix spirit of

one tool does one job only and does it right

split-file has no business whatsoever in doing recurisive cleanup like this. It's way too dangerous.

llvm/tools/split-file/split-file.cpp

if (input.empty())
     fatal("", "input filename is not specified");
   if (output.empty())
     fatal("", "output directory is not specified");
   ErrorOr<std::unique_ptr<MemoryBuffer>> bufferOrErr =
       MemoryBuffer::getFileOrSTDIN(input);
   if (std::error_code ec = bufferOrErr.getError())
     fatal(input, ec.message());
 
   // Delete output if it is a file or an empty directory, so that we can create
   // a directory.
   sys::fs::file_status status;
   if (std::error_code ec = sys::fs::status(output, status))
     if (ec.value() != static_cast<int>(std::errc::no_such_file_or_directory))
       fatal(output, ec.message());
   if (status.type() != sys::fs::file_type::file_not_found &&
       status.type() != sys::fs::file_type::directory_file &&
       status.type() != sys::fs::file_type::regular_file)
     fatal(output, "output cannot be a special file");
   if (std::error_code ec = sys::fs::remove(output, /*IgnoreNonExisting=*/true))
-    if (ec.value() != static_cast<int>(std::errc::directory_not_empty))
+    if (ec.value() != static_cast<int>(std::errc::directory_not_empty) &&
+        ec.value() != static_cast<int>(std::errc::file_exists))
       fatal(output, ec.message());
   return handle(**bufferOrErr, input);
 }