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

[Stella Stamenova via Phabricator via llvm-commits]

stella.stamenova added inline comments.


================
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.
----------------
@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.


================
Comment at: llvm/utils/lit/lit/llvm/config.py:26
             path = None
             lit_tools_dir = getattr(config, 'lit_tools_dir', None)
             required_tools = ['cmp.exe', 'grep.exe', 'sed.exe', 'diff.exe', 'echo.exe']
----------------
rnk wrote:
> I thought the implication was that getToolsPath will fail if you pass it None. I thought the suggestion was to make the default value here by `[]`, so it will succeed.
getToolsPath (and other calls in that code path) check for None, so passing None works correctly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88850