[PATCH] D113130: [llvm-libtool-darwin] Print a warning if object file names are repeated

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

In D113130#3251852 <https://reviews.llvm.org/D113130#3251852>, @smeenai wrote:

> For the test where we specify one file via `-l` and the other via its path, the output on Windows looks like
>
>   warning: file 'L-and-l.test.tmp-input1.o' was specified multiple times.
>   in: C:\Users\smeenai\llvm-project\build\Release\test\tools\llvm-libtool-darwin\Output\L-and-l.test.tmp/copy/L-and-l.test.tmp-input1.o
>   in: C:\Users\smeenai\llvm-project\build\Release\test\tools\llvm-libtool-darwin\Output\L-and-l.test.tmp/copy\L-and-l.test.tmp-input1.o
>
> Note that the last slash is a forward slash in one path and a backslash in the other. The forward slash is coming from our path specification in the test, and the backslash is coming from `sys::path::append` in libtool (to concatenate `-L` directories and `-l` names).
>
> One way to fix this would be to introduce a new `%{sep}` substitution in lit (similar to the existing `%{pathsep}`), so we could use the appropriate slashes for the platform in our test automatically. Another would be to use `sys::path::Style::windows_slash` in our `sys::path::append` call, but that could break with `\\?\` paths, which can't use forward slashes.
>
> @mstorsjo, I believe you have a bunch of experience dealing with slash differences on Windows. Any suggestions on the best way to proceed here?

Hmm... Overall I'm not a huge fan of tests like `FileCheck -DSOMETHING=%t/suffix` or similar, because as we've noticed, the FileCheck literal checks are quite brittle for such things - especially as we can configure which kind of separator that `sys::path::append` uses too. Although I guess we could mirror that into the lit subtsitutions too. (Also, for such cases, we could maybe make other lit substitutions, like %t, use the same path separator as is configured to be the default for `sys::path` - that would help fix a bunch of test cases that still are broken with forward slash on Windows.) But it's quite unintuitive to write the match patterns for such cases, to try to mimic exactly what combination of slashes you end up with in each case (when considering people writing tests on unix without access to testing thigs on Windows.)

I don't think considering using `sys::path::Style::windows_slash` explicitly is the right way to go either. I don't think we should complicate the code to do something non-obvious just for the sake of making the tools produce the path that the test framework expects.

For cases within code itself, I've considered adding a `sys::path::equals` that accepts either slash direction, and which would be case insensitive on windows (and darwin?). I wonder how hard it would be to add something similar to FileCheck. I don't offhand know exactly how it would work... But I think something like that would be good for easily writing testcases on unix, without spending lots of unnecessary fiddling on making them pass on Windows.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113130/new/

https://reviews.llvm.org/D113130