Auto merge of #429 - vheon:python-binary-from-path, r=micbou

[READY] Search user defined python binary also in PATH

I'm marking as an RFC because there is also #424 which does almost the same thing.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Review on Reviewable"/>](https://reviewable.io/reviews/valloric/ycmd/429)
<!-- Reviewable:end -->

ycmd/completers/python/jedi_completer.py

@@ -77,16 +77,12 @@ def __init__( self, user_options ):
 
   def _UpdatePythonBinary( self, binary ):
     if binary:
-      if not self._CheckBinaryExists( binary ):
+      resolved_binary = utils.FindExecutable( binary )
+      if not resolved_binary:
         msg = BINARY_NOT_FOUND_MESSAGE.format( binary )
         self._logger.error( msg )
         raise RuntimeError( msg )
-      self._python_binary_path = binary
-
-
-  def _CheckBinaryExists( self, binary ):
-    """This method is here to help testing"""
-    return os.path.isfile( binary )
+      self._python_binary_path = resolved_binary
 
 
   def SupportedFiletypes( self ):

ycmd/completers/typescript/typescript_completer.py

@@ -99,6 +99,7 @@ def __init__( self, user_options ):
     if not binarypath:
       _logger.error( BINARY_NOT_FOUND_MESSAGE )
       raise RuntimeError( BINARY_NOT_FOUND_MESSAGE )
+    _logger.info( 'Found TSServer at {0}'.format( binarypath ) )
 
     self._logfile = _LogFileName()
     tsserver_log = '-file {path} -level {level}'.format( path = self._logfile,

ycmd/tests/python/user_defined_python_test.py

@@ -86,8 +86,7 @@ def UserDefinedPython_WithoutAnyOption_DefaultToYcmdPython_test( app, *args ):
 
 @IsolatedYcmd
 @patch( 'ycmd.utils.SafePopen' )
-@patch( 'ycmd.completers.python.jedi_completer.JediCompleter.'
-          '_CheckBinaryExists', return_value = False )
+@patch( 'ycmd.utils.FindExecutable', return_value = None )
 def UserDefinedPython_WhenNonExistentPythonIsGiven_ReturnAnError_test( app,
                                                                        *args ):
   python = '/non/existing/path/python'
@@ -103,8 +102,7 @@ def UserDefinedPython_WhenNonExistentPythonIsGiven_ReturnAnError_test( app,
 
 @IsolatedYcmd
 @patch( 'ycmd.utils.SafePopen' )
-@patch( 'ycmd.completers.python.jedi_completer.JediCompleter.'
-          '_CheckBinaryExists', return_value = True )
+@patch( 'ycmd.utils.FindExecutable', side_effect = lambda x: x )
 def UserDefinedPython_WhenExistingPythonIsGiven_ThatIsUsed_test( app, *args ):
   python = '/existing/python'
   with UserOption( 'python_binary_path', python ):
@@ -114,8 +112,7 @@ def UserDefinedPython_WhenExistingPythonIsGiven_ThatIsUsed_test( app, *args ):
 
 @IsolatedYcmd
 @patch( 'ycmd.utils.SafePopen' )
-@patch( 'ycmd.completers.python.jedi_completer.JediCompleter.'
-          '_CheckBinaryExists', return_value = True )
+@patch( 'ycmd.utils.FindExecutable', side_effect = lambda x: x )
 def UserDefinedPython_RestartServerWithoutArguments_WillReuseTheLastPython_test(
   app, *args ):
   request = BuildRequest( filetype = 'python',
@@ -126,8 +123,7 @@ def UserDefinedPython_RestartServerWithoutArguments_WillReuseTheLastPython_test(
 
 @IsolatedYcmd
 @patch( 'ycmd.utils.SafePopen' )
-@patch( 'ycmd.completers.python.jedi_completer.JediCompleter.'
-          '_CheckBinaryExists', return_value = True )
+@patch( 'ycmd.utils.FindExecutable', side_effect = lambda x: x )
 def UserDefinedPython_RestartServerWithArgument_WillUseTheSpecifiedPython_test(
   app, *args ):
   python = '/existing/python'
@@ -139,8 +135,7 @@ def UserDefinedPython_RestartServerWithArgument_WillUseTheSpecifiedPython_test(
 
 @IsolatedYcmd
 @patch( 'ycmd.utils.SafePopen' )
-@patch( 'ycmd.completers.python.jedi_completer.JediCompleter.'
-          '_CheckBinaryExists', return_value = False )
+@patch( 'ycmd.utils.FindExecutable', return_value = None )
 def UserDefinedPython_RestartServerWithNonExistingPythonArgument_test( app,
                                                                        *args ):
   python = '/non/existing/python'

ycmd/tests/test_utils.py

@@ -34,6 +34,9 @@
 import contextlib
 import nose
 import functools
+import os
+import tempfile
+import stat
 
 from ycmd import handlers, user_options_store
 from ycmd.completers.completer import Completer
@@ -156,6 +159,25 @@ def UserOption( key, value ):
     handlers.UpdateUserOptions( current_options )
 
 
+@contextlib.contextmanager
+def CurrentWorkingDirectory( path ):
+  old_cwd = os.getcwd()
+  os.chdir( path )
+  try:
+    yield
+  finally:
+    os.chdir( old_cwd )
+
+
+# The "exe" suffix is needed on Windows and not harmful on other platforms.
+@contextlib.contextmanager
+def TemporaryExecutable( extension = '.exe' ):
+  with tempfile.NamedTemporaryFile( prefix = 'Temp',
+                                    suffix = extension ) as executable:
+    os.chmod( executable.name, stat.S_IXUSR )
+    yield executable.name
+
+
 def SetUpApp():
   bottle.debug( True )
   handlers.SetServerStateToDefaults()

ycmd/tests/utils_test.py

@@ -27,13 +27,16 @@
 
 import os
 import subprocess
+import tempfile
 from shutil import rmtree
 import ycm_core
 from future.utils import native
 from mock import patch, call
 from nose.tools import eq_, ok_
 from ycmd import utils
-from ycmd.tests.test_utils import Py2Only, Py3Only, WindowsOnly
+from ycmd.tests.test_utils import ( Py2Only, Py3Only, WindowsOnly,
+                                    CurrentWorkingDirectory,
+                                    TemporaryExecutable )
 from ycmd.tests import PathToTestFile
 
 # NOTE: isinstance() vs type() is carefully used in this test file. Before
@@ -463,3 +466,43 @@ def SplitLines_test():
 
   for test in tests:
     yield lambda: eq_( utils.SplitLines( test[ 0 ] ), test[ 1 ] )
+
+
+def FindExecutable_AbsolutePath_test():
+  with TemporaryExecutable() as executable:
+    eq_( executable, utils.FindExecutable( executable ) )
+
+
+def FindExecutable_RelativePath_test():
+  with TemporaryExecutable() as executable:
+    dirname, exename = os.path.split( executable )
+    relative_executable = os.path.join( '.', exename )
+    with CurrentWorkingDirectory( dirname ):
+      eq_( relative_executable, utils.FindExecutable( relative_executable ) )
+
+
+@patch.dict( 'os.environ', { 'PATH': tempfile.gettempdir() } )
+def FindExecutable_ExecutableNameInPath_test():
+  with TemporaryExecutable() as executable:
+    dirname, exename = os.path.split( executable )
+    eq_( executable, utils.FindExecutable( exename ) )
+
+
+def FindExecutable_ReturnNoneIfFileIsNotExecutable_test():
+  with tempfile.NamedTemporaryFile() as non_executable:
+    eq_( None, utils.FindExecutable( non_executable.name ) )
+
+
+@WindowsOnly
+def FindExecutable_CurrentDirectory_test():
+  with TemporaryExecutable() as executable:
+    dirname, exename = os.path.split( executable )
+    with CurrentWorkingDirectory( dirname ):
+      eq_( executable, utils.FindExecutable( exename ) )
+
+
+@WindowsOnly
+@patch.dict( 'os.environ', { 'PATHEXT': '.xyz' } )
+def FindExecutable_AdditionalPathExt_test():
+  with TemporaryExecutable( extension = '.xyz' ) as executable:
+    eq_( executable, utils.FindExecutable( executable ) )

ycmd/utils.py

@@ -38,8 +38,6 @@
 # Creation flag to disable creating a console window on Windows. See
 # https://msdn.microsoft.com/en-us/library/windows/desktop/ms684863.aspx
 CREATE_NO_WINDOW = 0x08000000
-# Executable extensions used on Windows
-WIN_EXECUTABLE_EXTS = [ '.exe', '.bat', '.cmd' ]
 
 # Don't use this! Call PathToCreatedTempDir() instead. This exists for the sake
 # of tests.
@@ -49,6 +47,8 @@
 ACCESSIBLE_TO_ALL_MASK = ( stat.S_IROTH | stat.S_IWOTH | stat.S_IXOTH |
                            stat.S_IRGRP | stat.S_IWGRP | stat.S_IXGRP )
 
+EXECUTABLE_FILE_MASK = os.F_OK | os.X_OK
+
 
 # Python 3 complains on the common open(path).read() idiom because the file
 # doesn't get closed. So, a helper func.
@@ -208,27 +208,56 @@ def PathToFirstExistingExecutable( executable_name_list ):
   return None
 
 
-# On Windows, distutils.spawn.find_executable only works for .exe files
-# but .bat and .cmd files are also executables, so we use our own
-# implementation.
+def _GetWindowsExecutable( filename ):
+  def _GetPossibleWindowsExecutable( filename ):
+    pathext = [ ext.lower() for ext in
+                os.environ.get( 'PATHEXT', '' ).split( os.pathsep ) ]
+    base, extension = os.path.splitext( filename )
+    if extension.lower() in pathext:
+      return [ filename ]
+    else:
+      return [ base + ext for ext in pathext ]
+
+  for exe in _GetPossibleWindowsExecutable( filename ):
+    if os.path.isfile( exe ):
+      return exe
+  return None
+
+
+# Check that a given file can be accessed as an executable file, so controlling
+# the access mask on Unix and if has a valid extension on Windows. It returns
+# the path to the executable or None if no executable was found.
+def GetExecutable( filename ):
+  if OnWindows():
+    return _GetWindowsExecutable( filename )
+
+  if ( os.path.isfile( filename )
+       and os.access( filename, EXECUTABLE_FILE_MASK ) ):
+    return filename
+  return None
+
+
+# Adapted from https://hg.python.org/cpython/file/3.5/Lib/shutil.py#l1081
+# to be backward compatible with Python2 and more consistent to our codebase.
 def FindExecutable( executable ):
+  # If we're given a path with a directory part, look it up directly rather
+  # than referring to PATH directories. This includes checking relative to the
+  # current directory, e.g. ./script
+  if os.path.dirname( executable ):
+    return GetExecutable( executable )
+
   paths = os.environ[ 'PATH' ].split( os.pathsep )
-  base, extension = os.path.splitext( executable )
 
-  if OnWindows() and extension.lower() not in WIN_EXECUTABLE_EXTS:
-    extensions = WIN_EXECUTABLE_EXTS
-  else:
-    extensions = ['']
-
-  for extension in extensions:
-    executable_name = executable + extension
-    if not os.path.isfile( executable_name ):
-      for path in paths:
-        executable_path = os.path.join(path, executable_name )
-        if os.path.isfile( executable_path ):
-          return executable_path
-    else:
-      return executable_name
+  if OnWindows():
+    # The current directory takes precedence on Windows.
+    curdir = os.path.abspath( os.curdir )
+    if curdir not in paths:
+      paths.insert( 0, curdir )
+
+  for path in paths:
+    exe = GetExecutable( os.path.join( path, executable ) )
+    if exe:
+      return exe
   return None