[llvm-dev] Using clang/lld from the PATH when testing

[James Henderson via llvm-dev]

I've uploaded https://reviews.llvm.org/D102630 to go ahead with disabling
PATH usage for all but one pair of cases that makes use of
use_clang/use_lld/use_llvm_tool.

On Mon, 17 May 2021 at 09:58, James Henderson <jh7370.2008 at my.bristol.ac.uk>
wrote:

> Thanks, I tend to agree with you.
>
> Regarding the variables - the functionality already exists for clang. It
> seems like removing it would be counter-productive, especially as it mostly
> relies on common code I need. For the debuginfo-tests specifically, Adrian
> requested an option to test against an LLDB installed on the system (see
> https://reviews.llvm.org/D95339#2638619), which the environment variable
> would provide (in fact, it already exists, but is currently broken, as
> mentioned in the patch). Fixing the LLDB variable would only require
> changing one string (from "CLANG" to "LLDB" or equivalent), so is trivial
> to add. That leaves clang++/clang-cl in a slightly weird place, and LLD as
> also looking a bit different, but I guess we can add support for those when
> requested. I'm happy to proceed on that basis, unless there are any
> objections.
>
> On Fri, 14 May 2021 at 17:27, David Blaikie <dblaikie at gmail.com> wrote:
>
>> This sounds more accidental than intentional - if it only happens if you
>> haven't built the specified binary in your build tree & you're running the
>> tests for that specified binary. That seems super niche and someone could,
>> at worst, copy some arbitrary binary into their build tree to test it,
>> rather than relying on this fallback.
>>
>> I'd say remove it & probably don't bother adding the variables,
>> personally. If someone comes back with a use case (they should get a pretty
>> hard failure - since previously/currently they'd get a fallback to their
>> intended binary and in the future/proposal they'd get a "cannot find
>> executable" - don't suppose there's some way to make that failure a bit
>> more specific without a ton of work? They can bisect back to the revision
>> that caused the change I guess) - can add the variables to support
>> it/discuss what it should look like then.
>>
>> On Thu, May 6, 2021 at 3:49 AM James Henderson via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>>
>>> Hi all,
>>>
>>> Summary: can we stop allowing clang/lld to be picked up from the PATH
>>> environment when running testing?
>>>
>>> Details:
>>>
>>> As part of my work on a cross-project testsuite (see
>>> https://reviews.llvm.org/D95339 and other related patches), I noticed
>>> that where lit has been configured using `use_clang()` or `use_lld()` the
>>> respective tools will be selected from the system PATH, if they aren't
>>> found in the build directory. If users are running check-* to run the
>>> relevant tests, this isn't an issue, as the tools will be built (and
>>> therefore used in preference).
>>>
>>> I personally, and I'm sure many others, routinely don't use these
>>> check-* targets, preferring to manually build and run the subset of tests
>>> that have the potential to be impacted by my change. Potentially this
>>> leaves room for human error (e.g. a misconfigured build bot or simply a
>>> user error when checking things) to not build clang (or lld) in a clean
>>> environment, and as such, the "wrong" tool might be used, causing spurious
>>> tests failures (or worse, spurious passes).
>>>
>>> An environment variable ('CLANG') can be specified to specify the clang
>>> executable to use, in preference to a built one or one on the PATH. I plan
>>> to make a similar change for LLD soon too. As such, do we need the "use the
>>> PATH version" behaviour anymore? Dropping it will allow simplifying code
>>> and reduce room for human error.
>>>
>>> Thanks,
>>>
>>> James
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210517/2c0537da/attachment.html>