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

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 <http://llvm.org/docs/GettingStartedVS.html#software> 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?
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160816/f7830a57/attachment.html>

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
>

On Tue, Aug 16, 2016 at 9:19 AM, 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 <http://llvm.org/docs/GettingStartedVS.html#software> 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.
>
Or implement the shutil as a python script based on python's shutil?




> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160816/1f258b9f/attachment.html>

Zachary Turner via llvm-dev <llvm-dev at lists.llvm.org>
writes:
> 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:
Anecdotally, most of the times I've seen people add "REQUIRES:
shell" on
a test they really meant "disable on windows". I think/hope that's
usually temporary while they investigate problems, but it might explain
why so many tests say they require shell when they don't.
> // 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.
To some degree, we already do this. See for example r231017, which adds
'cd' to lit's "internal shell". Adding some more common
things here
seems reasonable, as long as they aren't too complicated.
> 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:
>From a quick grep, I'm not even convinced one test in llvm uses awk -
there's a test (X86/hoist-spill) with a comment that has a shell
pipeline including awk, but it isn't in a RUN: line at all :/

On Tue, Aug 16, 2016 at 12: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?
I would *love* it if we got rid of the GnuWin32 dependency! Making lit
tests more uniform is also a really great win, but anything we can do
to reduce our dependencies on Windows is a win, and as you pointed
out, this particular one is complicated and comes with downsides when
you install it.

~Aaron
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>

>
> Anecdotally, most of the times I've seen people add "REQUIRES:
shell" on
> a test they really meant "disable on windows". I think/hope
that's
> usually temporary while they investigate problems, but it might explain
> why so many tests say they require shell when they don't.
>
>
In at least one case I can think of
("clang/test/Format/style-on-command-line.cpp") the "REQUIRES:
shell" is
there to work around annoying intermittent failures on Windows where
'rm'
was failing with permission denied errors just occasionally enough to be a
problem.  I've definitely seen this happen in a number of other places
previously although I can't think whether this affects any other lit tests
off-hand, but I know that personally I've had to implement 'robust
rm'
(basically, keep on trying until the error goes away on its own) on Windows
in a number of different systems to work around this sort of problem
before.  Replacing gnuwin32 with something where we could easily implement
something like 'robust <cmd>' versions for Windows when we
encounter these
sorts of issues would be fantastic, so I'm definitely in favour of removing
the dependency.

-Greg
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160816/7c6d5ea2/attachment.html>

On 16 August 2016 at 17:19, Zachary Turner via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> 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.
How about just using busybox-w32 for find, grep, sed, and other
standard UNIX commands https://github.com/rmyorston/busybox-w32. It's
a single exe < 500KiB.

Alex

If you went this route, you'd really want toybox, which is BSD licensed,
and not written by people with a penchant for litigation :)
www.landley.net/toybox/about.html


On Tue, Aug 16, 2016 at 12:39 PM, Alex Bradbury via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> On 16 August 2016 at 17:19, Zachary Turner via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
> > 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.
>
> How about just using busybox-w32 for find, grep, sed, and other
> standard UNIX commands https://github.com/rmyorston/busybox-w32. It's
> a single exe < 500KiB.
>
> Alex
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160816/ec927e68/attachment.html>

I had considered this as well, and someone also asked the same question
offline.  Here's the response I gave:
---
The one thing that keeps me from being totally on board is that as part of
this, I want to encourage people to think of ways to write simpler tests.
Do we really need all this crap running sed, awk, grep, piping through 4
different intermediate processes, copying files around, and more?  Maybe at
that point you need to consider adding a new command line option to your
tool specificially make it easier to test.  Or do soemthing differently.
Having a FULL command set available means people will just do whatever
crazy thing they can think of instead of properly designing a test.  A good
example of this is the clang-tidy test I mentioned in the original post.  I
added an additional command line option to clang-tidy and rewrote that
entire test purely with run lines
<https://reviews.llvm.org/D23533#56e9c922>
.
I think people should only be doing really simple shell activity from tests.
--

I agree it's much simpler.  On the other hand, maybe if we end up
implementing all this stuff directly into lit it's a moot point anyway.
Implementing it in lit has the added advantage that it doesn't involve
invoking a new process for every command, which is major performance
bottleneck on windows when running the test suite.

On Tue, Aug 16, 2016 at 12:39 PM Alex Bradbury <asb at asbradbury.org>
wrote:
> On 16 August 2016 at 17:19, Zachary Turner via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
> > 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.
>
> How about just using busybox-w32 for find, grep, sed, and other
> standard UNIX commands https://github.com/rmyorston/busybox-w32. It's
> a single exe < 500KiB.
>
> Alex
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160816/9e5481ab/attachment.html>