[libcxx-commits] [libcxx] 8bd9d0b - [libcxx] [test] Query the target platform, not the host one
Louis Dionne via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Dec 10 13:36:21 PST 2019
Author: Louis Dionne
Date: 2019-12-10T16:36:07-05:00
New Revision: 8bd9d0bff21b6732c122365de793de12fef9c681
URL: https://github.com/llvm/llvm-project/commit/8bd9d0bff21b6732c122365de793de12fef9c681
DIFF: https://github.com/llvm/llvm-project/commit/8bd9d0bff21b6732c122365de793de12fef9c681.diff
LOG: [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
Added:
Modified:
libcxx/utils/libcxx/test/config.py
libcxx/utils/libcxx/test/executor.py
libcxx/utils/libcxx/test/format.py
libcxx/utils/libcxx/test/target_info.py
Removed:
################################################################################
diff --git a/libcxx/utils/libcxx/test/config.py b/libcxx/utils/libcxx/test/config.py
index befe75c20e76..403abe9ddfb8 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 @@ def __init__(self, lit_config, config):
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 @@ def get_modules_enabled(self):
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 @@ def configure_executor(self):
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 @@ def configure_features(self):
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 @@ def configure_features(self):
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 @@ def configure_compile_flags(self):
# 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 @@ def configure_default_compile_flags(self):
# 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 @@ def configure_compile_flags_header_includes(self):
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 @@ def configure_compile_flags_header_includes(self):
'-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 @@ def configure_link_flags(self):
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 @@ def configure_link_flags_cxx_library_path(self):
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 @@ def configure_link_flags_abi_library_path(self):
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 @@ def configure_link_flags_abi_library(self):
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 @@ def configure_coroutines(self):
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 @@ def configure_modules(self):
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 @@ def configure_substitutions(self):
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 @@ def configure_env(self):
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 4581ce7a19be..4ba6164e1257 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 @@ def run(self, exe_path, cmd, local_cwd, file_deps=None, env=None):
"""
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 @@ def run(self, exe_path, cmd=None, work_dir='.', file_deps=None, env=None):
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 @@ def __init__(self, duration, chain):
class RemoteExecutor(Executor):
def __init__(self):
+ super(RemoteExecutor, self).__init__()
self.local_run = executeCommand
def remote_temp_dir(self):
@@ -120,7 +143,12 @@ def run(self, exe_path, cmd=None, work_dir='.', file_deps=None, env=None):
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 @@ def _copy_in_file(self, src, dst):
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 f8d0a85545d0..55f179a20e15 100644
--- a/libcxx/utils/libcxx/test/format.py
+++ b/libcxx/utils/libcxx/test/format.py
@@ -154,7 +154,7 @@ def _execute(self, test, lit_config):
# 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 6da30e672e14..d622daa2a871 100644
--- a/libcxx/utils/libcxx/test/target_info.py
+++ b/libcxx/utils/libcxx/test/target_info.py
@@ -23,6 +23,12 @@ def __init__(self, full_config):
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 @@ def allow_cxxabi_link(self): return True
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
More information about the libcxx-commits
mailing list