[PATCH] D83834: Add test utility 'extract'

Tue Jul 28 01:32:28 PDT 2020

jhenderson added a comment.

I don't think I have much new to add that hasn't been said already. My personal opinions are:

1. Naming - I don't really mind whether it has a long or short name. Given it's usually going to be on a separate line anyway, I suspect, rather than writing stuff to stdout for feeding into stdin, I don't think a longer name is an issue. The point about it not being a unix tool equivalent makes some sense to me for adding an llvm- prefix, although there are counter-examples for this. I also have no preference on the exact name, as long as it conveys meaning well enough ([llvm-]split-file seems clear enough for example).
2. I want my test inputs to be all in one file where possible, as it makes it much easier to iterate on a test, understand it, etc, by keeping everything in the single file. There is a place for separate inputs, where they are going to be commonly reused across many tests (see for example the recent llvm-libtool-darwin testing, which reuses the same YAML repeatedly), but I think these are more the exception than the rule. It seems to me that the `%s.xxx` is not much better than being in `%S\Inputs` in this regards.
3. @dblaikie's concern about possible extension collisions with the `%s.xxx` approach is one I'd share. One example might be an lld or llvm-objdump test consuming multiple asm files. A naive user might call them `%s.1.s`, `%s.2.s` etc and if they're just writing the one test, won't notice that when the whole directory is tested, including those files, resulting in lit failures.
4. An "extract all" option would be useful. In fact, assuming we don't start adding other functionality to the tool (such as string substitution), I suspect the extract everything approach is going to be almost universally the better option, so might want to be the default behaviour.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83834