[Lldb-commits] [PATCH] D54567: Fix LLDB's lit files

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In https://reviews.llvm.org/D54567#1300670, @zturner wrote:

> In https://reviews.llvm.org/D54567#1300641, @JDevlieghere wrote:
>
> > Makes sense. Just to be clear, I'm not advocating running a product for the lit suite, just having one option that controls both dotest and lit.
> >
> > How is this handled today? Do we have tests that do something like that?
>
>
> The tests themselves don't try to do it, usually our tests try to be compiler agnostic, but then people will run the tests with one or more different compilers.  I think people use dotest in this manner, by specifying a path to a compiler with a version number in it, but I think it may be all downstream stuff, with no bot coverage.  Mostly they do it by specifying `LLDB_TEST_C_COMPILER=/path/to/clang-7.0` or something, and dotest is smart enough to figure this out.  We could certainly have this same logic in the lit files though once we get to that point.


I was aware of the dotest stuff (like Adrian mentioned our bots are using that). My question was specific to lit, but I guess nobody is using that functionality there (yet).

>> I'm all for iteration! We just wanna make sure we share the same "end goal".
>> 
>> I'm worried that the directory approach is incompatible with settings a specific compiler (like gcc).
> 
> I don't think the directory approach is fundamentally incompatible with using non-clang compilers.  But it's important to differentiate what the test suite itself supports and what the test suite supports //when invoked via a CMake-generated target//.  That is to say, we don't have to expose all the power of the underlying test suite through CMake.  I actually think trying to do so is a mistake, because you're often going to be circumventing it anyway to run the test suite in some special way that wasn't how you configured CMake.  dotest.py, for example, takes tons of different command line arguments, many of which are only available if you invoke it directly, as the CMake-generated command line will never use those arguments.

I agree. However I believe that *if* we expose it through CMake (for the convenience of dotest and the lldb-dotest wrapper) it should apply to CMake as well, with the notable exception of arguments we pass through the LLDB_DOTEST_ARGS variable.

> So I think this is similar.  You can pass arbitrary parameters to lit when you point it to a test directory, so there's fundamental limitation in terms of what's possible this way.  Of course, we don't currently //recognize// any, and yes it means you can't specify a specific compiler binary (such as GCC anymore), but I'm not entirely sure how useful that is in the first place, because we're talking about a static CMake configuration.

Yup, I think we're on the same page here, both dotest and lit accept parameters. We already have a mechanism to forward args to dotest, so if we wanted we could hook things up to make a lit argument work with either or both.

> So I think as far as what we expose through CMake, it should be as simple as possible and support the developer use case and the most common buildbot use cases, but more advanced use cases are still supported by directly invoking llvm-lit.py with some special arguments (which we would still need to teach the LLDB lit configuration files to understand).

No objections to this.

> Hopefully this all makes sense.

It does, thanks!


https://reviews.llvm.org/D54567