[llvm] [lit] Are all RUN lines skipped in windows cmd? (PR #65242)

Mon Sep 4 03:45:03 PDT 2023

mstorsjo wrote:

I haven't been so much involved in the recent Lit changes, and it's been a while since I poked at these things, so it took me a little bit of fiddling around to get back on track about what the actual status quo is here. I'll try to summarize my findings, please doublecheck if I've missed something:

Status quo
---
- `ShTest` defaults to `execute_external=False` (i.e., internal shell) if no parameter has been passed: https://github.com/llvm/llvm-project/blob/llvmorg-18-init/llvm/utils/lit/lit/formats/shtest.py#L22
- The LLVM testsuite initializes this based on `llvm_config.use_lit_shell` - https://github.com/llvm/llvm-project/blob/llvmorg-18-init/llvm/test/lit.cfg.py#L21. The vast majority of testsuites in LLVM also copies this behaviour.
- The LLVM testsuite defaults to `self.use_lit_shell = False`, except that on Windows it defaults to `self.use_lit_shell = True`. This can be overridden with the environment variable `LIT_USE_INTERNAL_SHELL`. https://github.com/llvm/llvm-project/blob/llvmorg-18-init/llvm/utils/lit/lit/llvm/config.py#L23-L52 I.e. if people don't explicitly opt in to it by setting `LIT_USE_INTERNAL_SHELL=0`, people will be using the internal shell on Windows
- The OpenMP testsuite initializes `ShTest` with the defaults, i.e. always using the internal shell: https://github.com/llvm/llvm-project/blob/llvmorg-18-init/openmp/runtime/test/lit.cfg#L42
- compiler-rt's tests have custom setup logic that doesn't reuse the same setup as most other tools with `llvm_config.use_lit_shell`, but has essentially equivalent logic here: https://github.com/llvm/llvm-project/blob/llvmorg-18-init/compiler-rt/test/lit.common.cfg.py#L113-L128
- The libcxx testsuite (which is reused for libcxxabi and libunwind) defaults to the internal shell, without any option for overriding it: https://github.com/llvm/llvm-project/blob/llvmorg-18-init/libcxx/utils/libcxx/test/format.py#L384-L387
- If using an external shell for running tests, this will be bash, if bash was found in the path - if not, it will be the win32 cmd.exe: https://github.com/llvm/llvm-project/blob/llvmorg-18-init/llvm/utils/lit/lit/TestRunner.py#L1068-L1069

As additional context: The libcxx testsuite used to use the external shell a few years ago, but this was problematic for running the tests on Windows, it didn't really work right there (I don't have exact details). This was changed in 39bbfb77264a4a7a216921c2b70a30ba0f27eb56 - before that, running the libcxx tests required making sure that `bash` was available in path.

I.e., unless jumping through hoops, none of these testsuites really run things with the win32 cmd.exe.

Breakage
---
Running things with win32 cmd.exe as external shell did use to work, but currently doesn't. This actually seems to have broken in 1041a9642ba035fd2685f925911d705e8edf5bb0 already. If running a simple llvm shell test with `LIT_USE_INTERNAL_SHELL=0`, before that, I got a successful run:
```
Script:
--
echo 'RUN: at line 3' > nul &&   c:\code\llvm-project\llvm\build\bin\llvm-mc.exe -triple aarch64-pc-win32 -filetype=obj C:/code/llvm-project/llvm/test/MC/AArch64/seh.s | c:\code\llvm-project\llvm\build\bin\llvm-readobj.exe -S -r -u - | c:\code\llvm-project\llvm\build\bin\filecheck.exe C:/code/llvm-project/llvm/test/MC/AArch64/seh.s
echo 'RUN: at line 8' > nul &&   c:\code\llvm-project\llvm\build\bin\llvm-mc.exe -triple aarch64-pc-win32 -filetype=asm C:/code/llvm-project/llvm/test/MC/AArch64/seh.s | c:\code\llvm-project\llvm\build\bin\llvm-mc.exe -triple aarch64-pc-win32 -filetype=obj - | c:\code\llvm-project\llvm\build\bin\llvm-readobj.exe -S -r -u - | c:\code\llvm-project\llvm\build\bin\filecheck.exe C:/code/llvm-project/llvm/test/MC/AArch64/seh.s
--
Exit Code: 0
```

After that commit, it instead breaks like this:
```
Script:
--
echo 'RUN: at line 3' > nul &&
echo 'RUN: at line 8' > nul &&
--
Exit Code: 255

Command Output (stderr):
--
The syntax of the command is incorrect.
```

It further breaks at 64d19542e78a43edb7ae26ea6762a2b1c360a916 as you've noticed, since the RUN lines essentially just become no-op goto labels.
```
Script:
--
: 'RUN: at line 3';   c:\code\llvm-project\llvm\build\bin\llvm-mc.exe -triple aarch64-pc-win32 -filetype=obj C:/code/llvm-project/llvm/test/MC/AArch64/seh.s | c:\code\llvm-project\llvm\build\bin\llvm-readobj.exe -S -r -u - | c:\code\llvm-project\llvm\build\bin\filecheck.exe C:/code/llvm-project/llvm/test/MC/AArch64/seh.s
: 'RUN: at line 8';   c:\code\llvm-project\llvm\build\bin\llvm-mc.exe -triple aarch64-pc-win32 -filetype=asm C:/code/llvm-project/llvm/test/MC/AArch64/seh.s | c:\code\llvm-project\llvm\build\bin\llvm-mc.exe -triple aarch64-pc-win32 -filetype=obj - | c:\code\llvm-project\llvm\build\bin\llvm-readobj.exe -S -r -u - | c:\code\llvm-project\llvm\build\bin\filecheck.exe C:/code/llvm-project/llvm/test/MC/AArch64/seh.s
--
Exit Code: 0
```
(It prints this even if injecting an error in the test.)

Furthermore, 09b6e457d91ce84088e6e21783913e5f1e5bd227 changes this case further, making it just print this:
```
PASS: LLVM :: MC/AArch64/seh.s (1 of 1)
Exit Code: 0
```
(Even when executing with `-a -v -v`.)

Separate side note; running the LLVM testsuite with bash as external executor (if it happens to be available in path) also seems to fail:
```
Exit Code: 127

Command Output (stderr):
--
+ : 'RUN: at line 3'
+ c:codellvm-projectllvmbuildbinllvm-mc.exe -triple aarch64-pc-win32 -filetype=obj C:/code/llvm-project/llvm/test/MC/AArch64/seh.s
C:\code\\llvm-project\llvm\build\test\MC\AArch64\Output\seh.s.script: line 1: c:codellvm-projectllvmbuildbinllvm-mc.exe: command not found
+ c:codellvm-projectllvmbuildbinllvm-readobj.exe -S -r -u -
C:\code\\llvm-project\llvm\build\test\MC\AArch64\Output\seh.s.script: line 1: c:codellvm-projectllvmbuildbinllvm-readobj.exe: command not found
+ c:codellvm-projectllvmbuildbinfilecheck.exe C:/code/llvm-project/llvm/test/MC/AArch64/seh.s
C:\code\\llvm-project\llvm\build\test\MC\AArch64\Output\seh.s.script: line 1: c:codellvm-projectllvmbuildbinfilecheck.exe: command not found

--
```
(This seems to have been broken throughout this time period at least. However before 39bbfb77264a4a7a216921c2b70a30ba0f27eb56, when libcxx tests were using the external executor, they did work when executed with bash as external executor.)

Conclusion
---
It certainly looks like this configuration is severely broken. I'm not aware of any parts of the configurations that use it by default, and even then, it's generally problematic, so I'm not sure if it's worth spending effort on fixing. (The suggestion that was made somewhere around these discussions, to move most things towards using the internal shell even on unix, sounds to me like a good direction in general .)

That said; for echoing executed commands, when using the internal executor, it's probably very valuable if they're printed in a form that is easy to copypaste and execute in cmd.exe (where possible - at least for the majority of simple cases).

https://github.com/llvm/llvm-project/pull/65242