[libcxx] [test] Query the target platform, not the host one

target_info is inferred to WindowsLocalTI on Windows hosts unless
specified otherwise. In the latter case, it doesn't make sense to use
Windows-specific settings if the target is not Windows.

This change should not break anything, because target_info is inferred
based on what platform.system() returns. self.is_windows was set based
on the same platform.system() call.

Thanks to Sergej Jaskiewicz for the patch.

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

4 files changed, 91 insertions, 36 deletions

diff --git a/libcxx/utils/libcxx/test/config.py b/libcxx/utils/libcxx/test/config.py
index befe75c20e7..403abe9ddfb 100644
--- a/libcxx/utils/libcxx/test/config.py
+++ b/libcxx/utils/libcxx/test/config.py
@@ -59,7 +59,6 @@ class Configuration(object):
     def __init__(self, lit_config, config):
         self.lit_config = lit_config
         self.config = config
-        self.is_windows = platform.system() == 'Windows'
         self.cxx = None
         self.cxx_is_clang_cl = None
         self.cxx_stdlib_under_test = None
@@ -71,7 +70,7 @@ class Configuration(object):
         self.abi_library_root = None
         self.link_shared = self.get_lit_bool('enable_shared', default=True)
         self.debug_build = self.get_lit_bool('debug_build',   default=False)
-        self.exec_env = dict(os.environ)
+        self.exec_env = dict()
         self.use_target = False
         self.use_system_cxx_lib = False
         self.use_clang_verify = False
@@ -119,16 +118,16 @@ class Configuration(object):
 
     def make_static_lib_name(self, name):
         """Return the full filename for the specified library name"""
-        if self.is_windows:
+        if self.target_info.is_windows():
             assert name == 'c++'  # Only allow libc++ to use this function for now.
             return 'lib' + name + '.lib'
         else:
             return 'lib' + name + '.a'
 
     def configure(self):
+        self.configure_target_info()
         self.configure_executor()
         self.configure_use_system_cxx_lib()
-        self.configure_target_info()
         self.configure_cxx()
         self.configure_triple()
         self.configure_deployment()
@@ -201,6 +200,9 @@ class Configuration(object):
             te = LocalExecutor()
             if self.lit_config.useValgrind:
                 te = ValgrindExecutor(self.lit_config.valgrindArgs, te)
+
+        te.target_info = self.target_info
+
         self.executor = te
 
     def configure_target_info(self):
@@ -419,7 +421,7 @@ class Configuration(object):
             self.config.available_features.add('availability')
             self.add_deployment_feature('availability')
 
-        if platform.system() == 'Darwin':
+        if self.target_info.is_darwin():
             self.config.available_features.add('apple-darwin')
 
         # Insert the platform name into the available features as a lower case.
@@ -471,7 +473,7 @@ class Configuration(object):
                 intMacroValue(macros['__cpp_deduction_guides']) < 201611:
             self.config.available_features.add('libcpp-no-deduction-guides')
 
-        if self.is_windows:
+        if self.target_info.is_windows():
             self.config.available_features.add('windows')
             if self.cxx_stdlib_under_test == 'libc++':
                 # LIBCXX-WINDOWS-FIXME is the feature name used to XFAIL the
@@ -506,7 +508,7 @@ class Configuration(object):
         # Configure extra flags
         compile_flags_str = self.get_lit_conf('compile_flags', '')
         self.cxx.compile_flags += shlex.split(compile_flags_str)
-        if self.is_windows:
+        if self.target_info.is_windows():
             # FIXME: Can we remove this?
             self.cxx.compile_flags += ['-D_CRT_SECURE_NO_WARNINGS']
             # Required so that tests using min/max don't fail on Windows,
@@ -572,7 +574,7 @@ class Configuration(object):
         # the Windows build of libc++, the forced inclusion of a header requires
         # that _DEBUG is defined.  Incorrect ordering will result in -target
         # being elided.
-        if self.is_windows and self.debug_build:
+        if self.target_info.is_windows() and self.debug_build:
             self.cxx.compile_flags += ['-D_DEBUG']
         if self.use_target:
             if not self.cxx.addFlagIfSupported(
@@ -605,7 +607,7 @@ class Configuration(object):
         support_path = os.path.join(self.libcxx_src_root, 'test', 'support')
         self.configure_config_site_header()
         if self.cxx_stdlib_under_test != 'libstdc++' and \
-           not self.is_windows:
+           not self.target_info.is_windows():
             self.cxx.compile_flags += [
                 '-include', os.path.join(support_path, 'nasty_macros.h')]
         if self.cxx_stdlib_under_test == 'msvc':
@@ -613,7 +615,7 @@ class Configuration(object):
                 '-include', os.path.join(support_path,
                                          'msvc_stdlib_force_include.h')]
             pass
-        if self.is_windows and self.debug_build and \
+        if self.target_info.is_windows() and self.debug_build and \
                 self.cxx_stdlib_under_test != 'msvc':
             self.cxx.compile_flags += [
                 '-include', os.path.join(support_path,
@@ -757,7 +759,7 @@ class Configuration(object):
         if self.cxx_stdlib_under_test == 'libc++':
             self.cxx.link_flags += ['-nodefaultlibs']
             # FIXME: Handle MSVCRT as part of the ABI library handling.
-            if self.is_windows:
+            if self.target_info.is_windows():
                 self.cxx.link_flags += ['-nostdlib']
             self.configure_link_flags_cxx_library()
             self.configure_link_flags_abi_library()
@@ -780,20 +782,20 @@ class Configuration(object):
         if not self.use_system_cxx_lib:
             if self.cxx_library_root:
                 self.cxx.link_flags += ['-L' + self.cxx_library_root]
-                if self.is_windows and self.link_shared:
+                if self.target_info.is_windows() and self.link_shared:
                     self.add_path(self.cxx.compile_env, self.cxx_library_root)
             if self.cxx_runtime_root:
-                if not self.is_windows:
+                if not self.target_info.is_windows():
                     self.cxx.link_flags += ['-Wl,-rpath,' +
                                             self.cxx_runtime_root]
-                elif self.is_windows and self.link_shared:
+                elif self.target_info.is_windows() and self.link_shared:
                     self.add_path(self.exec_env, self.cxx_runtime_root)
         elif os.path.isdir(str(self.use_system_cxx_lib)):
             self.cxx.link_flags += ['-L' + self.use_system_cxx_lib]
-            if not self.is_windows:
+            if not self.target_info.is_windows():
                 self.cxx.link_flags += ['-Wl,-rpath,' +
                                         self.use_system_cxx_lib]
-            if self.is_windows and self.link_shared:
+            if self.target_info.is_windows() and self.link_shared:
                 self.add_path(self.cxx.compile_env, self.use_system_cxx_lib)
         additional_flags = self.get_lit_conf('test_linker_flags')
         if additional_flags:
@@ -804,7 +806,7 @@ class Configuration(object):
         self.abi_library_root = self.get_lit_conf('abi_library_path')
         if self.abi_library_root:
             self.cxx.link_flags += ['-L' + self.abi_library_root]
-            if not self.is_windows:
+            if not self.target_info.is_windows():
                 self.cxx.link_flags += ['-Wl,-rpath,' + self.abi_library_root]
             else:
                 self.add_path(self.exec_env, self.abi_library_root)
@@ -857,7 +859,7 @@ class Configuration(object):
             self.cxx.link_flags += ['-l%s%s' % (lib, debug_suffix) for lib in
                                     ['vcruntime', 'ucrt', 'msvcrt']]
         elif cxx_abi == 'none' or cxx_abi == 'default':
-            if self.is_windows:
+            if self.target_info.is_windows():
                 debug_suffix = 'd' if self.debug_build else ''
                 self.cxx.link_flags += ['-lmsvcrt%s' % debug_suffix]
         else:
@@ -1015,7 +1017,7 @@ class Configuration(object):
 
     def configure_modules(self):
         modules_flags = ['-fmodules']
-        if platform.system() != 'Darwin':
+        if not self.target_info.is_darwin():
             modules_flags += ['-Xclang', '-fmodules-local-submodule-visibility']
         supports_modules = self.cxx.hasCompileFlag(modules_flags)
         enable_modules = self.get_modules_enabled()
@@ -1039,7 +1041,7 @@ class Configuration(object):
 
     def configure_substitutions(self):
         tool_env = ''
-        if platform.system() == 'Darwin':
+        if self.target_info.is_darwin():
             # Do not pass DYLD_LIBRARY_PATH to the compiler, linker, etc. as
             # these tools are not meant to exercise the just-built libraries.
             tool_env += 'DYLD_LIBRARY_PATH="" '
@@ -1094,7 +1096,7 @@ class Configuration(object):
 
     def can_use_deployment(self):
         # Check if the host is on an Apple platform using clang.
-        if not self.target_info.platform() == "darwin":
+        if not self.target_info.is_darwin():
             return False
         if not self.target_info.is_host_macosx():
             return False
@@ -1205,9 +1207,4 @@ class Configuration(object):
         self.target_info.configure_env(self.exec_env)
 
     def add_path(self, dest_env, new_path):
-        if 'PATH' not in dest_env:
-            dest_env['PATH'] = new_path
-        else:
-            split_char = ';' if self.is_windows else ':'
-            dest_env['PATH'] = '%s%s%s' % (new_path, split_char,
-                                           dest_env['PATH'])
+        self.target_info.add_path(dest_env, new_path)
diff --git a/libcxx/utils/libcxx/test/executor.py b/libcxx/utils/libcxx/test/executor.py
index 4581ce7a19b..4ba6164e125 100644
--- a/libcxx/utils/libcxx/test/executor.py
+++ b/libcxx/utils/libcxx/test/executor.py
@@ -8,12 +8,16 @@
 
 import platform
 import os
+import posixpath
+import ntpath
 
 from libcxx.test import tracing
 from libcxx.util import executeCommand
 
-
 class Executor(object):
+    def __init__(self):
+        self.target_info = None
+
     def run(self, exe_path, cmd, local_cwd, file_deps=None, env=None):
         """Execute a command.
             Be very careful not to change shared state in this function.
@@ -29,6 +33,20 @@ class Executor(object):
         """
         raise NotImplementedError
 
+    def merge_environments(self, current_env, updated_env):
+        """Merges two execution environments.
+
+        If both environments contain the PATH variables, they are also merged
+        using the proper separator.
+        """
+        result_env = dict(current_env)
+        for k, v in updated_env.items():
+            if k == 'PATH' and self.target_info:
+                self.target_info.add_path(result_env, v)
+            else:
+                result_env[k] = v
+        return result_env
+
 
 class LocalExecutor(Executor):
     def __init__(self):
@@ -39,6 +57,10 @@ class LocalExecutor(Executor):
         cmd = cmd or [exe_path]
         if work_dir == '.':
             work_dir = os.getcwd()
+
+        if env:
+            env = self.merge_environments(os.environ, env)
+
         out, err, rc = executeCommand(cmd, cwd=work_dir, env=env)
         return (cmd, out, err, rc)
 
@@ -88,6 +110,7 @@ class TimeoutExecutor(PrefixExecutor):
 
 class RemoteExecutor(Executor):
     def __init__(self):
+        super(RemoteExecutor, self).__init__()
         self.local_run = executeCommand
 
     def remote_temp_dir(self):
@@ -120,7 +143,12 @@ class RemoteExecutor(Executor):
         target_cwd = None
         try:
             target_cwd = self.remote_temp_dir()
-            target_exe_path = os.path.join(target_cwd, 'libcxx_test.exe')
+            executable_name = 'libcxx_test.exe'
+            if self.target_info.is_windows():
+                target_exe_path = ntpath.join(target_cwd, executable_name)
+            else:
+                target_exe_path = posixpath.join(target_cwd, executable_name)
+
             if cmd:
                 # Replace exe_path with target_exe_path.
                 cmd = [c if c != exe_path else target_exe_path for c in cmd]
@@ -191,15 +219,29 @@ class SSHExecutor(RemoteExecutor):
         cmd = [scp, '-p', src, remote + ':' + dst]
         self.local_run(cmd)
 
+    def _export_command(self, env):
+        if not env:
+            return []
+
+        export_cmd = ['export']
+
+        for k, v in env.items():
+            v = v.replace('\\', '\\\\')
+            if k == 'PATH':
+                # Pick up the existing paths, so we don't lose any commands
+                if self.target_info and self.target_info.is_windows():
+                    export_cmd.append('PATH="%s;%PATH%"' % v)
+                else:
+                    export_cmd.append('PATH="%s:$PATH"' % v)
+            else:
+                export_cmd.append('"%s"="%s"' % (k, v))
+
+        return export_cmd
+
     def _execute_command_remote(self, cmd, remote_work_dir='.', env=None):
         remote = self.user_prefix + self.host
         ssh_cmd = [self.ssh_command, '-oBatchMode=yes', remote]
-        if env:
-            export_cmd = \
-                ['export'] + ['"%s"="%s"' % (k, v) for k, v in env.items()]
-        else:
-            export_cmd = []
-
+        export_cmd = self._export_command(env)
         remote_cmd = ' '.join(cmd)
         if export_cmd:
             remote_cmd = ' '.join(export_cmd) + ' && ' + remote_cmd
diff --git a/libcxx/utils/libcxx/test/format.py b/libcxx/utils/libcxx/test/format.py
index f8d0a85545d..55f179a20e1 100644
--- a/libcxx/utils/libcxx/test/format.py
+++ b/libcxx/utils/libcxx/test/format.py
@@ -154,7 +154,7 @@ class LibcxxTestFormat(object):
                 # We can't run ShTest tests with a executor yet.
                 # For now, bail on trying to run them
                 return lit.Test.UNSUPPORTED, 'ShTest format not yet supported'
-            test.config.environment = dict(self.exec_env)
+            test.config.environment = self.executor.merge_environments(os.environ, self.exec_env)
             return lit.TestRunner._runShTest(test, lit_config,
                                              self.execute_external, script,
                                              tmpBase)
diff --git a/libcxx/utils/libcxx/test/target_info.py b/libcxx/utils/libcxx/test/target_info.py
index 6da30e672e1..d622daa2a87 100644
--- a/libcxx/utils/libcxx/test/target_info.py
+++ b/libcxx/utils/libcxx/test/target_info.py
@@ -23,6 +23,12 @@ class DefaultTargetInfo(object):
     def platform(self):
         return sys.platform.lower().strip()
 
+    def is_windows(self):
+        return self.platform() == 'win32'
+
+    def is_darwin(self):
+        return self.platform() == 'darwin'
+
     def add_locale_features(self, features):
         self.full_config.lit_config.warning(
             "No locales entry for target_system: %s" % self.platform())
@@ -34,6 +40,16 @@ class DefaultTargetInfo(object):
     def add_sanitizer_features(self, sanitizer_type, features): pass
     def use_lit_shell_default(self): return False
 
+    def add_path(self, dest_env, new_path):
+        if not new_path:
+            return
+        if 'PATH' not in dest_env:
+            dest_env['PATH'] = new_path
+        else:
+            split_char = ';' if self.is_windows() else ':'
+            dest_env['PATH'] = '%s%s%s' % (new_path, split_char,
+                                           dest_env['PATH'])
+
 
 def test_locale(loc):
     assert loc is not None