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

Fri Oct 2 19:39:16 PDT 2020

aganea added a comment.

In D84380#2309619 <https://reviews.llvm.org/D84380#2309619>, @stella.stamenova wrote:

> 1. We are *pre*pending to the path, so our existing working setup with gnuwin32 is ignored. I think it would be better to add the paths at the end, so that what already works is not broken.

It appears the behavior before was to preprend the tools' found path, to the application's %PATH%, to rightly use `C:\Program Files\Git\usr\bin\find.exe` but not `C:\WINDOWS\system32\find.EXE` which would appear most certainly first in %PATH%. I think the important change in behavior after this patch is that now we check `if lit_tools_dir:` (L28) which is empty, and most likely always use the git registry detection. 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?

> 2. All of the tools that are found along with git (tar.exe, echo.exe, etc.) never return when called in this setup. I am not sure why this is since we have a fairly new version of git and the tools, so they should work correctly, but they don't. This may be a simple update requirement on our part, but if that's the case, the updated documentation should specify what version of git and the tools is required.

Would it possible to share a bit more about your environement? Which python version? Which git version? Can you share the contents of PATH?
When the tests become stuck on your machine, can you look at the threads with Process Explorer or with a debugger?
Does this happen randomly, or all the time?

================
Comment at: llvm/utils/lit/lit/llvm/config.py:24
-            # For tests that require Windows to run.
-            features.add('system-windows')
-
----------------
stella.stamenova wrote:
> Why did you drop this? My understanding is that this is needed in the tests to determine when to run/not run some of them based on the system.
It was already there, please see L53.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84380