[libc++] Use the internal Lit shell to run the tests

[ldionne]

Summary

This makes the libc++ tests more portable -- almost all of them should
now work on Windows, except for some tests that assume a shell is
available on the target. We should probably provide a way to exclude
those anyway for the purpose of running tests on embedded targets.

[arichardson]

libcxx/test/libcxx/selftest/shell-escape.sh.cpp

18

Is this change required because the internal shell doesn't support !?

It might be useful to add ! support to the integrated shell, as that would allow avoiding some process spawns for the not utility.

[ldionne]

libcxx/test/libcxx/selftest/shell-escape.sh.cpp

18

Yes, that's correct. I implemented not as a shell builtin in https://reviews.llvm.org/D89493.

[ldionne]

Rebase onto main

[mstorsjo]

Awesome!

For some reason, when I've tried running this, in addition to run-error.sh.cpp and run-success.sh.cpp, using the internal shell also broke libcxx/test/libcxx/selftest/additional_compile_flags/substitutes-in-compil
e-flags.sh.cpp in the Windows CI configuration - I haven't figured out why yet.

libcxx/utils/libcxx/test/format.py

242

These changes shouldn't be necessary any longer, ditto below on line 252 and 283

[ldionne]

Rebase onto main and remove unnecessary nots

[ldionne]

Rebase onto main without stray change for adding not to the Lit shell (it has already been added)

[mstorsjo]

Ok, so there's just one test still that fails on Windows in this configuration, libcxx/selftest/additional_compile_flags/substitutes-in-compile-flags.sh.cpp. I can try to have a look into what the issue is what that one.

[mstorsjo]

In D89495#2717725, @mstorsjo wrote:

Ok, so there's just one test still that fails on Windows in this configuration, libcxx/selftest/additional_compile_flags/substitutes-in-compile-flags.sh.cpp. I can try to have a look into what the issue is what that one.

Ok, so the reason here is that substitutes-in-compile-flags.sh.cpp greps for %t in various incarnations. If running with an internal shell, %t is expanded to a path with backslashes, and when that is passed to grep, those backslashes would have to be escaped. I don't think we'd be missing out on much by just adding // UNSUPPORTED: windows on that test, instead of trying to fiddle with backslash-escaping of paths for grep.

With that in place, the tests can be run without bash as executor. However there's a couple other tests that manually invoke the bash executable as part of the test too (which probably also would fail the same for embedded targets; the tests are libcxx/selftest/remote-substitutions.sh.cpp, std/input.output/iostream.objects/narrow.stream.objects/cin.sh.cpp and std/input.output/iostream.objects/wide.stream.objects/wcin.sh.cpp), and a couple dozen more that require generic unix-like tools like cp to exist. (The generic LLVM tests also require such unix tools like cp and grep, but also tries to locate a usable version of such tools automatically, cf https://github.com/llvm/llvm-project/blob/llvmorg-12.0.0/llvm/utils/lit/lit/llvm/config.py#L129-L156.)

That said, we're soon getting the CI images upgraded to always have bash available in PATH, but this patch probably is worthwhile anyway.

[ldionne]

Thanks a lot for the analysis @mstorsjo . For the other tests that use bash explicitly, we'll need a way to fix those when running on embedded targets anyway, so punting to that moment sounds acceptable to me.

[ldionne]

Add UNSUPPORTED on Windows

[mstorsjo]

LGTM, thanks!

libcxx/test/libcxx/selftest/sh.cpp/run-error.sh.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // XFAIL: *
 
 // Make sure the test DOES NOT pass if it fails to run
 
-// RUN: exit 1
+// RUN: false

libcxx/test/libcxx/selftest/sh.cpp/run-success.sh.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // Make sure the test passes if it succeeds to run
 
-// RUN: exit 0
+// RUN: true

libcxx/test/libcxx/selftest/shell-escape.sh.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // Make sure that arguments of the %{exec} substitution are shell-escaped
 // properly. If that wasn't the case, the command would fail because the
 // shell would look for a matching `"`.
 
 // RUN: %{exec} echo '"'
-
-// Also make sure that we don't escape Shell builtins like `!`, because the
-// shell otherwise thinks it's a command and it can't find it.
-
-// RUN: ! false

libcxx/test/libcxx/selftest/shell-quote-pipes.sh.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-// Make sure manually escaped pipes are handled properly by the shell.
-// Specifically, we want to make sure that if we escape a pipe after %{exec},
-// it gets executed by the %{exec} substitution, as opposed to the result of
-// the %{exec} substitution being piped into the following command.
+// Make sure that quoted pipes are handled properly by the shell. Specifically,
+// we want to make sure that if we quote a pipe after %{exec}, it gets executed
+// by the %{exec} substitution, as opposed to the result of the %{exec}
+// substitution being piped into the following command.
 //
 // This is a bit tricky to test. To test this, we basically want to ensure
 // that both sides of the pipe are executed inside %{exec}. When we're inside
 // %{exec}, the one difference we can rely on is that we're in a temporary
 // directory with all file dependencies satisfied, so that's what we use.
 
 // RUN: touch %t.foobar
-// RUN: %{exec} echo \| ls > %t.out
+// RUN: %{exec} echo "|" ls > %t.out
 // RUN: grep -e ".foobar" %t.out

libcxx/test/std/input.output/iostream.objects/narrow.stream.objects/cin.sh.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // XFAIL: libcpp-has-no-stdin
 
 // <iostream>
 
 // istream cin;
 
 // RUN: %{build}
-// RUN: %{exec} echo "123" \| %t.exe > %t.out
+// RUN: %{exec} "echo" "123" "|" "%t.exe" > %t.out
 // RUN: grep -e 'The number is 123!' %t.out
 
 #include <iostream>
 #include <cassert>
 
 #include "test_macros.h"
 
 int main(int, char**)

libcxx/test/std/input.output/iostream.objects/wide.stream.objects/wcin.sh.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // XFAIL: libcpp-has-no-stdin
 
 // <iostream>
 
 // istream wcin;
 
 // RUN: %{build}
-// RUN: %{exec} echo "123" \| %t.exe > %t.out
+// RUN: %{exec} "echo" "123" "|" "%t.exe" > %t.out
 // RUN: grep -e 'The number is 123!' %t.out
 
 #include <iostream>
 #include <cassert>
 
 #include "test_macros.h"
 
 int main(int, char**)

libcxx/utils/libcxx/test/format.py

- preamble
         A list of commands to perform before any command in the test.
         These commands can contain unexpanded substitutions, but they
         must not be of the form 'RUN:' -- they must be proper commands
         once substituted.
     """
 
     # Get the default substitutions
     tmpDir, tmpBase = _getTempPaths(test)
-    useExternalSh = True
-    substitutions = lit.TestRunner.getDefaultSubstitutions(test, tmpDir, tmpBase,
-                                                           normalize_slashes=useExternalSh)
+    substitutions = lit.TestRunner.getDefaultSubstitutions(test, tmpDir, tmpBase)
 
     # Check base substitutions and add the %{build} and %{run} convenience substitutions
     _checkBaseSubstitutions(substitutions)
     substitutions.append(('%{build}', '%{cxx} %s %{flags} %{compile_flags} %{link_flags} -o %t.exe'))
     substitutions.append(('%{run}', '%{exec} %t.exe'))
 
     # Parse the test file, including custom directives
     additionalCompileFlags = []
def execute(self, test, litConfig):
             return self._executeShTest(test, litConfig, steps)
         elif filename.endswith('.compile.pass.cpp'):
             steps = [
                 "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} -fsyntax-only"
             ]
             return self._executeShTest(test, litConfig, steps)
         elif filename.endswith('.compile.fail.cpp'):
             steps = [
-                "%dbg(COMPILED WITH) ! %{cxx} %s %{flags} %{compile_flags} -fsyntax-only"
+                "%dbg(COMPILED WITH) not %{cxx} %s %{flags} %{compile_flags} -fsyntax-only"
             ]
             return self._executeShTest(test, litConfig, steps)
         elif filename.endswith('.link.pass.cpp'):
             steps = [
                 "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} %{link_flags} -o %t.exe"
             ]
             return self._executeShTest(test, litConfig, steps)
         elif filename.endswith('.link.fail.cpp'):
             steps = [
                 "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} -c -o %t.o",
-                "%dbg(LINKED WITH) ! %{cxx} %t.o %{flags} %{link_flags} -o %t.exe"
+                "%dbg(LINKED WITH) not %{cxx} %t.o %{flags} %{link_flags} -o %t.exe"
             ]
             return self._executeShTest(test, litConfig, steps)
         elif filename.endswith('.run.fail.cpp'):
             steps = [
                 "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} %{link_flags} -o %t.exe",
-                "%dbg(EXECUTED AS) %{exec} ! %t.exe"
+                # Note: This is technically incorrect -- we should instead run
+                #       `not` inside the executor, but that requires the target
+                #       to be able to run `not`, which isn't always the case.
+                "%dbg(EXECUTED AS) not %{exec} %t.exe"
             ]
             return self._executeShTest(test, litConfig, steps)
         elif filename.endswith('.verify.cpp'):
             if not supportsVerify:
                 return lit.Test.Result(lit.Test.UNSUPPORTED,
                     "Test {} requires support for Clang-verify, which isn't supported by the compiler".format(test.getFullName()))
             steps = [
                 # Note: Use -Wno-error to make sure all diagnostics are not treated as errors,
def execute(self, test, litConfig):
         # for backwards compatibility with the test suite.
         elif filename.endswith('.fail.cpp'):
             if supportsVerify:
                 steps = [
                     "%dbg(COMPILED WITH) %{{cxx}} %s %{{flags}} %{{compile_flags}} -fsyntax-only -Wno-error {}".format(VERIFY_FLAGS)
                 ]
             else:
                 steps = [
-                    "%dbg(COMPILED WITH) ! %{cxx} %s %{flags} %{compile_flags} -fsyntax-only"
+                    "%dbg(COMPILED WITH) not %{cxx} %s %{flags} %{compile_flags} -fsyntax-only"
                 ]
             return self._executeShTest(test, litConfig, steps)
         else:
             return lit.Test.Result(lit.Test.UNRESOLVED, "Unknown test suffix for '{}'".format(filename))
 
     # Utility function to add compile flags in lit.local.cfg files.
     def addCompileFlags(self, config, *flags):
         string = ' '.join(flags)
         config.substitutions = [(s, x + ' ' + string) if s == '%{compile_flags}' else (s, x) for (s, x) in config.substitutions]
 
     def _executeShTest(self, test, litConfig, steps):
         if test.config.unsupported:
             return lit.Test.Result(lit.Test.UNSUPPORTED, 'Test is unsupported')
 
         script = parseScript(test, steps)
         if isinstance(script, lit.Test.Result):
             return script
 
         if litConfig.noExecute:
             return lit.Test.Result(lit.Test.XFAIL if test.isExpectedToFail() else lit.Test.PASS)
         else:
             _, tmpBase = _getTempPaths(test)
-            useExternalSh = True
+            useExternalSh = False
             return lit.TestRunner._runShTest(test, litConfig, useExternalSh, script, tmpBase)