Search user defined python binary also in PATH

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/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,7 @@
 import contextlib
 import nose
 import functools
+import os
 
 from ycmd import handlers, user_options_store
 from ycmd.completers.completer import Completer
@@ -156,6 +157,16 @@ def UserOption( key, value ):
     handlers.UpdateUserOptions( current_options )
 
 
+@contextlib.contextmanager
+def CurrentWorkingDirectory( path ):
+  old_cwd = os.getcwd()
+  try:
+    os.chdir( path )
+    yield
+  finally:
+    os.chdir( old_cwd )
+
+
 def SetUpApp():
   bottle.debug( True )
   handlers.SetServerStateToDefaults()

ycmd/tests/utils_test.py

@@ -33,13 +33,18 @@
 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 )
 from ycmd.tests import PathToTestFile
 
 # NOTE: isinstance() vs type() is carefully used in this test file. Before
 # changing things here, read the comments in utils.ToBytes.
 
 
+ROOT_PATH = os.path.join( os.path.dirname( os.path.abspath( __file__ ) ),
+                          '..', '..' )
+
+
 @Py2Only
 def ToBytes_Py2Bytes_test():
   value = utils.ToBytes( bytes( 'abc' ) )
@@ -463,3 +468,15 @@ def SplitLines_test():
 
   for test in tests:
     yield lambda: eq_( utils.SplitLines( test[ 0 ] ), test[ 1 ] )
+
+
+def FindExecutable_ReturnSameRelativePath_IfFileIsExecutable_test():
+  with CurrentWorkingDirectory( ROOT_PATH ):
+    executable = os.path.join( '.', 'build.py' )
+    eq_( executable, utils.FindExecutable( executable ) )
+
+
+def FindExecutable_ReturnNone_IfFileIsNotExecutable_test():
+  with CurrentWorkingDirectory( ROOT_PATH ):
+    executable = os.path.join( '.', 'README.md' )
+    eq_( None, 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,50 @@ 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.
+# Check that a given file can be accessed as an executable file.
+# Additionally check that `file` is not a directory, as on Windows
+# directories pass the os.access check.
+def IsExecutable( filename ):
+  return ( os.path.exists( filename )
+            and os.access( filename, EXECUTABLE_FILE_MASK )
+            and not os.path.isdir( filename ) )
+
+
+# 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 ):
+    if IsExecutable( executable ):
+      return executable
+    return None
+
   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
+  if OnWindows():
+    # The current directory takes precedence on Windows.
+    if os.curdir not in paths:
+      paths.insert( 0, os.curdir )
+
+    # See if the given file matches any of the expected path extensions.  This
+    # will allow us to short circuit when given "python.exe".  If it does
+    # match, only test that one, otherwise we have to try others.
+    pathext = os.environ.get( 'PATHEXT', '' ).split( os.pathsep )
+    if extension in pathext:
+      files = [ executable ]
     else:
-      return executable_name
+      files = [ base + ext for ext in pathext ]
+  else:
+    files = [ executable ]
+
+  for path in paths:
+    for file in files:
+      name = os.path.join( path, file )
+      if IsExecutable( name ):
+        return name
   return None