[libcxx-commits] [PATCH] D98703: [libcxx] [test] Readd randomness to temporary directory names

Arthur O'Dwyer via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Tue Mar 16 07:04:23 PDT 2021


Quuxplusone added inline comments.


================
Comment at: libcxx/test/support/filesystem_test_helper.h:324-331
+#if !defined(_LIBCPP_HAS_NO_RANDOM_DEVICE)
+        base += "-" + random_utils::random_string(4);
+#endif
         int i = 0;
         fs::path p = tmp / (base + "-static_env." + std::to_string(i));
         while (utils::exists(p.string())) {
             p = tmp / (base + "-static_env." + std::to_string(++i));
----------------
How about (suggested edit)? My logic is that if this piece of code ever produces a collision EVER, then we need a programmer to revisit this piece of code; there's no sense in trying to make the code "work around" a collision, because the whole point is that collisions shouldn't happen.

However, I still don't understand why there's a collision here in the first place. You say "As some multiple test subdirectories contain tests with the same file names..." but shouldn't we just be using the entire (unique) path, then? Why (and where) are we throwing away the parts of the path that aren't the filename?

Also, is it possible that we could still have collisions when the same llvm-lit process spawns two copies of the same test (does it ever do that? idk) or when two different llvm-lit processes spawn the same test at the same time (do we support running two llvm-lit runs simultaneously? idk)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98703



More information about the libcxx-commits mailing list