[windows] Avoid %r for quoting module-cache-path in Windows.

%r returns a representation of the object that is valid Python syntax.
For numbers and strings this representation is very compatible with
Unix shells, but for Windows, the quoting performed by Python will not
be compatible. For example the Windows path separator `\` will be
escaped as `\\`, which is not necessary for Windows and might make
some tests that try to match path fail.

Python 3.3 has `shlex.quote`, which was previously available as
`pipes.quote` in earlier Python versions. `pipes.quote` was indeed
used in several points of the `lit.cfg` file. For Windows, one need to
do their own quoting.

This change introduces `shell_quote` which uses `shlex.quote` or
`pipes.quote` in Unix depending on the Python version, and provides an
implementation of `shell_quote` for Windows. It replaces every usage
of `pipes.quotes` for `shell_quote`, and modifies the value of
`mcp_opt` to use `shell_quote` and not `%r`.

This should fix the test `Driver\working-directory.swift` in the
Windows Visual Studio 2017 builder.

Author: drodriguez

test/Driver/working-directory.swift

@@ -60,9 +60,9 @@
 // -output-file-map itself
 // RUN: echo "{\"main.swift\": {\"object\": \"main-modified.o\"}}" > %t/ofmo2.json
 // RUN: touch %t/main.swift
-// RUN: cd %S && %swiftc_driver -driver-print-jobs -working-directory %/t -c main.swift -output-file-map ofmo2.json | %FileCheck %s -check-prefix=OUTPUT_FILE_MAP_2
+// RUN: cd %S && %swiftc_driver -driver-print-jobs -working-directory %/t -c main.swift -output-file-map ofmo2.json | %FileCheck %s -check-prefix=OUTPUT_FILE_MAP_2 --enable-yaml-compatibility
 // -output-file-map= is an alias for -output-file-map
-// RUN: cd %S && %swiftc_driver -driver-print-jobs -working-directory %/t -c main.swift -output-file-map=ofmo2.json | %FileCheck %s -check-prefix=OUTPUT_FILE_MAP_2
+// RUN: cd %S && %swiftc_driver -driver-print-jobs -working-directory %/t -c main.swift -output-file-map=ofmo2.json | %FileCheck %s -check-prefix=OUTPUT_FILE_MAP_2 --enable-yaml-compatibility
 // OUTPUT_FILE_MAP_2: BUILD_DIR{{.*}}main-modified.o
 
 // RUN: %empty-directory(%t/sub)

test/lit.cfg

@@ -110,6 +110,27 @@ def get_lldb_python_interpreter(lldb_build_root):
         return None
     return python_path
 
+if not platform.system() == 'Windows':
+    # Python 3.3 has shlex.quote, while previous Python have pipes.quote
+    if sys.version_info[0:2] >= (3, 2):
+        shell_quote = shlex.quote
+    else:
+        shell_quote = pipes.quote
+else:
+    # In Windows neither pipe.quote nor shlex.quote works.
+    def shell_quote(s):
+        # Quote the argument if it is empty, or contains a quote or a space.
+        if len(s) == 0 or re.search(r'["\s]', s):
+            s = '"' + s.replace('"', r'\"') + '"'
+        return s
+
+def escape_for_substitute_captures(s):
+    # SubstituteCaptures strings are used as replacement patterns for regular
+    # expressions. In them escapes like \1, \2 are used as references, but that
+    # means that simple \ will try to be interpreted, so we need to escape them
+    # with \\.
+    return s.replace("\\", "\\\\")
+
 ###
 
 # Check that the object root is known.
@@ -380,7 +401,7 @@ config.sil_test_options = os.environ.get('SIL_TEST_OPTIONS', '')
 
 config.clang_module_cache_path = make_path(config.swift_test_results_dir, "clang-module-cache")
 shutil.rmtree(config.clang_module_cache_path, ignore_errors=True)
-mcp_opt = "-module-cache-path %r" % config.clang_module_cache_path
+mcp_opt = "-module-cache-path %s" % shell_quote(config.clang_module_cache_path)
 clang_mcp_opt = "-fmodules-cache-path=%r" % config.clang_module_cache_path
 lit_config.note("Using Clang module cache: " + config.clang_module_cache_path)
 lit_config.note("Using test results dir: " + config.swift_test_results_dir)
@@ -396,7 +417,7 @@ config.substitutions.append( ('%llvm_obj_root', config.llvm_obj_root) )
 config.substitutions.append( ('%llvm_src_root', config.llvm_src_root) )
 config.substitutions.append( ('%swift_obj_root', config.swift_obj_root) )
 config.substitutions.append( ('%swift_src_root', config.swift_src_root) )
-config.substitutions.append( ('%{python}', pipes.quote(sys.executable)) )
+config.substitutions.append( ('%{python}', shell_quote(sys.executable)) )
 config.substitutions.append( ('%{python.unquoted}', sys.executable) )
 config.substitutions.append( ('%mcp_opt', mcp_opt) )
 config.substitutions.append( ('%swift_driver_plain', "%r" % config.swift) )
@@ -1096,7 +1117,8 @@ elif run_os in ['windows-msvc']:
     subst_target_swift_frontend_mock_sdk_after = ''
 
     config.target_build_swift_dylib =                                            \
-            SubstituteCaptures("%s -parse-as-library -emit-library -o \\1" % (config.target_build_swift))
+            SubstituteCaptures(r"%s -parse-as-library -emit-library -o \1" % (
+                escape_for_substitute_captures(config.target_build_swift)))
     config.target_add_rpath = r''
 
     config.target_clang =                                                        \
@@ -1280,7 +1302,7 @@ elif run_os == 'linux-androideabi' or run_os == 'linux-android':
     toolchain_directory = make_path(
         config.android_ndk_path, "toolchains", toolchain_directory_name,
         "prebuilt", prebuilt_directory)
-    tools_directory = pipes.quote(make_path(
+    tools_directory = shell_quote(make_path(
         toolchain_directory, ndk_platform_triple, "bin"))
     lit_config.note("Testing Android " + config.variant_triple)
     config.target_object_format = "elf"
@@ -1290,20 +1312,20 @@ elif run_os == 'linux-androideabi' or run_os == 'linux-android':
     config.target_swift_autolink_extract = inferSwiftBinary("swift-autolink-extract")
     config.target_sdk_name = "android"
     android_link_paths_opt = "-L {} -L {} -L {}".format(
-        pipes.quote(make_path(
+        shell_quote(make_path(
             config.android_ndk_path, "sources", "cxx-stl", "llvm-libc++",
             "libs", ndk_platform_tuple)),
-        pipes.quote(make_path(
+        shell_quote(make_path(
             toolchain_directory, "lib", "gcc", ndk_platform_triple,
             "{}.x".format(config.android_ndk_gcc_version))),
-        pipes.quote(make_path(
+        shell_quote(make_path(
             toolchain_directory, ndk_platform_triple, "lib")))
     # Since NDK r14 the headers are unified under $NDK_PATH/sysroot, so the -sdk
     # switch is not enough. Additionally we have to include both the unified
     # sysroot, and the architecture sysroot.
-    unified_android_include_path = pipes.quote(make_path(
+    unified_android_include_path = shell_quote(make_path(
         config.android_ndk_path, "sysroot", "usr", "include"))
-    architecture_android_include_path = pipes.quote(make_path(
+    architecture_android_include_path = shell_quote(make_path(
         config.android_ndk_path, "sysroot", "usr", "include",
         ndk_platform_triple))
     android_include_paths_opt = "-I {} -I {}".format(
@@ -1749,21 +1771,27 @@ if not kIsWindows:
 
 if not getattr(config, 'target_run_simple_swift', None):
     config.target_run_simple_swift_parameterized = SubstituteCaptures(
-        "%%empty-directory(%%t) && "
-        "%s %s %%s \\1 -o %%t/a.out -module-name main  && "
-        "%s %%t/a.out && "
-        "%s %%t/a.out"
-        % (config.target_build_swift, mcp_opt, config.target_codesign, config.target_run)
+        r"%%empty-directory(%%t) && "
+        r"%s %s %%s \1 -o %%t/a.out -module-name main  && "
+        r"%s %%t/a.out && "
+        r"%s %%t/a.out"
+        % (escape_for_substitute_captures(config.target_build_swift),
+           escape_for_substitute_captures(mcp_opt),
+           escape_for_substitute_captures(config.target_codesign),
+           escape_for_substitute_captures(config.target_run))
     )
     config.target_run_simple_swiftgyb_parameterized = SubstituteCaptures(
-        "%%empty-directory(%%t) && "
-        "%%gyb %%s -o %%t/main.swift && "
-        "%%line-directive %%t/main.swift -- "
-        "%s %s %%t/main.swift \\1 -o %%t/a.out -module-name main  && "
-        "%s %%t/a.out && "
-        "%%line-directive %%t/main.swift -- "
-        "%s %%t/a.out"
-        % (config.target_build_swift, mcp_opt, config.target_codesign, config.target_run)
+        r"%%empty-directory(%%t) && "
+        r"%%gyb %%s -o %%t/main.swift && "
+        r"%%line-directive %%t/main.swift -- "
+        r"%s %s %%t/main.swift \1 -o %%t/a.out -module-name main  && "
+        r"%s %%t/a.out && "
+        r"%%line-directive %%t/main.swift -- "
+        r"%s %%t/a.out"
+        % (escape_for_substitute_captures(config.target_build_swift),
+          escape_for_substitute_captures(mcp_opt),
+          escape_for_substitute_captures(config.target_codesign),
+          escape_for_substitute_captures(config.target_run))
     )
 
     config.target_run_simple_swift = (
@@ -1804,7 +1832,7 @@ config.target_resilience_test = (
      '%s %s --target-build-swift "%s" --target-run "%s" --t %%t --S %%S '
      '--s %%s --lib-prefix "%s" --lib-suffix "%s" --target-codesign "%s" '
      '--additional-compile-flags "%s" --triple "%s"'
-     % (pipes.quote(sys.executable), config.rth, config.target_build_swift,
+     % (shell_quote(sys.executable), config.rth, config.target_build_swift,
         config.target_run, config.target_shared_library_prefix,
         config.target_shared_library_suffix, config.target_codesign,
         rth_flags, config.variant_triple))
@@ -1833,8 +1861,9 @@ config.substitutions.append(('%target-swift-emit-ir\(mock-sdk:([^)]+)\)',
                              SubstituteCaptures(r'%target-swift-frontend(mock-sdk:\1) -emit-ir -verify-syntax-tree')))
 config.substitutions.append(('%target-swift-emit-ir', '%target-swift-frontend -emit-ir -verify-syntax-tree'))
 config.substitutions.append(('%target-swift-frontend\(mock-sdk:([^)]+)\)',
-                             SubstituteCaptures(r'%s \1 %s' % (subst_target_swift_frontend_mock_sdk,
-                                                               subst_target_swift_frontend_mock_sdk_after))))
+                             SubstituteCaptures(r'%s \1 %s' % (
+                                 escape_for_substitute_captures(subst_target_swift_frontend_mock_sdk),
+                                 escape_for_substitute_captures(subst_target_swift_frontend_mock_sdk_after)))))
 config.substitutions.append(('%target-swift-frontend', config.target_swift_frontend))
 
 
@@ -1872,22 +1901,23 @@ else:
        SubstituteCaptures(r'ln \1 \2 || cp \1 \2')))
 
 config.substitutions.append(('%utils', config.swift_utils))
-config.substitutions.append(('%line-directive', '%s %s' % (pipes.quote(sys.executable), config.line_directive)))
-config.substitutions.append(('%gyb', '%s %s' % (pipes.quote(sys.executable), config.gyb)))
+config.substitutions.append(('%line-directive', '%s %s' % (shell_quote(sys.executable), config.line_directive)))
+config.substitutions.append(('%gyb', '%s %s' % (shell_quote(sys.executable), config.gyb)))
 config.substitutions.append(('%round-trip-syntax-test',
-                             '%s %s' % (pipes.quote(sys.executable),
+                             '%s %s' % (shell_quote(sys.executable),
                                         config.round_trip_syntax_test)))
-config.substitutions.append(('%rth', '%s %s' % (pipes.quote(sys.executable), config.rth)))
+config.substitutions.append(('%rth', '%s %s' % (shell_quote(sys.executable), config.rth)))
 config.substitutions.append(('%scale-test',
                              '{} {} --swiftc-binary={} --tmpdir=%t'.format(
-                                 pipes.quote(sys.executable), config.scale_test,
+                                 shell_quote(sys.executable), config.scale_test,
                                  config.swiftc)))
 config.substitutions.append(('%empty-directory\(([^)]+)\)',
                              SubstituteCaptures(r'rm -rf "\1" && mkdir -p "\1"')))
 
 config.substitutions.append(('%target-sil-opt\(mock-sdk:([^)]+)\)',
-                             SubstituteCaptures(r'%s \1 %s' % (subst_target_sil_opt_mock_sdk,
-                                                               subst_target_sil_opt_mock_sdk_after))))
+                             SubstituteCaptures(r'%s \1 %s' % (
+                                 escape_for_substitute_captures(subst_target_sil_opt_mock_sdk),
+                                 escape_for_substitute_captures(subst_target_sil_opt_mock_sdk_after)))))
 # NOTE: This needs to be appended after the mock-sdk expansion to ensure that we
 # first expand the mock-sdk when lit is processing.
 config.substitutions.append(('%target-sil-opt', config.target_sil_opt))
@@ -1897,9 +1927,10 @@ config.substitutions.append(('%target-sil-llvm-gen', config.target_sil_llvm_gen)
 config.substitutions.append(('%target-sil-nm', config.target_sil_nm))
 
 config.substitutions.append(('%target-swift-ide-test\(mock-sdk:([^)]+)\)',
-                             SubstituteCaptures(r'%s \1 %s -swift-version %s' % (subst_target_swift_ide_test_mock_sdk,
-                                                                                 subst_target_swift_ide_test_mock_sdk_after,
-                                                                                 swift_version))))
+                             SubstituteCaptures(r'%s \1 %s -swift-version %s' % (
+                                 escape_for_substitute_captures(subst_target_swift_ide_test_mock_sdk),
+                                 escape_for_substitute_captures(subst_target_swift_ide_test_mock_sdk_after),
+                                 escape_for_substitute_captures(swift_version)))))
 config.substitutions.append(('%target-swift-ide-test', "%s -swift-version %s" % (config.target_swift_ide_test, swift_version)))
 
 config.substitutions.append(('%target-swift-symbolgraph-extract', config.target_swift_symbolgraph_extract))
@@ -1944,8 +1975,9 @@ config.substitutions.append(('%target-object-format', config.target_object_forma
 config.substitutions.append(('%{target-shared-library-prefix}', config.target_shared_library_prefix))
 config.substitutions.append(('%{target-shared-library-suffix}', config.target_shared_library_suffix))
 config.substitutions.insert(0, ('%target-library-name\(([^)]+)\)',
-                                SubstituteCaptures(r'%s\1%s' % (config.target_shared_library_prefix,
-                                                                config.target_shared_library_suffix))))
+                                SubstituteCaptures(r'%s\1%s' % (
+                                    escape_for_substitute_captures(config.target_shared_library_prefix),
+                                    escape_for_substitute_captures(config.target_shared_library_suffix)))))
 config.substitutions.append(('%target-rpath\(([^)]+)\)',
                              SubstituteCaptures(config.target_add_rpath)))
 
@@ -1958,14 +1990,20 @@ if hasattr(config, 'otool_classic'):
     config.substitutions.append(('%otool-classic', config.otool_classic))
 
 config.substitutions.append(('%FileCheck',
-                             '%s %r --sanitize BUILD_DIR=%r --sanitize SOURCE_DIR=%r --use-filecheck %r %s' % (
-        pipes.quote(sys.executable),
-        config.PathSanitizingFileCheck,
-        swift_obj_root,
-        config.swift_src_root,
-        config.filecheck,
+                             '%s %s --sanitize BUILD_DIR=%s --sanitize SOURCE_DIR=%s --use-filecheck %s %s' % (
+        shell_quote(sys.executable),
+        shell_quote(config.PathSanitizingFileCheck),
+        # LLVM Lit performs realpath with the config path, so all paths are relative
+        # to the real path. swift_obj_root and swift_src_root come from CMake, which
+        # might not do real path. Because we have to match what Lit uses against what
+        # we provide we use realpath here. Because PathSanitizingFileCheck only
+        # understands sanitize patterns with forward slashes, and realpath normalizes
+        # the slashes, we have to replace them back to forward slashes.
+        shell_quote(os.path.realpath(swift_obj_root).replace("\\", "/")),
+        shell_quote(os.path.realpath(config.swift_src_root).replace("\\", "/")),
+        shell_quote(config.filecheck),
         '--enable-windows-compatibility' if kIsWindows else '')))
-config.substitutions.append(('%raw-FileCheck', pipes.quote(config.filecheck)))
+config.substitutions.append(('%raw-FileCheck', shell_quote(config.filecheck)))
 config.substitutions.append(('%import-libdispatch', getattr(config, 'import_libdispatch', '')))
 
 if config.lldb_build_root != "":