[llvm-dev] RFC: A cross platform way of using shell commands in lit tests

Tue Aug 16 09:42:57 PDT 2016

I'm all for both:
Removing all meaningless instances of requires: shell, and remove
calls to external programs if they're not needed;
Removing reliance on anything that "requires: shell".

Annoying things like creating/removing directories would become easier
to always get right, and have them work everywhere.
There are harder things to support, like backticks in some tests:
http://llvm.org/klaus/compiler-rt/blob/master/test/asan/TestCases/Posix/coverage.cc#L-5
This one is especially annoying, as any test you make that would end
up telling you if you have the "shell" feature available will apply to
the host, not the target.

Unsure if you'll be able to do the "cd" command in shutil, but we
already deal with it in lit: _executeShCmd in
utils/lit/lit/TestRunner.py:148
I'd rather have lit do most of it than doing a totally separate
utility for the shell commands which has a probability of being
misused and have people start relying on it as a compatibility layer.
Only if it doesn't become a big burden to keep everything in lit, of
course. I'm not opposed to something like you suggested (llvm-shutil).

Thank you,
 Filipe

On Tue, Aug 16, 2016 at 5:19 PM, Zachary Turner via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> I see many tests that are prefixed with "requires: shell".  A quick search
> over the codebase shows 101 tests that contain this directive.  This
> basically means there are 101 tests that say "I don't support running on
> Windows".
>
> I would like to get this number to 0.
>
> Ironically, many of these tests can be made to run on Windows simply by
> removing the requires: shell line.  As in, it doesn't *actually* require a
> shell.  This is because the meaning of requires shell is not intuitive.
> GnuWin32 is already a required dependency, so any shell command that you are
> familiar with probably exists on Windows already.  For example, consider the
> following test in clang-tidy:
>
> // REQUIRES: shell
> // RUN: mkdir -p %T/compilation-database-test/include
> // RUN: mkdir -p %T/compilation-database-test/a
> // RUN: mkdir -p %T/compilation-database-test/b
> // RUN: echo 'int *AA = 0;' > %T/compilation-database-test/a/a.cpp
> // RUN: echo 'int *AB = 0;' > %T/compilation-database-test/a/b.cpp
> // RUN: echo 'int *BB = 0;' > %T/compilation-database-test/b/b.cpp
> // RUN: echo 'int *BC = 0;' > %T/compilation-database-test/b/c.cpp
> // RUN: echo 'int *HP = 0;' > %T/compilation-database-test/include/header.h
> // RUN: echo '#include "header.h"' > %T/compilation-database-test/b/d.cpp
> // RUN: sed 's|test_dir|%T/compilation-database-test|g'
> %S/Inputs/compilation-database/template.json > %T/compile_commands.json
> // RUN: clang-tidy --checks=-*,modernize-use-nullptr -p %T
> %T/compilation-database-test/b/not-exist -header-filter=.*
> // RUN: clang-tidy --checks=-*,modernize-use-nullptr -p %T
> %T/compilation-database-test/a/a.cpp %T/compilation-database-test/a/b.cpp
> %T/compilation-database-test/b/b.cpp %T/compilation-database-test/b/c.cpp
> %T/compilation-database-test/b/d.cpp -header-filter=.* -fix
> // RUN: FileCheck -input-file=%T/compilation-database-test/a/a.cpp %s
> -check-prefix=CHECK-FIX1
> // RUN: FileCheck -input-file=%T/compilation-database-test/a/b.cpp %s
> -check-prefix=CHECK-FIX2
> // RUN: FileCheck -input-file=%T/compilation-database-test/b/b.cpp %s
> -check-prefix=CHECK-FIX3
> // RUN: FileCheck -input-file=%T/compilation-database-test/b/c.cpp %s
> -check-prefix=CHECK-FIX4
> // RUN: FileCheck -input-file=%T/compilation-database-test/include/header.h
> %s -check-prefix=CHECK-FIX5
>
> This test does not require a shell!  Remove the line (and change the %S and
> %T on the sed line to %/S and %/T) and this test already works on Windows!
>
> I have inspected around 20 tests so far that claim to require shell, and so
> far only about 3 of them are more complicated than removing the line.
>
> "Great, so just remove the lines!  Why are you posting this?"
>
> Well, It would be nice if we could remove the GnuWin32 dependency as well.
> Lots of tests use grep, sed, cd, ln, or whatever.  Anyone who's ever tried
> downloading and installing GnuWin32 knows what a pain it is.  It installs
> programs called, for example, "link" which interfere with standard windows
> tools, and puts them in your path.  There are other examples of this.  And
> it's also HUGE.  And nobody can ever figure out how to download it
> correctly.  Plus there are some minor differences in the tools behavior.
>
> I'm proposing that we replace shell commands in tests with a tool that we
> create which implements the most common shell commands and options.
>
> it doesn't need to be a complete implementation of the commands.  Nobody
> cares about echo -n, or sed -u, for example.  We could keep it simple with
> the most common commands, most of which would be trivially implementable
> using the llvm Support library already.  Complicated commands like awk, for
> example, could be absent.  Chances are you need to simplify your test if
> you're using something like that (presently exactly 1 test in all of LLVM
> uses it).  What I'm imagining is something like this:
>
> D:\src\llvmbuild\ninja>bin\llvm-shutil --help
> OVERVIEW: LLVM Shell Utility Program
>
> USAGE: llvm-shutil.exe [subcommand] [options]
>
> SUBCOMMANDS:
>
>   cd - Change current working directory
>   mkdir   - Make a directory
>   ln - Create a link
>   grep - Search for content in a file
>   sort - Sort lines
>   // etc, rest of commands go here.
>
>   Type "llvm-shutil.exe <subcommand> -help" to get more help on a specific
> subcommand
>
> OPTIONS:
>
> Generic Options:
>
>   -stdout=<file>   - Redirect stdout to the specified file
>   -stderr=<file>    - Redirect stderr to the specified file
>   -stdin=<file>     - Read stdin from the specified file
>   -help      - Display available options (-help-hidden for more)
>   -help-list - Display list of available options (-help-list-hidden for
> more)
>   -version   - Display the version of this program
>
>
> With something like this, we could kill two birds with one stone.  GnuWin32
> dependency is gone, and there is no more confusion about what "requires:
> shell" actually means.
>
> The reason we wrote lit the way we did is so that everyone could be speaking
> the same language when they write tests, no matter which platform they're
> on.  So it would be nice to eliminate the last area where this is still not
> the case.
>
> thoughts?
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>