[PATCH] D154984: [lit] Drop "Script:", make -v and -a imply -vv

Mon Jul 17 17:22:28 PDT 2023

jdenny added a comment.

@MaskRay and @awarzynski: Thanks for your comments.  You both have pointed out that the `-vv`-style command trace breaks apart commands joined by `|` (and other command separators) so it's hard to copy-and-paste command lines to a terminal.

Which of the following would you prefer?

1. Keep "Script:".  To address the redundancy problem that motivated this patch, disable "Script:" by default when just given -a or -v, and add some new command-line option to enable it (or maybe recycle -vv).  To address the PYTHON line problem that motivated this patch, "Script:" would not include RUN lines beyond the first PYTHON line.

2. Extend lit's internal shell to print full shell command lines before executing them.  Reconfigure tests suites that need this functionality to use lit's internal shell.  (The examples you showed appear to be using external shells.)  Perhaps improve the formatting of stdout/stderr to better distinguish it so shell commands are easier to find.

1 would be the easiest to implement now, but its solutions to the original motivations for this patch feel like a bad design with an inconsistent user interface.

2 requires more work, including adjusting some test suites that want to use it.  However, if people can live with it, I feel like it's the cleanest path forward.

I started to propose a third alternative to handle external shells.  However, I feel like we should encourage use of lit's internal shell for portability anyway, and we should avoid hacking around external shells to try to make them do what we want.

In D154984#4504368 <https://reviews.llvm.org/D154984#4504368>, @awarzynski wrote:

> Last, but not least, thank you for caring about LIT and improving it for us - that's greatly appreciated!

You're welcome!

In D154984#4508213 <https://reviews.llvm.org/D154984#4508213>, @ldionne wrote:

> TBH, I'd love to avoid introducing another flag to control the output of Lit. This just seems like adding complexity to me. I think your proposed changes are better and more general, and they unblock useful feature work. I would go for the pure approach and then we can all learn to live with the new output results and improve them (without adding new flags) as we gain experience with that new output.
>
> Again, just my .02 but I'd be fine if you didn't try to satisfy everyone's current usage. We'll learn to live with it and we'll improve the baseline output of the tool as we go, but let's not add complexity to the tool just for the sake of getting consensus here.

Thanks for this opinion and your other answers.  It sounds to me like your thinking is compatible with option 2 above... but with the added note that we don't have to perfect the trace format immediately.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154984