This is an archive of the discontinued LLVM Phabricator instance.

Quote '?' in llvm-rc test
ClosedPublic

Authored by rnk on Jul 25 2017, 1:14 PM.

Details

Summary

Bash interperets the '?' character as matching an arbitrary character.
On systems that have a file or directory with exactly one character in
their root directory, '/?' gets reinterpreted into that pathname, which
fails to match the expected Help text for llvm-rc.
This patch quotes the '/?' to avoid that edge case.

Diff Detail

Repository
rL LLVM

Event Timeline

I'm not sure off the top of my head if this works the same on Windows' cmd.exe. If someone can check this on a Windows box that would be useful.

ecbeckmann edited edge metadata.Jul 25 2017, 1:51 PM

I'm not sure off the top of my head if this works the same on Windows' cmd.exe. If someone can check this on a Windows box that would be useful.

I believe this is not the case in cmd.exe, since most commands use /? to display the help text.

Anyways, this explains some of the failures I've seen running help text tests.

mnbvmar edited edge metadata.Jul 25 2017, 2:32 PM

It seems that single-quotes fail on Windows ('/h' is considered a whole argument, including the quotes). Double-quotes should work, though.

rnk accepted this revision.Jul 25 2017, 2:39 PM

It seems that single-quotes fail on Windows ('/h' is considered a whole argument, including the quotes). Double-quotes should work, though.

We don't actually use cmd to run tests on Windows, so the single quotes should work. We use lit's internal shell, which is effectively a limited implementation of bash.

This revision is now accepted and ready to land.Jul 25 2017, 2:39 PM
ruiu added a subscriber: ruiu.Jul 25 2017, 2:43 PM
ruiu added inline comments.
test/tools/llvm-rc/helpmsg.test
1 ↗(On Diff #108144)

nit: you don't need to quote this, but only '?'

dyung added a subscriber: dyung.Jul 25 2017, 3:21 PM

We have hit this corner case in our internal Windows bot! I can confirm that your fix, fixes it on our bot, so please submit it when you can!

rnk commandeered this revision.Jul 26 2017, 9:24 AM
rnk edited reviewers, added: jgravelle-google; removed: rnk.
This revision now requires review to proceed.Jul 26 2017, 9:25 AM
rnk marked an inline comment as done.Jul 26 2017, 9:25 AM
This revision was automatically updated to reflect the committed changes.