[PATCH] D88850: [lit, windows] Fix the search for git tools on Windows to check the path first

Alexandre Ganea via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 5 14:25:29 PDT 2020


aganea added a comment.

@stella.stamenova Strangely, your initial bug description sounds like a deadlock initiated by `tar_version.communicate()` in https://github.com/llvm/llvm-project/blob/f7941d98091827b8d0b6fdabb731e38c99f44b13/lld/test/lit.cfg.py#L110
Are the lit tests using python 3.6 and not the 2.7 that you seem to have installed as well? In any case, I wonder if setting `stderr=open(os.devnull)` at lld/test/lit.cfg.py#L108, would fix the issue? Or perhaps just check the 'Threads' tab in Process Explorer on tar.exe, see why it is blocked.



================
Comment at: llvm/utils/lit/lit/llvm/config.py:23
         # Tweak PATH for Win32 to decide to use bash.exe or not.
         if sys.platform == 'win32':
             # Seek necessary tools in directories and set to $PATH.
----------------
stella.stamenova wrote:
> @aganea: From the discussion on zturner's change: Do you know `if sys.platform == 'win32'` is equivalent to `platform.system() == 'windows'`? If it is, we can clean this up. If it is not, we need to bring back `features.add('system-windows')`. As far as I can tell, they should be the same, but we are not using them that way.
`sys.platform == 'win32'` should be equivalent to `platform.system() == 'windows'`: https://stackoverflow.com/a/54837707 - were you thinking of an edge case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88850



More information about the llvm-commits mailing list