Merge pull request swiftlang#34176 from drodriguez/windows-percent-r-is-bad

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

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 != "":