[libcxx-commits] [PATCH] D98960: [libcxx] [test] Allow C:\System Volume Information to be missing

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone accepted this revision as: Quuxplusone.
Quuxplusone added a comment.

Still looks plausible to me, mod comment.



================
Comment at: libcxx/test/support/filesystem_test_helper.h:691-698
+    // Check that it indeed is inaccessible as expected
+    (void)fs::exists(ent, ec);
+    if (!ec) {
+      fprintf(stderr, "The expected inaccessible directory \"%s\" was found "
+                      "but seems to be accessible, skipping tests "
+                      "regarding it\n", dir.string().c_str());
+      return fs::path();
----------------
This codepath actually confuses me. It's new (appears on the right-hand side but not the left-hand side of the diff), and it seems to be checking that `ent.exists()` (line 685) and yet `!fs::exists(ent)` (line 692)? This could well be consistent with reality — I mean, I'd expect std::filesystem to do a terrible/weird job of handling visible-but-inaccessible paths — but could you please just manually triple-check that `GetWindowsInaccessibleDir()` does still return a non-empty path //sometimes//, and hasn't been completely nerfed by this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98960