This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] [test] Tweak a regex pattern to accommodate for quirks in MSYS based grep
AbandonedPublic

Authored by mstorsjo on Mar 25 2021, 4:50 AM.

Details

Reviewers
rnk
Summary

Currently on Windows, 'not' is a regular win32 executable, which
parses command line parameters according to the canonical rules of
the platform. 'grep' can be a tool running either on the regular
win32 runtime, or on the MSYS runtime. If based on the MSYS runtime,
it parses the command line parameters quite significantly differently.

When 'not' invokes 'grep', it quotes the arguments differently than
how lit's TestRunner does, making this particular case of regex
pattern work with MSYS grep when invoked by 'not', but fails if
invoked by TestRunner directly.

(If TestRunner is modified to quote arguments like 'not' does, a
whole bunch of other tests fail instead though.)

In this particular case, TestRunner doesn't quote the argument
^[a-zA-Z0-9_.\-]\+$ as it doesn't believe it needs to. The
'[' char in the argument trigger MSYS's argument globbing [1],
which also causes it to interpret the single backslashes as
escapes, making those backslashes vanish.

If we make TestRunner quote arguments if an argument contains a
backslash (which is what the 'not' executable does, with quoting
logic in llvm/lib/Support/Windows/Process.inc), then arguments with
double backslashes end up shrunk to a single backslash (contrary to
regular windows argument quoting rules), breaking a whole bunch of
other tests.

This was the cause of the revert in
f3dd783b239f5587213d528dc642b599f43452b6.

This allows switching 'not' to a lit internal command, making
TestRunner invoke 'grep' directly in this particular test.

The alternative, to making TestRunner's command line flattening
truly reliable, would be to make TestRunner identify if the target
executable is MSYS based (requiring a PE format inspector in python,
checking if an executable links against msys-2.0.dll or similar),
and apply different quoting rules in those cases.

[1] https://github.com/msys2/msys2-runtime/blob/msys2-3_1_7-release/winsup/cygwin/dcrt0.cc#L208

Diff Detail