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

Stella Stamenova via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 5 15:02:27 PDT 2020


stella.stamenova marked an inline comment as done.
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 wrote:
> 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?
In my experience, there are frequently edge cases around 32b vs. 64b vs. wow64 and I don't know enough about python's platform and sys.platform to be sure that there isn't one here. Certainly, the code is not written as if the two conditions are equivalent.


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