[PATCH] D94766: Add lit env variable to disable indirect checks

Thomas Preud'homme via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 21 12:31:50 PST 2021


thopre added a comment.

In D94766#2513022 <https://reviews.llvm.org/D94766#2513022>, @jdenny wrote:

> So the lit invocations are in a script of some sort, right?  Such a script could perform a feature test to handle this use case: if `--no-indirectly-run-check` is supported, add it to some lit options variable used by all relevant lit invocations in the script, and do not otherwise.  Is that difficult to do?
>
> I'm not strongly opposed to this patch, especially if it helps many people.  However, given that this use case is external and temporary, I'm inclined to think a feature test is more reasonable: it's also simple, but it's external like the use case and can be dropped by users when they decide they no longer need to support old lit versions.  What do you think?

In our case lit is invoked for each testcase (1 lit invocation per testcase) and testcases are run in parallel. That would require detection before the tests but then you would need a fake test for that because lit does not have a mechanism to ask about whether an option exist except running a known good test with the option. But all these are specifics to our setup (we have a workaround anyway), I'm more concerned about having a migration strategy.

The new option was made a default because we fixed all testcases internally to LLVM but it remains a disruption for uses outside of LLVM. Note that I've approved that patch so I blame myself for not thinking of that. I'm just trying to fix it now by having a nice migration plan for external users. Perhaps a more palatable approach would be to make all options have a corresponding lit config rather than something specific to the `--no-indirect-run-check`. How does that sound?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94766



More information about the llvm-commits mailing list