-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[Utils] Sanitize %t with PathSanitizingFileCheck #84843
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
72099f2
9e2a18d
bc6a034
02dbc11
186b333
b22e4d2
98fe7c0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,15 @@ | ||
| // RUN: %empty-directory(%t.module-cache) | ||
| // RUN: %empty-directory(%t.scanner-cache) | ||
|
|
||
| // RUN: %target-swift-frontend -scan-dependencies -module-cache-path %t.module-cache -clang-scanner-module-cache-path %t.scanner-cache %s -o %t.deps.json -I %S/Inputs/SwiftDifferent -dump-clang-diagnostics 2>&1 | %FileCheck %s --check-prefix=CHECK-CLANG-COMMAND | ||
| // RUN: %validate-json %t.deps.json | %FileCheck %s --check-prefix=CHECK-DEPS | ||
| // RUN: %target-swift-frontend -scan-dependencies -module-cache-path %t.module-cache -clang-scanner-module-cache-path %t.scanner-cache %s -o %t.deps.json -I %S/Inputs/SwiftDifferent -dump-clang-diagnostics 2>&1 | %FileCheck %s --check-prefix=CHECK-CLANG-COMMAND --enable-yaml-compatibility | ||
| // RUN: %validate-json %t.deps.json | %FileCheck %s --check-prefix=CHECK-DEPS --enable-yaml-compatibility | ||
|
|
||
| // Ensure we prefer clang scanner module cache path | ||
| // CHECK-CLANG-COMMAND: '-fmodules-cache-path={{.*}}.scanner-cache' | ||
|
|
||
| // Ensure we the modules' output path is set to the module cache | ||
| // CHECK-DEPS: "swift": "A" | ||
| // CHECK-DEPS: "swift": "A" | ||
| // CHECK-DEPS: "modulePath": "{{.*}}separate_clang_scan_cache.swift.tmp.module-cache{{/|\\\\}}A-{{.*}}.swiftmodule" | ||
| // CHECK-DEPS: "modulePath": "TMP_DIR.module-cache{{/|\\\\}}A-{{.*}}.swiftmodule" | ||
|
|
||
| import A |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3114,17 +3114,12 @@ if hasattr(config, 'target_link_sdk_future_version'): | |
| config.substitutions.append(('%target-link-sdk-future-version', | ||
| config.target_link_sdk_future_version)) | ||
|
|
||
| run_filecheck = '%s %s --allow-unused-prefixes --sanitize BUILD_DIR=%s --sanitize SOURCE_DIR=%s --ignore-runtime-warnings --use-filecheck %s %s %s' % ( | ||
| run_filecheck = '%s %s --allow-unused-prefixes --sanitize TMP_DIR=%s --sanitize BUILD_DIR=%s --sanitize SOURCE_DIR=%s --ignore-runtime-warnings --use-filecheck %s %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. cmake_binary_dir 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(lit.util.abs_path_preserve_drive(config.cmake_binary_dir).replace("\\", "/")), | ||
| shell_quote(lit.util.abs_path_preserve_drive(config.swift_src_root).replace("\\", "/")), | ||
| shell_quote('%t'), | ||
| shell_quote(config.cmake_binary_dir), | ||
| shell_quote(config.swift_src_root), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This feels like its breaking developer setups. The
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure I follow; I don't see the call to it there.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| shell_quote(config.filecheck), | ||
| '--color' if config.color_output else '', | ||
| '--enable-windows-compatibility' if kIsWindows else '') | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,10 +11,26 @@ | |
|
|
||
| import argparse | ||
| import io | ||
| import os | ||
| import platform | ||
| import re | ||
| import subprocess | ||
| import sys | ||
|
|
||
| # LLVM Lit performs realpath with the config path, so all paths are relative | ||
| # to the real path. Paths that come from CMake (like cmake_binary_dir and | ||
| # swift_src_root), might not do real path. Use realpath to normalize. Because | ||
| # this normalizes Windows paths to use backslashes, we have to replace them | ||
| # back to forward slashes. | ||
| def normalize_if_path(s): | ||
| # Check dirname for cases like a file named `%t.out.txt` | ||
| # There won't be a `%t` path, but we still want to match this path substring. | ||
| if not os.path.exists(s) and not os.path.exists(os.path.dirname(s)): | ||
| return s | ||
| if platform.system() == "Windows": | ||
| return os.path.abspath(s).replace('\\', '/') | ||
| else: | ||
| return os.path.realpath(s) | ||
|
|
||
| def main(): | ||
| parser = argparse.ArgumentParser( | ||
|
|
@@ -78,13 +94,13 @@ constants.""") | |
|
|
||
| stdin = io.open(sys.stdin.fileno(), 'r', encoding='utf-8', errors='ignore').read() | ||
|
|
||
| for s in args.sanitize_strings: | ||
| for s in sorted(args.sanitize_strings, key=len, reverse=True): | ||
| replacement, pattern = s.split('=', 1) | ||
| # Since we want to use pattern as a regex in some platforms, we need | ||
| # to escape it first, and then replace the escaped slash | ||
| # literal (r'\\/') for our platform-dependent slash regex. | ||
| stdin = re.sub(re.sub(r'\\/' if sys.version_info[0] < 3 else r'/', | ||
| slashes_re, re.escape(pattern)), | ||
| slashes_re, re.escape(normalize_if_path(pattern))), | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @compnerd This is the call to
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, but I don't see the call to the function inside the implementation of
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah I see. It basically does the same thing, with some extra on top. Here's what the implementation of |
||
| replacement, stdin) | ||
|
|
||
| # Because we force the backtracer on in the tests, we can get runtime | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bnbarham Turns out this matcher doesn't actually match a file name, just a substring in the expansion of the
%tpath. There are other matchers in this file that do still match a file name like intended, so I'm not sure if this is just a mistake in the test, or indicative of a regression.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably more of a @nkcsgexi, but this seems fine as the test is written - we're not building
SDKDependencies.swifthere so it makes sense it's not in the dependencies. Though I'm unclear if it was meant to be buildingSDKDependencies.swifthere or not (ie.%srather than-in the run line above)