-
Notifications
You must be signed in to change notification settings - Fork 345
Fix the following CAS-related tests on Windows #11236
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
base: next
Are you sure you want to change the base?
Conversation
@swift-ci please test llvm |
fe578a2
to
69a9aba
Compare
@swift-ci please test llvm |
There are unrelated test failures:
|
@@ -7,10 +7,10 @@ | |||
// Try again to ensure a pre-populated CASDB doesn't change output. | |||
// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-include-tree -cas-path %t/cas > %t/result2.txt | |||
// RUN: diff -u %t/result1.txt %t/result2.txt | |||
// RUN: FileCheck %s -input-file %t/result1.txt -DPREFIX=%/t | |||
// RUN: cat %t/result1.txt | sed 's/\\/\//g' | FileCheck %s -DPREFIX=%/t |
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.
How bad it is to update all the tests with regex match, rather than use sed
to replace separators? I prefer the actually fix the CHECK line.
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.
ok I'll take a pass over these tests to see
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.
Sorry, a correction: the macros like %/t use forward slashes on windows (except %clang doesn't :).
The reason we get mixed slashes is the output from windows will add backslashes like
C:/Users/hiroshi/cas2/llvm-project/build/tools/clang/test/ClangScanDeps/Output/include-tree.c.tmp\top.h
In the particular line, the issue is that PREFIX is
# RUN: at line 11
cat C:\Users\hiroshi\cas2\llvm-project\build\tools\clang\test\ClangScanDeps\Output\include-tree.c.tmp/result1.txt | c:\users\hiroshi\cas2\llvm-project\build\bin\filecheck.exe C:\Users\hiroshi\cas2\llvm-project\clang\test\ClangScanDeps\include-tree.c -DPREFIX=C:/Users/hiroshi/cas2/llvm-project/build/tools/clang/test/ClangScanDeps/Output/include-tree.c.tmp
# executed command: cat 'C:\Users\hiroshi\cas2\llvm-project\build\tools\clang\test\ClangScanDeps\Output\include-tree.c.tmp/result1.txt'
# executed command: 'c:\users\hiroshi\cas2\llvm-project\build\bin\filecheck.exe' 'C:\Users\hiroshi\cas2\llvm-project\clang\test\ClangScanDeps\include-tree.c' -DPREFIX=C:/Users/hiroshi/cas2/llvm-project/build/tools/clang/test/ClangScanDeps/Output/include-tree.c.tmp
# .---command stderr------------
# | C:\Users\hiroshi\cas2\llvm-project\clang\test\ClangScanDeps\include-tree.c:116:11: error: CHECK: expected string not found in input
# | // CHECK: [[@LINE]]:1 [[PREFIX]]{{/\\}}top.h
# | ^
# | <stdin>:1:1: note: scanning from here
# | llvmcas://f1b6b94db71c27d5c63749278ad79a956c91c3d60624f0bd746c898c0573ebcc - C:/Users/hiroshi/cas2/llvm-project/build/tools/clang/test/ClangScanDeps/Output/include-tree.c.tmp/t.c
# | ^
# | <stdin>:1:1: note: with "@LINE" equal to "116"
# | llvmcas://f1b6b94db71c27d5c63749278ad79a956c91c3d60624f0bd746c898c0573ebcc - C:/Users/hiroshi/cas2/llvm-project/build/tools/clang/test/ClangScanDeps/Output/include-tree.c.tmp/t.c
# | ^
# | <stdin>:1:1: note: with "PREFIX" equal to "C:/Users/hiroshi/cas2/llvm-project/build/tools/clang/test/ClangScanDeps/Output/include-tree.c.tmp"
# | llvmcas://f1b6b94db71c27d5c63749278ad79a956c91c3d60624f0bd746c898c0573ebcc - C:/Users/hiroshi/cas2/llvm-project/build/tools/clang/test/ClangScanDeps/Output/include-tree.c.tmp/t.c
# | ^
# | <stdin>:4:98: note: possible intended match here
# | 116:1 C:/Users/hiroshi/cas2/llvm-project/build/tools/clang/test/ClangScanDeps/Output/include-tree.c.tmp\top.h llvmcas://28e0c6e56a4313a99d95f0d8f10fcd920eda0d15c4802eeb033392d314fb991d
# | ^
# |
#
Using regex matches seem fine for this test. Will see for the rest.
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.
I think regex alone won't work for json output because paths in jsons use double-backslashes (backslashes escaped inside json strings) like
"C:\\Users\\hiroshi\\cas2\\llvm-project\\build\\tools\\clang\\test\\ClangScanDeps\\Output\\modules-cas-context-hash.c.tmp/outputs\\EFTKLP18I1KM8EAL3GSZTFNU5\\Mod-EFTKLP18I1KM8EAL3GSZTFNU5.pcm"
whereas PREFIX is
C:/Users/hiroshi/cas2/llvm-project/build/tools/clang/test/ClangScanDeps/Output/modules-cas-context-hash.c.tmp
where the path prefix in PREFIX doesn't match, and it isn't easy to convert slashes or apply regex inside a variable like PREFIX.
I think I'd need to use sed for tests that check json output. If you see how to handle this with a regex, let me know.
# executed command: cat 'C:\Users\hiroshi\cas2\llvm-project\build\tools\clang\test\ClangScanDeps\Output\modules-cas-context-hash.c.tmp/result.json'
# executed command: 'c:\users\hiroshi\cas2\llvm-project\build\bin\filecheck.exe' 'C:\Users\hiroshi\cas2\llvm-project\clang\test\ClangScanDeps\modules-cas-context-hash.c' -DPREFIX=C:/Users/hiroshi/cas2/llvm-project/build/tools/clang/test/ClangScanDeps/Output/modules-cas-context-hash.c.tmp
# .---command stderr------------
# | C:\Users\hiroshi\cas2\llvm-project\clang\test\ClangScanDeps\modules-cas-context-hash.c:36:11: error: CHECK: expected string not found in input
# | // CHECK: "[[PREFIX]]/outputs/[[HASH]]/Mod-[[HASH]].pcm"
# | ^
# | <stdin>:118:27: note: scanning from here
# | "-fmodule-file-cache-key",
# | ^
# | <stdin>:118:27: note: with "PREFIX" equal to "C:/Users/hiroshi/cas2/llvm-project/build/tools/clang/test/ClangScanDeps/Output/modules-cas-context-hash.c.tmp"
# | "-fmodule-file-cache-key",
# | ^
# | <stdin>:118:27: note: with "HASH" equal to "EFTKLP18I1KM8EAL3GSZTFNU5"
# | "-fmodule-file-cache-key",
# | ^
# | <stdin>:118:27: note: with "HASH" equal to "EFTKLP18I1KM8EAL3GSZTFNU5"
# | "-fmodule-file-cache-key",
# | ^
# | <stdin>:119:122: note: possible intended match here
# | "C:\\Users\\hiroshi\\cas2\\llvm-project\\build\\tools\\clang\\test\\ClangScanDeps\\Output\\modules-cas-context-hash.c.tmp/outputs\\EFTKLP18I1KM8EAL3GSZTFNU5\\Mod-EFTKLP18I1KM8EAL3GSZTFNU5.pcm",
# | ^
if (is_style_windows(Style)) { | ||
// Canonicalize to backslahes | ||
InputPath.toVector(Storage); | ||
llvm::sys::path::make_preferred(Storage, Style); |
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.
Is this the difference? I don't quite understand why is this needed? Can you elaborate a bit?
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.
Yes, this is the slash canonicalization. The LLVM tests are written as if everything is forward slashes but the lit macros (like %/t) are backslashes on windows so we get mixed-slash paths (like C:\temp/t.c) and all-forward slahes from cdb (the paths from cdb jsons are all forward slashes) and the internal path handling in CachingOnDiskFileSystem/CASFileSystem support only the native slashes so I thought putting canonicalization at entries to CachingOnDiskFileSystem/CASFileSystem would be good. Did I understand the question right?
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.
This is the mixed path, makes sense. I wonder how does the existing tests work? There are lots of tests in clang/test/ClangScanDeps
that uses the %\t
patten and how does that work on windows?
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.
I think how these ClangScanDeps tests work is actually by using the sed to convert from backslahes to slashes like above so that the output from windows with backslashes would match the forward-slashes the llvm tests expect.
Not exactly but the sed command appear many times in the tests:
C:\Users\hiroshi\cas2\llvm-project\clang\test\ClangScanDeps>ls -l|wc -l
178
C:\Users\hiroshi\cas2\llvm-project\clang\test\ClangScanDeps>git grep sed|grep \\\\|wc -l
130
P1689.cppm:// RUN: | sed 's:\\\\\?:/:g' \
P1689.cppm:// RUN: | sed 's:\\\\\?:/:g' \
P1689.cppm:// RUN: | sed 's:\\\\\?:/:g' \
P1689.cppm:// RUN: | sed 's:\\\\\?:/:g' \
P1689.cppm:// RUN: | sed 's:\\\\\?:/:g' \
P1689.cppm:// RUN: | sed 's:\\\\\?:/:g' \
P1689.cppm:// RUN: | sed 's:\\\\\?:/:g' \
P1689.cppm:// RUN: | sed 's:\\\\\?:/:g' \
P1689.cppm:// RUN: | sed 's:\\\\\?:/:g' \
cas-fs-multiple-commands.c:// RUN: cat %t/deps.0.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
cas-fs-multiple-commands.c:// RUN: cat %t/deps.txt | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t -check-prefix=CHECK-LIBCLANG
cas-fs-multiple-commands.c:// RUN: cat %t/deps.1.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
diagnostics.c:// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
error.cpp:// RUN: cat %t/missing_tu.errs | sed 's:\\\\\?:/:g' | FileCheck %s --check-prefix=CHECK-MISSING-TU -DPREFIX=%/t
error.cpp:// RUN: cat %t/missing_header.errs | sed 's:\\\\\?:/:g' | FileCheck %s --check-prefix=CHECK-MISSING-HEADER -DPREFIX=%/t
generate-modules-path-args.c:// RUN: cat %t/deps.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t %s
generate-modules-path-args.c:// RUN: cat %t/deps_mt1.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t %s -check-prefix=DEPS_MT1
generate-modules-path-args.c:// RUN: cat %t/deps_mt2.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t %s -check-prefix=DEPS_MT2
generate-modules-path-args.c:// RUN: cat %t/deps_without.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t -check-prefix=WITHOUT %s
header-search-case-sensitivity.c:// RUN: clang-scan-deps -compilation-database=%t/cdb.json -format make -j 1 | sed 's:\\\\\?:/:g' | FileCheck %s
header-search-case-sensitivity.c:// RUN: clang-scan-deps -compilation-database=%t/cdb-rev.json -format make -j 1 | sed 's:\\\\\?:/:g' | FileCheck %s
header-search-pruning-transitive.c:// RUN: cat %t/results.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
header-search-pruning.cpp:// RUN: cat %t/result_a.json | sed 's:\\\\\?:/:g' | FileCheck --check-prefixes=CHECK_A %s
header-search-pruning.cpp:// RUN: cat %t/result_b.json | sed 's:\\\\\?:/:g' | FileCheck --check-prefixes=CHECK_B %s
header-search-pruning.cpp:// RUN: cat %t/result_ab.json | sed 's:\\\\\?:/:g' | FileCheck --check-prefixes=CHECK_AB %s
include-tree-multiple-commands.c:// RUN: cat %t/deps.0.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t -DSRC=%/t/src0 -DDST=%/t/dst0
include-tree-multiple-commands.c:// RUN: cat %t/deps.txt | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t -DSRC=%/t/src0 -DDST=%/t/dst0 -check-prefix=CHECK-LIBCLANG
include-tree-multiple-commands.c:// RUN: cat %t/deps.1.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t -DSRC=%/t/src1 -DDST=%/t/dst1
include-tree-prefix-mapping.c:// RUN: cat %t/deps.json | sed 's:\\\\\?:/:g' >> %t/full.txt
include-tree-with-pch.c:// RUN: cat %t/deps.json | sed 's:\\\\\?:/:g' >> %t/full.txt
include-tree.c:// RUN: cat %t/result1.txt | sed 's/\\/\//g' | FileCheck %s -DPREFIX=%/t
include-tree.c:// RUN: cat %t/result1.txt | sed 's/\\/\//g' | FileCheck %s -DPREFIX=%/t -check-prefix ORDER
include-tree.c:// RUN: cat %t/deps.json | sed 's:\\\\\?:/:g' >> %t/full.txt
include-tree.c:// RUN: cat %t/full.txt | sed 's/\\/\//g' | FileCheck %s -DPREFIX=%/t -DCLANG=%clang -check-prefix=FULL
line-directive.c:// RUN: cat %t/deps.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
link-libraries.c:// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t %s
modulemap-via-vfs.m:// RUN: cat %t.A.cc1.rsp | sed 's:\\\\\?:/:g' | FileCheck %s
modules-canononical-module-map-case.c:// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
modules-cas-context-hash.c:// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
modules-cas-full-by-mod-name.c:// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t %s
modules-cc1.cpp:// RUN: cat %t/result | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
modules-context-hash-ignore-macros.c:// RUN: cat %t/deps.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t %s
modules-context-hash-module-map-path.c:// RUN: cat %t/deps.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t %s
modules-context-hash-outputs.c:// RUN: cat %t/deps.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t %s
modules-context-hash-warnings.c:// RUN: cat %t/deps.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t %s
modules-context-hash.c:// RUN: cat %t/result_a.json %t/result_b.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t -check-prefix=CHECK
modules-context-hash.c:// RUN: cat %t/result_b.json %t/result_b2.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t -check-prefix=FLAG_ONLY
modules-debug-dir.c:// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck %s
modules-dep-args.c:// RUN: cat %t/result_cache.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t --check-prefixes=CHECK,CHECK_CACHE
modules-dep-args.c:// RUN: cat %t/result_build.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t --check-prefixes=CHECK,CHECK_BUILD
modules-dep-args.c:// RUN: cat %t/result_lazy.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t --check-prefixes=CHECK,CHECK_LAZY
modules-dep-args.c:// RUN: cat %t/result_eager.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t --check-prefixes=CHECK,CHECK_EAGER
modules-excluded-header.m:// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
modules-extern-submodule.c:// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
modules-extern-unrelated.m:// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
modules-fmodule-name-no-module-built.m:// RUN: cat %t.result | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t.dir --check-prefixes=CHECK %s
modules-full-by-mod-name.c:// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t %s
modules-full-named-modules.cppm:// RUN: | sed 's:\\\\\?:/:g' \
modules-full-named-modules.cppm:// RUN: | sed 's:\\\\\?:/:g' \
modules-full-named-modules.cppm:// RUN: | sed 's:\\\\\?:/:g' \
modules-full-named-modules.cppm:// RUN: | sed 's:\\\\\?:/:g' \
modules-full-named-modules.cppm:// RUN: | sed 's:\\\\\?:/:g' \
modules-full-named-modules.cppm:// RUN: | sed 's:\\\\\?:/:g' \
modules-full-named-modules.cppm:// RUN: | sed 's:\\\\\?:/:g' \
....<snip>
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.
Does clang-scan-deps also reproduce mixed separator kind? Both sed
and extra are all fragile on this. If clang-scan-deps also has the problem, we can propose a fix upstream for all the scanning tools and use that here.
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.
I think yes, as in the other comment thread, in the example of clang\test\ClangScanDeps\modules-cas-context-hash.c it outputs a mixed slash path:
"C:\\Users\\hiroshi\\cas2\\llvm-project\\build\\tools\\clang\\test\\ClangScanDeps\\Output\\modules-cas-context-hash.c.tmp/outputs\\EFTKLP18I1KM8EAL3GSZTFNU5\\Mod-EFTKLP18I1KM8EAL3GSZTFNU5.pcm"
which I think comes from a flag value:
-module-files-dir C:\Users\hiroshi\cas2\llvm-project\build\tools\clang\test\ClangScanDeps\Output\modules-cas-context-hash.c.tmp/outputs
which originates in -module files-dir %t/outputs
in the RUN line
// RUN: clang-scan-deps -compilation-database %t/cdb1.json -module-files-dir %t/outputs \
// RUN: -cas-path %t/cas1 -format experimental-include-tree-full \
// RUN: > %t/result.json
Here's the test log when it fails without the sed
# RUN: at line 11
c:\users\hiroshi\cas2\llvm-project\build\bin\clang-scan-deps.exe -compilation-database C:\Users\hiroshi\cas2\llvm-project\build\tools\clang\test\ClangScanDeps\Output\modules-cas-context-hash.c.tmp/cdb2.json -module-files-dir C:\Users\hiroshi\cas2\llvm-project\build\tools\clang\test\ClangScanDeps\Output\modules-cas-context-hash.c.tmp/outputs -cas-path C:\Users\hiroshi\cas2\llvm-project\build\tools\clang\test\ClangScanDeps\Output\modules-cas-context-hash.c.tmp/cas2 -format experimental-include-tree-full >> C:\Users\hiroshi\cas2\llvm-project\build\tools\clang\test\ClangScanDeps\Output\modules-cas-context-hash.c.tmp/result.json
# executed command: 'c:\users\hiroshi\cas2\llvm-project\build\bin\clang-scan-deps.exe' -compilation-database 'C:\Users\hiroshi\cas2\llvm-project\build\tools\clang\test\ClangScanDeps\Output\modules-cas-context-hash.c.tmp/cdb2.json' -module-files-dir 'C:\Users\hiroshi\cas2\llvm-project\build\tools\clang\test\ClangScanDeps\Output\modules-cas-context-hash.c.tmp/outputs' -cas-path 'C:\Users\hiroshi\cas2\llvm-project\build\tools\clang\test\ClangScanDeps\Output\modules-cas-context-hash.c.tmp/cas2' -format experimental-include-tree-full
# RUN: at line 16
cat C:\Users\hiroshi\cas2\llvm-project\build\tools\clang\test\ClangScanDeps\Output\modules-cas-context-hash.c.tmp/result.json | c:\users\hiroshi\cas2\llvm-project\build\bin\filecheck.exe C:\Users\hiroshi\cas2\llvm-project\clang\test\ClangScanDeps\modules-cas-context-hash.c -DPREFIX=C:/Users/hiroshi/cas2/llvm-project/build/tools/clang/test/ClangScanDeps/Output/modules-cas-context-hash.c.tmp
# executed command: cat 'C:\Users\hiroshi\cas2\llvm-project\build\tools\clang\test\ClangScanDeps\Output\modules-cas-context-hash.c.tmp/result.json'
# executed command: 'c:\users\hiroshi\cas2\llvm-project\build\bin\filecheck.exe' 'C:\Users\hiroshi\cas2\llvm-project\clang\test\ClangScanDeps\modules-cas-context-hash.c' -DPREFIX=C:/Users/hiroshi/cas2/llvm-project/build/tools/clang/test/ClangScanDeps/Output/modules-cas-context-hash.c.tmp
# .---command stderr------------
# | C:\Users\hiroshi\cas2\llvm-project\clang\test\ClangScanDeps\modules-cas-context-hash.c:37:11: error: CHECK: expected string not found in input
# | // CHECK: "[[PREFIX]]/outputs/[[HASH]]/Mod-[[HASH]].pcm"
# | ^
# | <stdin>:118:27: note: scanning from here
# | "-fmodule-file-cache-key",
# | ^
# | <stdin>:118:27: note: with "PREFIX" equal to "C:/Users/hiroshi/cas2/llvm-project/build/tools/clang/test/ClangScanDeps/Output/modules-cas-context-hash.c.tmp"
# | "-fmodule-file-cache-key",
# | ^
# | <stdin>:118:27: note: with "HASH" equal to "EFTKLP18I1KM8EAL3GSZTFNU5"
# | "-fmodule-file-cache-key",
# | ^
# | <stdin>:118:27: note: with "HASH" equal to "EFTKLP18I1KM8EAL3GSZTFNU5"
# | "-fmodule-file-cache-key",
# | ^
# | <stdin>:119:122: note: possible intended match here
# | "C:\\Users\\hiroshi\\cas2\\llvm-project\\build\\tools\\clang\\test\\ClangScanDeps\\Output\\modules-cas-context-hash.c.tmp/outputs\\EFTKLP18I1KM8EAL3GSZTFNU5\\Mod-EFTKLP18I1KM8EAL3GSZTFNU5.pcm",
# | ^
Here's the entire json output from this test: result.json
I think that clang-scan-deps doesn't modify slashes
Can you elaborate on the fix you're thinking of?
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.
I don't know enough about Windows to suggest a better fix but now I care less if you sed
or use regex
to match. I think this is a bigger problem that needs to be addressed separately. @compnerd any idea? How should we update clang-scan-deps to probably emit paths that aren't mix separators?
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.
I don't think that we have a funnel point that works well. We could canonicalise the path at the point of emission in clang-scan-deps
. I have a related concern that the VFS is going to fail to match paths due to the paths being in non-canonical form. We have a similar problem in swiftc
where paths are not canonicalised throughout. I have a patch up that partially improves the situation (and incidentally makes tests start passing), but I haven't been able to figure out one of the issues it stumbles upon with macOS.
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.
I think
- in the above attached example json file, mixed paths are in the
command-line
section (flag values, as verbatim). The paths infile-deps
andinput-file
are not mixed (though theinput-file
paths are forward-slash, not backslash). - as I understand,
clang-scan-deps
loads files into CAS throughCachingOnDiskFileSystem
andclang
accesses cached files throughCASFileSystem
and this PR adds canonicalization to them.
Does that help?
Clang :: ClangScanDeps/include-tree.c Clang :: ClangScanDeps/modules-cas-context-hash.c Clang :: ClangScanDeps/modules-cas-full-by-mod-name.c Clang :: ClangScanDeps/modules-cas-module-cache-hash.c Clang :: ClangScanDeps/modules-cas-trees-exclude-files-from-casfs.c Clang :: ClangScanDeps/modules-include-tree-implementation.c Clang :: ClangScanDeps/modules-include-tree-inferred.m Clang :: ClangScanDeps/modules-include-tree-missing-submodule.c Clang :: ClangScanDeps/modules-include-tree-pch-with-private.c Clang :: ClangScanDeps/modules-include-tree-submodules.c Clang :: ClangScanDeps/modules-include-tree-with-pch.c Clang :: ClangScanDeps/modules-include-tree-working-directory.c Clang :: ClangScanDeps/modules-include-tree.c Clang :: Index/Core/scan-deps-cas.m LLVM :: tools/llvm-cas/merge.test LLVM :: tools/llvm-cas/print-id.test LLVM :: tools/llvm-cas/validation.test
Updated per discussion so far |
Fix the following CAS-related tests on Windows
Clang :: ClangScanDeps/include-tree.c
Clang :: ClangScanDeps/modules-cas-context-hash.c
Clang :: ClangScanDeps/modules-cas-full-by-mod-name.c
Clang :: ClangScanDeps/modules-cas-module-cache-hash.c
Clang :: ClangScanDeps/modules-cas-trees-exclude-files-from-casfs.c
Clang :: ClangScanDeps/modules-include-tree-implementation.c
Clang :: ClangScanDeps/modules-include-tree-inferred.m
Clang :: ClangScanDeps/modules-include-tree-missing-submodule.c
Clang :: ClangScanDeps/modules-include-tree-pch-with-private.c
Clang :: ClangScanDeps/modules-include-tree-submodules.c
Clang :: ClangScanDeps/modules-include-tree-with-pch.c
Clang :: ClangScanDeps/modules-include-tree-working-directory.c
Clang :: ClangScanDeps/modules-include-tree.c
Clang :: Index/Core/scan-deps-cas.m
LLVM :: tools/llvm-cas/merge.test
LLVM :: tools/llvm-cas/print-id.test
LLVM :: tools/llvm-cas/validation.test