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

Thu Jan 21 13:33:26 PST 2021

jdenny added a comment.

In D94766#2513362 <https://reviews.llvm.org/D94766#2513362>, @thopre wrote:

> 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.

Yes, that's what I had in mind.  It seems simple, but maybe I'm missing something.

A fake test is probably the most robust way to check.  But an even simpler check that should work for now is: `llvm-lit --help | grep -q -- --no-indirectly-run-check`.

> But all these are specifics to our setup (we have a workaround anyway), I'm more concerned about having a migration strategy.

What makes it specific to your setup?  Feature tests when building/testing a project are common.

> 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.

Are there many external users suffering from this?  If so, hopefully they can chime in here.

> 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?

By "lit config", do you mean a flag you can set in a lit config file, like `lit.cfg.py`?  How do you imagine that old lit versions should handle lit configs they don't recognize?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94766