Merge c9fb548 into 47304e1

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,8 +34,11 @@
 import contextlib
 import nose
 import functools
+import os
+import tempfile
+import stat
 
-from ycmd import handlers, user_options_store
+from ycmd import handlers, user_options_store, utils
 from ycmd.completers.completer import Completer
 from ycmd.responses import BuildCompletionData
 from ycmd.utils import OnMac, OnWindows, ToUnicode
@@ -156,6 +159,37 @@ 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 )
+
+
+# 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
+
+
+@contextlib.contextmanager
+def TemporaryEnvironVariables( **environ ):
+  old_environ = os.environ.copy()
+  for key, value in iteritems( environ ):
+    utils.SetEnviron( os.environ, key, value )
+  try:
+    yield
+  finally:
+    os.environ.clear()
+    os.environ.update( old_environ )
+
+
 def SetUpApp():
   bottle.debug( True )
   handlers.SetServerStateToDefaults()

ycmd/tests/utils_test.py

@@ -27,19 +27,27 @@
 
 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,
+                                    TemporaryEnvironVariables,
+                                    TemporaryExecutable )
 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 +471,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.name, utils.FindExecutable( executable.name ) )
+
+
+def FindExecutable_RelativePath_test():
+  with TemporaryExecutable() as executable:
+    dirname, exename = os.path.split( executable.name )
+    relative_executable = os.path.join( '.', exename )
+    with CurrentWorkingDirectory( dirname ):
+      eq_( relative_executable, utils.FindExecutable( relative_executable ) )
+
+
+def FindExecutable_ExecutableNameInPath_test():
+  with TemporaryExecutable() as executable:
+    dirname, exename = os.path.split( executable.name )
+    with TemporaryEnvironVariables( PATH = dirname ):
+      eq_( executable.name, utils.FindExecutable( exename ) )
+
+
+def FindExecutable_ReturnNone_IfFileIsNotExecutable_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.name )
+    with CurrentWorkingDirectory( dirname ):
+      eq_( executable.name, utils.FindExecutable( exename ) )
+
+
+@WindowsOnly
+def FindExecutable_AdditionalPathExt_test():
+  with TemporaryEnvironVariables( PATHEXT = 'xyz' ):
+    with TemporaryExecutable( extension = 'xyz' ) as executable:
+      eq_( executable.name, utils.FindExecutable( executable.name ) )

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,61 @@ 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 _GetAllPossibleWindowsExecutable( filename ):
+  """Returns all the possible Windows executables given a path to a file.
+  If the given file matches any of the expected path extensions (i.e.
+  "python.exe", returns a list with the file as the only element otherwise
+  return the file with all the possible extension."""
+  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 ]
+
+
+# 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.
+def IsExecutable( filename ):
+  if OnWindows():
+    return ( not os.path.isdir( filename )
+             and any( os.path.exists( exe ) for exe
+                      in _GetAllPossibleWindowsExecutable( 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
+  if OnWindows():
+    # The current directory takes precedence on Windows.
+    if os.curdir not in paths:
+      paths.insert( 0, os.path.abspath( os.curdir ) )
+
+    files = _GetAllPossibleWindowsExecutable( executable )
   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
+    files = [ executable ]
+
+  for path in paths:
+    for file in files:
+      name = os.path.join( path, file )
+      if IsExecutable( name ):
+        return name
   return None