[compiler-rt] a89d54f - [compiler-rt] Better Windows support for running tests in external shell

[Sergej Jaskiewicz via llvm-commits]

Author: Sergej Jaskiewicz
Date: 2020-07-09T19:40:22+03:00
New Revision: a89d54fd61a6e7a05f7434491135e667306a22e7

URL: https://github.com/llvm/llvm-project/commit/a89d54fd61a6e7a05f7434491135e667306a22e7
DIFF: https://github.com/llvm/llvm-project/commit/a89d54fd61a6e7a05f7434491135e667306a22e7.diff

LOG: [compiler-rt] Better Windows support for running tests in external shell

Summary:
These changes are necessary to support remote running compiler-rt tests
that were compiled on Windows.

Most of the code here has been copy-pasted from other lit configs.

Why do we remove the conversions to ASCII in the crt config?

We set the `universal_newlines` argument to `True` in `Popen` instead.
This is supported in both Python 2.7 and 3, is easier
(no need to do the `str(dir.decode('ascii'))` dance) and less
error prone.

Also, this is necessary because if the config is executed on Windows,
and `execute_external` is `True`, we take the branch
`if sys.platform in ['win32'] and execute_external`,
and if we use Python 3, then the `dir` variable is a byte-like object,
not str, but the ``replace method on byte-like objects requires its
arguments to also be byte-like objects, which is incompatible with
Python 2 etc etc.

It is a lot simpler to just work with strings in the first place, which
is achieved by setting `universal_newlines` to `True`. As far as
I understand, this way wasn't taken because of the need to support
Python <2.7, but this is not the case now.

Reviewers: compnerd, phosek, weimingz

Reviewed By: compnerd

Subscribers: dberris, #sanitizers

Tags: #sanitizers

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

Added: 
    

Modified: 
    compiler-rt/test/builtins/Unit/lit.cfg.py
    compiler-rt/test/crt/lit.cfg.py

Removed: 
    


################################################################################
diff  --git a/compiler-rt/test/builtins/Unit/lit.cfg.py b/compiler-rt/test/builtins/Unit/lit.cfg.py
index 8fdb1a216ee2..c8888078be50 100644
--- a/compiler-rt/test/builtins/Unit/lit.cfg.py
+++ b/compiler-rt/test/builtins/Unit/lit.cfg.py
@@ -5,6 +5,17 @@
 
 import lit.formats
 
+# Choose between lit's internal shell pipeline runner and a real shell.  If
+# LIT_USE_INTERNAL_SHELL is in the environment, we use that as an override.
+use_lit_shell = os.environ.get("LIT_USE_INTERNAL_SHELL")
+if use_lit_shell:
+    # 0 is external, "" is default, and everything else is internal.
+    execute_external = (use_lit_shell == "0")
+else:
+    # Otherwise we default to internal on Windows and external elsewhere, as
+    # bash on Windows is usually very slow.
+    execute_external = (not sys.platform in ['win32'])
+
 def get_required_attr(config, attr_name):
   attr_value = getattr(config, attr_name, None)
   if attr_value == None:
@@ -35,10 +46,16 @@ def get_required_attr(config, attr_name):
 else:
   base_lib = os.path.join(config.compiler_rt_libdir, "libclang_rt.builtins%s.a"
                           % config.target_suffix)
+  if sys.platform in ['win32'] and execute_external:
+    # Don't pass dosish path separator to msys bash.exe.
+    base_lib = base_lib.replace('\\', '/')
   config.substitutions.append( ("%librt ", base_lib + ' -lc -lm ') )
 
 builtins_source_dir = os.path.join(
   get_required_attr(config, "compiler_rt_src_root"), "lib", "builtins")
+if sys.platform in ['win32'] and execute_external:
+  # Don't pass dosish path separator to msys bash.exe.
+  builtins_source_dir = builtins_source_dir.replace('\\', '/')
 builtins_lit_source_dir = get_required_attr(config, "builtins_lit_source_dir")
 
 extra_link_flags = ["-nodefaultlibs"]

diff  --git a/compiler-rt/test/crt/lit.cfg.py b/compiler-rt/test/crt/lit.cfg.py
index dc15e456fe19..68e7eda7d59b 100644
--- a/compiler-rt/test/crt/lit.cfg.py
+++ b/compiler-rt/test/crt/lit.cfg.py
@@ -26,15 +26,15 @@ def get_library_path(file):
                             config.target_cflags.strip(),
                             '-print-file-name=%s' % file],
                            stdout=subprocess.PIPE,
-                           env=config.environment)
+                           env=config.environment,
+                           universal_newlines=True)
     if not cmd.stdout:
       lit_config.fatal("Couldn't find the library path for '%s'" % file)
     dir = cmd.stdout.read().strip()
     if sys.platform in ['win32'] and execute_external:
         # Don't pass dosish path separator to msys bash.exe.
         dir = dir.replace('\\', '/')
-    # Ensure the result is an ascii string, across Python2.5+ - Python3.
-    return str(dir.decode('ascii'))
+    return dir
 
 
 def get_libgcc_file_name():
@@ -42,15 +42,15 @@ def get_libgcc_file_name():
                             config.target_cflags.strip(),
                             '-print-libgcc-file-name'],
                            stdout=subprocess.PIPE,
-                           env=config.environment)
+                           env=config.environment,
+                           universal_newlines=True)
     if not cmd.stdout:
       lit_config.fatal("Couldn't find the library path for '%s'" % file)
     dir = cmd.stdout.read().strip()
     if sys.platform in ['win32'] and execute_external:
         # Don't pass dosish path separator to msys bash.exe.
         dir = dir.replace('\\', '/')
-    # Ensure the result is an ascii string, across Python2.5+ - Python3.
-    return str(dir.decode('ascii'))
+    return dir
 
 
 def build_invocation(compile_flags):
@@ -66,6 +66,11 @@ def build_invocation(compile_flags):
 
 base_lib = os.path.join(
     config.compiler_rt_libdir, "clang_rt.%%s%s.o" % config.target_suffix)
+
+if sys.platform in ['win32'] and execute_external:
+    # Don't pass dosish path separator to msys bash.exe.
+    base_lib = base_lib.replace('\\', '/')
+
 config.substitutions.append(('%crtbegin', base_lib % "crtbegin"))
 config.substitutions.append(('%crtend', base_lib % "crtend"))