[PATCH] D83834: Add test utility 'split-file'
Fangrui Song via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 10 13:35:40 PDT 2020
MaskRay added a comment.
In D83834#2206034 <https://reviews.llvm.org/D83834#2206034>, @jhenderson wrote:
> In D83834#2201444 <https://reviews.llvm.org/D83834#2201444>, @aeubanks wrote:
>
>> There seem to be newline issues on Windows, causing llvm/test/tools/llvm-strings/radix.test and llvm/test/tools/split-file/basic.test to fail.
>>
>> $ xxd llvm/test/tools/split-file/Inputs/basic-aa.txt
>> 00000000: 0d0a 6161 0d0a ..aa..
>> $ xxd build_debug/obj/llvm/test/tools/split-file/Output/basic.test.tmp/aa
>> 00000000: 0a61 610d 0a .aa..
>>
>> Any simple fix?
>
> Have you got your git line-endings checkout settings correct? It's possible the `\r` is being added when you check out a file somewhere, I guess?
>
> In D83834#2201952 <https://reviews.llvm.org/D83834#2201952>, @ro wrote:
>
>> Some of the new tests `FAIL` when run in the same tree a second time:
>>
>> LLVM :: tools/llvm-strings/radix.test
>> LLVM :: tools/split-file/basic.test
>> LLVM :: tools/split-file/empty.test
>> LLVM :: tools/split-file/no-leading-lines.test
>>
>> The failure mode is always the same, e.g.
>>
>> split-file: error: /var/llvm/local-sparcv9-relwithdebinfo-A/test/tools/llvm-strings/Output/radix.test.tmp: File exists
>>
>> This affects the Solaris buildbots (e.g. the Solaris/sparcv9 one <http://lab.llvm.org:8014/builders/clang-solaris11-sparcv9>) and all others that run with `clean=False`.
>
> I just tried running the tests twice on my Windows machine, and they pass (no cleaning in between). At a guess, this is something different in the Solaris OS behaviour?
>
> @MaskRay, I just noticed that this new code is in llvm/tools, but I think it belongs in llvm/utils, because it is more like tools like FileCheck, not, count etc intended for internal testing. What do you think?
According to http://lists.llvm.org/pipermail/llvm-dev/2020-August/143995.html
> 1. llvm/utils. -> this builds some executables like tablegen
> 2. llvm/lib. -> This is all libraries, shouldn’t include executables.
> 3. llvm/tools. -> Generally executables, also some libraries that are tool specific.
> 4. tests.. -> Things that depend on the above.
I am on the fence which of llvm/tools/split-file and llvm/utils/split-file makes more sense. I'd likely to collect a bit more opinions.
@ro Can you set a breakpoint while running split-file, checking whether split-file can delete an existing file and replace it with a directory?
This behavior is intentionally added to support changing `%t` from a file to a directory. It appears to work well on macOS, Linux and Windows.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83834/new/
https://reviews.llvm.org/D83834
More information about the llvm-commits
mailing list