[PATCH] D84380: [lit] Support running tests on Windows without GnuWin32.

[Stella Stamenova via Phabricator via llvm-commits]

stella.stamenova added a comment.

In D84380#2310895 <https://reviews.llvm.org/D84380#2310895>, @zturner wrote:

> In D84380#2310504 <https://reviews.llvm.org/D84380#2310504>, @stella.stamenova wrote:
>
>> In D84380#2309885 <https://reviews.llvm.org/D84380#2309885>, @stella.stamenova wrote:
>>
>>> In D84380#2309853 <https://reviews.llvm.org/D84380#2309853>, @aganea wrote:  Maybe if L28 wasn't there, things would work correctly if you already had the required tools in %PATH%? Would you possibly try that please?
>>>
>>>> 
>>>
>>> I will try it. I'll let you know if it makes a difference.
>>
>> Yes, that works. We don't end up prepending the lit path and the tests work as before.
>
> For some context, L28 was added because the previous code made the underlying assumption that this variable was defined.  Specifically, this:
>
>   path = self.lit_config.getToolsPath(lit_tools_dir, config.environment['PATH'], required_tools)
>
> will raise an exception in the case that `lit_tools_dir==None`.  It sounds like you're saying `lit_tools_dir = []` in your case.  If so, instead of removing this line, can it be changed to `if lit_tools_dir != None:`?

`if lit_tools_dir is not None:` works also. @rnk - Does that work on your bot also? It would be ideal to not break either setup until we can figure out why the git tools don't work for everyone.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84380