[PATCH] D11960: [windows] Always use the lit shell on Windows, even if bash is present

Reid Kleckner via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 11 15:39:28 PDT 2015


rnk added inline comments.

================
Comment at: test/lit.common.cfg:15
@@ -17,1 +14,3 @@
+# Setup test format. Use bash on Unix and the lit shell on Windows.
+execute_external = (not sys.platform in ['win32'])
 config.test_format = lit.formats.ShTest(execute_external)
----------------
filcab wrote:
> I would prefer if we could test the *target*, instead of the *host*.
> I can always work around it by adding to the condition, but still. This looks fragile to me.
> If we have a bash-like shell in the path, should we try it, and set `execute_external`?
This is inconsistent with LLVM and Clang, and is exactly what I want to disable.

================
Comment at: test/lit.common.cfg:118
@@ +117,3 @@
+else:
+  config.available_features.add('posix')
+
----------------
filcab wrote:
> Please name them host-windows and host-posix, as these features are about the host.
> If you're running Android tests on a Windows host, for example, you won't run tests which REQUIRES: host-posix. But you should be able to run tests requiring posix functions, since they will run on a posix target.
> 
Sure, that seems reasonable.

================
Comment at: test/ubsan/TestCases/TypeCheck/misaligned.cpp:3
@@ +2,3 @@
+// which throws away DWARF debug info.
+// REQUIRES: posix
+//
----------------
filcab wrote:
> From the comment above, it seems this should be an `XFAIL` when we have *both* `host-windows` and "target Windows", no?
What this test really requires is symbols. We could make a "symbols" feature generally available, and then remove it when the MSVC linker is in use. Everything else generally works.


http://reviews.llvm.org/D11960





More information about the llvm-commits mailing list