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

[ldionne]

Summary

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.

[broadwaylamb]

This is awesome. I'm working on running filesystem tests on a remote machine right now, this should be of much help.

[EricWF]

libcxx/test/support/filesystem_test_helper.h

232

Why did you drop the const?

[ldionne]

libcxx/test/support/filesystem_test_helper.h

232

Because of test_root = fs::canonical(test_root); above in the constructor. Note that I can't set that in the initializer list, because fs::canonical requires the test_root to exist, which is set up in the constructor body.

[EricWF]

libcxx/test/support/filesystem_test_helper.h

232

Alright. I'm OK with this.

I would rather it be const, but *shrug*.

libcxx/test/CMakeLists.txt

if (LIBCXX_CONFIGURE_IDE)
   file(GLOB_RECURSE LIBCXX_HEADERS ${CMAKE_CURRENT_SOURCE_DIR}/../include/*)
   add_executable(libcxx_test_objects EXCLUDE_FROM_ALL
           ${LIBCXX_TESTS} ${LIBCXX_TEST_HEADERS} ${LIBCXX_HEADERS})
   add_dependencies(libcxx_test_objects cxx)
 
   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}")
 
   split_list(LIBCXX_COMPILE_FLAGS)
   split_list(LIBCXX_LINK_FLAGS)
 
   set_target_properties(libcxx_test_objects
           PROPERTIES
             COMPILE_FLAGS "${LIBCXX_COMPILE_FLAGS}"
             LINK_FLAGS "${LIBCXX_LINK_FLAGS}"
             EXCLUDE_FROM_ALL ON
   )
 endif()

libcxx/test/support/filesystem_dynamic_test_helper.py

 import sys
 import os
 import socket
 import stat
 
 # Ensure that this is being run on a specific platform
 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
 
 """
 Some of the tests restrict permissions to induce failures.
 Before we delete the test environment, we have to walk it and re-raise the
 permissions.
 """
 def clean_recursive(root_p):
     if not os.path.islink(root_p):
         os.chmod(root_p, 0o777)
     for ent in os.listdir(root_p):
         p = os.path.join(root_p, ent)
         if os.path.islink(p) or not os.path.isdir(p):
             os.remove(p)
         else:
             assert os.path.isdir(p)
             clean_recursive(p)
             os.rmdir(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)
 
 
 def destroy_test_directory(root_p):
     root_p = sanitize(root_p)
     clean_recursive(root_p)
     os.rmdir(root_p)
def create_socket(source):
     # AF_UNIX sockets may have very limited path length, so split it
     # into chdir call (with technically unlimited length) followed
     # by bind() relative to the directory
     os.chdir(os.path.dirname(sanitized_source))
     sock.bind(os.path.basename(sanitized_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)

libcxx/test/support/filesystem_test_helper.h

static const fs::path RecDirFollowSymlinksIterationList[] = {
     makePath("dir1/dir2/symlink_to_dir3"),
     makePath("dir1/dir2/symlink_to_dir3/file5"),
 };
 
 } // namespace StaticEnv
 
 #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
 #endif
 
 namespace random_utils {
 inline char to_hex(int ch) {
   return ch < 10 ? static_cast<char>('0' + ch)
                  : static_cast<char>('a' + (ch - 10));
 }
 
 inline char random_hex_char() {
   static std::mt19937 rd{std::random_device{}()};
   static std::uniform_int_distribution<int> mrand{0, 15};
   return to_hex(mrand(rd));
 }
 
 } // namespace random_utils
 
 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)); }
 
     scoped_test_env(scoped_test_env const &) = delete;
     scoped_test_env & operator=(scoped_test_env const &) = delete;
 
     fs::path make_env_path(std::string p) { return sanitize_path(p); }

 #if !defined(__FreeBSD__) && !defined(__APPLE__)
     std::string create_socket(std::string file) {
         file = sanitize_path(std::move(file));
         fs_helper_run(fs_make_cmd("create_socket", file));
         return file;
     }
 #endif
 
-    fs::path const test_root;
+    fs::path test_root;
 
 private:
     static std::string unique_path_suffix() {
         std::string model = "test.%%%%%%";
         for (auto & ch :  model) {
           if (ch == '%')
             ch = random_utils::random_hex_char();
         }
         return model;
     }
 
     // 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;
     }
 
     static inline std::string make_arg(std::string const& arg) {
         return "'" + arg + "'";
     }
 
     static inline std::string make_arg(std::size_t arg) {
         return std::to_string(arg);
     }
 
     template <class T>
     static inline std::string
     fs_make_cmd(std::string const& cmd_name, T const& arg) {
         return cmd_name + "(" + make_arg(arg) + ")";
     }
 
     template <class T, class U>
     static inline std::string
     fs_make_cmd(std::string const& cmd_name, T const& arg1, U const& arg2) {
         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)}
 
 struct MultiStringType {
   const char* s;
   const wchar_t* w;
   const char16_t* u16;

libcxx/utils/libcxx/test/config.py

class Configuration(object):
 
     def configure_filesystem_compile_flags(self):
         static_env = os.path.join(self.libcxx_src_root, 'test', 'std',
                                   'input.output', 'filesystems', 'Inputs', 'static_test_env')
         static_env = os.path.realpath(static_env)
         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)
 
         self.cxx.compile_flags += ['-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER="%s %s"'
                                    % (sys.executable, dynamic_helper)]