[libc++] Do not rely on the environment to run filesystem tests

Previously, filesystem tests would require LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT
to be present in the environment and to match the value provided when
compiling, as a macro. This has the problem that it only allows for the
filesystem tests to be run on the same machine they are created.

Instead, we create a temporary directory for each test. Technically,
this is tricky to do because we're relying on some of the code that
we're testing to do this. However, there's no other portable way of
creating temporary direcories in C++, so this is difficult to avoid.

Differential Revision: https://reviews.llvm.org/D76731

diff --git a/libcxx/test/CMakeLists.txt b/libcxx/test/CMakeLists.txt
index e63e18a..78a7b88 100644 (file)
--- a/libcxx/test/CMakeLists.txt
+++ b/libcxx/test/CMakeLists.txt
@@ -142,9 +142,6 @@ if (LIBCXX_CONFIGURE_IDE)
   set(STATIC_ROOT ${LIBCXX_SOURCE_DIR}/test/std/input.output/filesystems/Inputs/static_test_env)
   add_definitions(-DLIBCXX_FILESYSTEM_STATIC_TEST_ROOT="${STATIC_ROOT}")
 
-  set(DYNAMIC_ROOT ${LIBCXX_BINARY_DIR}/test/filesystem/Output/dynamic_env)
-  add_definitions(-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT="${DYNAMIC_ROOT}")
-
   set(DYNAMIC_HELPER "python ${LIBCXX_SOURCE_DIR}/test/support/filesystem_dynamic_test_helper.py ")
   add_definitions(-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER="${DYNAMIC_HELPER}")
 

diff --git a/libcxx/test/support/filesystem_dynamic_test_helper.py b/libcxx/test/support/filesystem_dynamic_test_helper.py
index 0789582..c769ac8 100644 (file)
--- a/libcxx/test/support/filesystem_dynamic_test_helper.py
+++ b/libcxx/test/support/filesystem_dynamic_test_helper.py
@@ -8,20 +8,14 @@ assert sys.platform.startswith('linux') or sys.platform.startswith('darwin') \
     or sys.platform.startswith('cygwin') or sys.platform.startswith('freebsd') \
     or sys.platform.startswith('netbsd')
 
-def env_path():
-    ep = os.environ.get('LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT')
-    assert ep is not None
-    ep = os.path.realpath(ep)
-    assert os.path.isdir(ep)
-    return ep
+test_root = None
 
-env_path_global = env_path()
-
-# Make sure we don't try and write outside of env_path.
+# Make sure we don't try and write outside of the test root.
 # All paths used should be sanitized
 def sanitize(p):
+    assert os.path.isdir(test_root)
     p = os.path.realpath(p)
-    if os.path.commonprefix([env_path_global, p]):
+    if os.path.commonprefix([test_root, p]):
         return p
     assert False
 
@@ -44,7 +38,7 @@ def clean_recursive(root_p):
 
 
 def init_test_directory(root_p):
-    root_p = sanitize(root_p)
+    root_p = os.path.realpath(root_p)
     assert not os.path.exists(root_p)
     os.makedirs(root_p)
 
@@ -87,6 +81,7 @@ def create_socket(source):
 
 
 if __name__ == '__main__':
-    command = " ".join(sys.argv[1:])
+    test_root = os.path.realpath(sys.argv[1])
+    command = " ".join(sys.argv[2:])
     eval(command)
     sys.exit(0)

diff --git a/libcxx/test/support/filesystem_test_helper.h b/libcxx/test/support/filesystem_test_helper.h
index 8161820..2c15606 100644 (file)
--- a/libcxx/test/support/filesystem_test_helper.h
+++ b/libcxx/test/support/filesystem_test_helper.h
@@ -103,9 +103,6 @@ static const fs::path RecDirFollowSymlinksIterationList[] = {
 
 #endif // LIBCXX_FILESYSTEM_STATIC_TEST_ROOT
 
-#ifndef LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT
-#warning LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT must be defined
-#else // LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT
 
 #ifndef LIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER
 #error LIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER must be defined
@@ -127,8 +124,16 @@ inline char random_hex_char() {
 
 struct scoped_test_env
 {
-    scoped_test_env() : test_root(random_env_path())
-        { fs_helper_run(fs_make_cmd("init_test_directory", test_root)); }
+    scoped_test_env() : test_root(random_path())
+    {
+        fs_helper_run(fs_make_cmd("init_test_directory", test_root));
+
+        // Ensure that the root_path is fully resolved, i.e. it contains no
+        // symlinks. The filesystem tests depend on that. We do this after
+        // creating the root_path, because `fs::canonical` requires the
+        // path to exist.
+        test_root = fs::canonical(test_root);
+    }
 
     ~scoped_test_env()
         { fs_helper_run(fs_make_cmd("destroy_test_directory", test_root)); }
@@ -229,7 +234,7 @@ struct scoped_test_env
     }
 #endif
 
-    fs::path const test_root;
+    fs::path test_root;
 
 private:
     static std::string unique_path_suffix() {
@@ -243,10 +248,10 @@ private:
 
     // This could potentially introduce a filesystem race with other tests
     // running at the same time, but oh well, it's just test code.
-    static inline fs::path random_env_path() {
-        static const char* env_path = LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT;
-        fs::path p = fs::path(env_path) / unique_path_suffix();
-        assert(p.parent_path() == env_path);
+    static inline fs::path random_path() {
+        fs::path tmp = fs::temp_directory_path();
+        fs::path p = fs::path(tmp) / unique_path_suffix();
+        assert(p.parent_path() == tmp);
         return p;
     }
 
@@ -270,39 +275,15 @@ private:
         return cmd_name + "(" + make_arg(arg1) + ", " + make_arg(arg2) + ")";
     }
 
-    static inline void fs_helper_run(std::string const& raw_cmd) {
-        // check that the fs test root in the environment matches what we were
-        // compiled with.
-        static bool checked = checkDynamicTestRoot();
-        ((void)checked);
+    void fs_helper_run(std::string const& raw_cmd) {
         std::string cmd = LIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER;
+        cmd += " \"" + test_root.native() + "\"";
         cmd += " \"" + raw_cmd + "\"";
         int ret = std::system(cmd.c_str());
         assert(ret == 0);
     }
-
-    static bool checkDynamicTestRoot() {
-        // LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT is expected not to contain symlinks.
-        char* fs_root = std::getenv("LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT");
-        if (!fs_root) {
-            std::printf("ERROR: LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT must be a defined "
-                         "environment variable when running the test.\n");
-            std::abort();
-        }
-        if (std::string(fs_root) != LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT) {
-            std::printf("ERROR: LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT environment variable"
-                        " must have the same value as when the test was compiled.\n");
-            std::printf("   Current Value:  '%s'\n", fs_root);
-            std::printf("   Expected Value: '%s'\n", LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT);
-            std::abort();
-        }
-        return true;
-    }
-
 };
 
-#endif // LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT
-
 // Misc test types
 
 #define MKSTR(Str) {Str, TEST_CONCAT(L, Str), TEST_CONCAT(u, Str), TEST_CONCAT(U, Str)}

diff --git a/libcxx/utils/libcxx/test/config.py b/libcxx/utils/libcxx/test/config.py
index d53bd6c..4babf50 100644 (file)
--- a/libcxx/utils/libcxx/test/config.py
+++ b/libcxx/utils/libcxx/test/config.py
@@ -731,14 +731,6 @@ class Configuration(object):
         assert os.path.isdir(static_env)
         self.cxx.compile_flags += ['-DLIBCXX_FILESYSTEM_STATIC_TEST_ROOT="%s"' % static_env]
 
-        dynamic_env = os.path.join(self.config.test_exec_root,
-                                   'filesystem', 'Output', 'dynamic_env')
-        dynamic_env = os.path.realpath(dynamic_env)
-        if not os.path.isdir(dynamic_env):
-            os.makedirs(dynamic_env)
-        self.cxx.compile_flags += ['-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT="%s"' % dynamic_env]
-        self.exec_env['LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT'] = ("%s" % dynamic_env)
-
         dynamic_helper = os.path.join(self.libcxx_src_root, 'test', 'support',
                                       'filesystem_dynamic_test_helper.py')
         assert os.path.isfile(dynamic_helper)