[PATCH] D52831: [lit] Only return a found bash executable on Windows if it can understand Windows paths

Greg Bedwell via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 3 08:17:51 PDT 2018


gbedwell created this revision.
gbedwell added reviewers: ddunbar, zturner, rnk.
Herald added a subscriber: delcypher.

I've been putting up with a couple of failing tests locally for a little while now and finally made the time to investigate the problem:

  Failing Tests (2):
      lit :: shtest-format.py
      lit :: shtest-run-at-line.py

This seemed to be happening on my PC only.  Digging deeper, I found this error nested deep in the logs:

  ******************** TEST 'shtest-format :: external_shell/fail.txt' FAILED ********************
  Script:
  --
  : 'RUN: at line 3';   echo "line 1: failed test output on stdout"
  : 'RUN: at line 4';   echo "line 2: failed test output on stdout"
  : 'RUN: at line 5';   cat "does-not-exist"
  --
  Exit Code: 127
  
  Command Output (stderr):
  --
  /bin/bash: E:workupstream-llvmbuild-vs2015-native-ninjautilslittestsInputsshtest-formatexternal_shellOutputfail.txt.script: No such file or directory
  
  --

It turns out that the problem was that WSL installation had placed bash.exe in C:\windows\system32 which was now used in preference to my other version of bash ("C:\Program Files\Git\usr\bin\bash.exe").  The primary difference being that unlike git's bash.exe, WSL's bash.exe can't cope with being invoked with

  bash.exe c:\\foo\\script.sh

and would instead need to be invoked with:

  bash.exe /mnt/c/foo/script.sh

I've worked around the problem by just changing the order of paths in my environment variable so that git bash gets preference, but to save someone else running into the same thing, here's an attempt at fixing it.  It's not the most elegant solution, but seems the least intrusive in terms of changing current behaviour.

An obvious question is whether we should ever be using bash on Windows, considering everything seems to pass without it anyway.  Is there a specific use-case for that combination or could we just force it to use 'cmd /c' in TestRunner::executeScript when on Windows for all cases nowadays?


https://reviews.llvm.org/D52831

Files:
  utils/lit/lit/LitConfig.py


Index: utils/lit/lit/LitConfig.py
===================================================================
--- utils/lit/lit/LitConfig.py
+++ utils/lit/lit/LitConfig.py
@@ -120,6 +120,17 @@
         if self.bashPath is None:
             self.bashPath = ''
 
+        # Check whether the found version of bash is able to cope with paths in
+        # the host path format. If not, don't return it as it can't be used to
+        # run scripts. For example, WSL's bash.exe requires '/mnt/c/foo' rather
+        # than 'C:\\foo' or 'C:/foo'.
+        if self.isWindows and self.bashPath:
+            command = [self.bashPath, '-c',
+                       '[[ -f "%s" ]]' % self.bashPath.replace('\\', '\\\\')]
+            _, _, exitCode = lit.util.executeCommand(command)
+            if exitCode:
+                self.bashPath = ''
+
         return self.bashPath
 
     def getToolsPath(self, dir, paths, tools):


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D52831.168114.patch
Type: text/x-patch
Size: 912 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181003/13359d5a/attachment.bin>


More information about the llvm-commits mailing list