[llvm-dev] [llvm-rc] absolute.test failing
Martin Storsjö via llvm-dev
llvm-dev at lists.llvm.org
Thu Jan 10 00:01:28 PST 2019
On Wed, 9 Jan 2019, David Greene wrote:
> I've come across a curious and pernicious problem in llvm-rc.
> absolute.test checks that llvm-rc can accept a filename that is an
> absolute path. And it works just fine. Until you run it with a file
> that starts with "/c."
Hmm, that's rather unfortunate indeed.
FWIW, this test doesn't test specifically whether llvm-rc can accept an
absolute filename as command line argument - all the llvm-rc tests run
llvm-rc with absolute filenames as arguments. This test checks whether
llvm-rc can handle an absolute filename reference within a rc file.
I presume you run into the same issue on all other tests in
test/tools/llvm-rc as well?
> These will fail:
>
> llvm-rc /crawl/through/some/path/to/my.rc
> llvm-rc /c/some/path/to/my.rc
>
> The option parser ends up interpreting "/" as an option prefix and then
> the parser matches it to this in tools/llvm-rc/Opts.td:
>
> def CODEPAGE : JoinedOrSeparate<[ "/", "-" ], "C">,
> HelpText<"Set the codepage used for input strings.">;
>
> The test then fails with:
> Exactly one input file should be provided.
>
> The same problem happens with files that begin with "/r"
> (/read/the/path/to/my.rc) "/sl" (/slink/along/the/path/to/my.rc) or any
> other path that happens to begin with the same text as an option in
> Opts.td.
>
> This triggered on one of our builders that just happens to build to a
> path that begins with "/c." Presumably none of the existing Buildbots
> build to paths that cause problems.
>
> It's easy enough to construct a test for this, but I'm not sure how/if
> llvm-rc should be fixed. I don't know why it accepts both "/" and "-"
> as option prefixes. As this mostly seems related to Windows (resource
> files), should tests be UNSUPPORTED on every other platform? Or is
> llvm-rc intended to be a cross-platform way to create resource files?
It's definitely intended as a cross-platform tool for generating windows
resource files, to allow for cross compilation etc.
> If the latter, then it seems like options ought to use the "/" prefix on
> Windows and "-" everywhere else so as not to conflict with path
> specifiers.
Well, build scripts that call llvm-rc might be using either (more or less
agnostic of what platform it runs on). I personally prefer always using
"-" everywhere though (which also is supported on windows, and also
supported by the original microsoft tools, even if their help listings
only display the form with a "/").
FWIW, lld-link also implements the same form of options using both
prefixes, but there's less risk of unintended matches as most option names
are full words, not single-char abbreviations.
One way of disambiguating between option and pathname for the sake of the
tests, would be to add '--' before the path arguments, which seems to be
handled by the LLVM options parser at least. Does that sound sensible to
you (and others CC:d)?
// Martin
More information about the llvm-dev
mailing list