[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 ↗

(On Diff #298456)

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 ↗

(On Diff #298456)

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

241

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/additional_compile_flags/substitutes-in-compile-flags.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
 //
 //===----------------------------------------------------------------------===//
 
+// This test greps for %t, which is expanded to a path with backslashes. When
+// that is passed to grep, those backslashes would have to be escaped, which we
+// don't do right now.
+// UNSUPPORTED: windows
+
 // Make sure that substitutions are performed inside additional compiler flags.
 
 // ADDITIONAL_COMPILE_FLAGS: -I %t.1
 // ADDITIONAL_COMPILE_FLAGS: -isystem %t.2 , -isysroot %t.3
 // RUN: echo "%{compile_flags}" | grep -e '-I %t.1 -isystem %t.2 -isysroot %t.3'

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/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"
             ]
             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'):
def _executeShTest(self, test, litConfig, steps):
         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)