This is an archive of the discontinued LLVM Phabricator instance.

[lit] Always quote arguments containing '[' on windows
ClosedPublic

Authored by mstorsjo on Apr 6 2021, 3:38 AM.

Details

Summary

This avoids breaking clang-tidy/infrastructure/validate-check-names.cpp
if 'not' is evaluated as a lit internal tool (making TestRunner
invoke 'grep' directly in that test, instead of invoking 'not', which
then invokes 'grep').

The quoting of arguments is still brittle if the executable is an
MSYS based tool though, as MSYS based tools incorrectly unescape
backslashes in quoted arguments (contrary to regular win32 argument
parsing rules), see D99406 and
https://github.com/msys2/msys2-runtime/issues/36 for more examples
of the issues.

Diff Detail

Event Timeline

mstorsjo created this revision.Apr 6 2021, 3:38 AM
mstorsjo requested review of this revision.Apr 6 2021, 3:38 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 6 2021, 3:38 AM
mstorsjo updated this revision to Diff 335464.Apr 6 2021, 3:50 AM

Updated to include a testcase for the cases it fixes, and a clear example of an argument that this particular workaround still doesn't handle.

rnk accepted this revision.Apr 13 2021, 2:39 PM

lgtm

llvm/test/Other/lit-quoting.txt
18–19 ↗(On Diff #335464)

RUN lines that don't work as expected are often written as RUNX: to disable them. Maybe that's an old convention.

This revision is now accepted and ready to land.Apr 13 2021, 2:39 PM
This revision was landed with ongoing or failed builds.Apr 14 2021, 2:57 AM
This revision was automatically updated to reflect the committed changes.