Auto merge of #2787 - micbou:improve-path-to-python-interpreter, r=puremourning
[READY] Improve path to python interpreter error handling YCM returns the cryptic error `YouCompleteMe unavailable: [Errno 2] No such file or directory` when the Python interpreter path used to start the server doesn't exist. See issues https://github.com/Valloric/YouCompleteMe/issues/2773 and https://github.com/Valloric/YouCompleteMe/issues/2775. This PR improves that by using the `FindExecutable` and `GetExecutable` functions from ycmd to check if the Python interpreter path exists (and is an executable) and by returning a much more helpful message if it doesn't. We use `FindExecutable` instead of `GetExecutable` on the `g:ycm_server_python_interpreter` option so that the user can specify a Python path with just the executable name (e.g. `python`, `python2`, `python3`). This also fixes a Python traceback in Vim that occurs when, after successfully starting the server, the `g:ycm_server_python_interpreter` option is set to an invalid path and the server is restarted with `:YcmRestartServer`. Steps to reproduce are: - start Vim with a working YCM; - type `:let g:ycm_server_python_interpreter = '/invalid/python/path'`; - type `:YcmRestartServer`. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/valloric/youcompleteme/2787) <!-- Reviewable:end -->
This commit is contained in:
commit
712c417529
@ -46,14 +46,15 @@ def PathToPythonInterpreter():
|
||||
|
||||
python_interpreter = vim.eval( 'g:ycm_server_python_interpreter' )
|
||||
if python_interpreter:
|
||||
if _EndsWithPython( python_interpreter ):
|
||||
python_interpreter = utils.FindExecutable( python_interpreter )
|
||||
if python_interpreter:
|
||||
return python_interpreter
|
||||
|
||||
raise RuntimeError( "Path in 'g:ycm_server_python_interpreter' option "
|
||||
"does not point to a valid Python 2.6+ or 3.3+." )
|
||||
|
||||
python_interpreter = _PathToPythonUsedDuringBuild()
|
||||
if _EndsWithPython( python_interpreter ):
|
||||
if python_interpreter and utils.GetExecutable( python_interpreter ):
|
||||
return python_interpreter
|
||||
|
||||
# On UNIX platforms, we use sys.executable as the Python interpreter path.
|
||||
@ -76,9 +77,9 @@ def PathToPythonInterpreter():
|
||||
if python_interpreter:
|
||||
return python_interpreter
|
||||
|
||||
raise RuntimeError( "Cannot find Python 2.6+ or 3.3+. You can set its path "
|
||||
"using the 'g:ycm_server_python_interpreter' "
|
||||
"option." )
|
||||
raise RuntimeError( "Cannot find Python 2.6+ or 3.3+. "
|
||||
"Set the 'g:ycm_server_python_interpreter' option "
|
||||
"to a Python interpreter path." )
|
||||
|
||||
|
||||
def _PathToPythonUsedDuringBuild():
|
||||
|
@ -55,7 +55,7 @@ def PathToTestFile( *args ):
|
||||
return os.path.join( dir_of_current_script, 'testdata', *args )
|
||||
|
||||
|
||||
def _MakeUserOptions( custom_options = {} ):
|
||||
def MakeUserOptions( custom_options = {} ):
|
||||
options = dict( user_options_store.DefaultOptions() )
|
||||
options.update( DEFAULT_CLIENT_OPTIONS )
|
||||
options.update( custom_options )
|
||||
@ -122,7 +122,7 @@ def YouCompleteMeInstance( custom_options = {} ):
|
||||
def Decorator( test ):
|
||||
@functools.wraps( test )
|
||||
def Wrapper( *args, **kwargs ):
|
||||
ycm = YouCompleteMe( _MakeUserOptions( custom_options ) )
|
||||
ycm = YouCompleteMe( MakeUserOptions( custom_options ) )
|
||||
WaitUntilReady()
|
||||
ycm.CheckIfServerIsReady()
|
||||
try:
|
||||
|
@ -181,12 +181,17 @@ def _MockVimMatchEval( value ):
|
||||
return None
|
||||
|
||||
|
||||
# This variable exists to easily mock the 'g:ycm_server_python_interpreter'
|
||||
# option in tests.
|
||||
server_python_interpreter = ''
|
||||
|
||||
|
||||
def _MockVimEval( value ):
|
||||
if value == 'g:ycm_min_num_of_chars_for_completion':
|
||||
return 0
|
||||
|
||||
if value == 'g:ycm_server_python_interpreter':
|
||||
return ''
|
||||
return server_python_interpreter
|
||||
|
||||
if value == 'tempname()':
|
||||
return '_TEMP_FILE_'
|
||||
|
@ -32,7 +32,10 @@ from hamcrest import ( assert_that, contains, empty, equal_to, is_in, is_not,
|
||||
matches_regexp )
|
||||
from mock import call, MagicMock, patch
|
||||
|
||||
from ycm.tests import StopServer, test_utils, YouCompleteMeInstance
|
||||
from ycm.paths import _PathToPythonUsedDuringBuild
|
||||
from ycm.youcompleteme import YouCompleteMe
|
||||
from ycm.tests import ( MakeUserOptions, StopServer, test_utils,
|
||||
WaitUntilReady, YouCompleteMeInstance )
|
||||
from ycm.client.base_request import _LoadExtraConfFile
|
||||
from ycmd.responses import ServerError
|
||||
|
||||
@ -42,6 +45,60 @@ def YouCompleteMe_YcmCoreNotImported_test( ycm ):
|
||||
assert_that( 'ycm_core', is_not( is_in( sys.modules ) ) )
|
||||
|
||||
|
||||
@patch( 'ycm.vimsupport.PostVimMessage' )
|
||||
def YouCompleteMe_InvalidPythonInterpreterPath_test( post_vim_message ):
|
||||
try:
|
||||
with patch( 'ycm.tests.test_utils.server_python_interpreter',
|
||||
'/invalid/path/to/python' ):
|
||||
ycm = YouCompleteMe( MakeUserOptions() )
|
||||
assert_that( ycm.IsServerAlive(), equal_to( False ) )
|
||||
post_vim_message.assert_called_once_with(
|
||||
"Unable to start the ycmd server. "
|
||||
"Path in 'g:ycm_server_python_interpreter' option does not point "
|
||||
"to a valid Python 2.6+ or 3.3+. "
|
||||
"Correct the error then restart the server with ':YcmRestartServer'." )
|
||||
|
||||
post_vim_message.reset_mock()
|
||||
|
||||
with patch( 'ycm.tests.test_utils.server_python_interpreter',
|
||||
_PathToPythonUsedDuringBuild() ):
|
||||
ycm.RestartServer()
|
||||
|
||||
assert_that( ycm.IsServerAlive(), equal_to( True ) )
|
||||
post_vim_message.assert_called_once_with( 'Restarting ycmd server...' )
|
||||
finally:
|
||||
WaitUntilReady()
|
||||
StopServer( ycm )
|
||||
|
||||
|
||||
@patch( 'ycmd.utils.PathToFirstExistingExecutable', return_value = None )
|
||||
@patch( 'ycm.paths._EndsWithPython', return_value = False )
|
||||
@patch( 'ycm.vimsupport.PostVimMessage' )
|
||||
def YouCompleteMe_NoPythonInterpreterFound_test( post_vim_message, *args ):
|
||||
try:
|
||||
with patch( 'ycmd.utils.ReadFile', side_effect = IOError ):
|
||||
|
||||
ycm = YouCompleteMe( MakeUserOptions() )
|
||||
assert_that( ycm.IsServerAlive(), equal_to( False ) )
|
||||
post_vim_message.assert_called_once_with(
|
||||
"Unable to start the ycmd server. Cannot find Python 2.6+ or 3.3+. "
|
||||
"Set the 'g:ycm_server_python_interpreter' option to a Python "
|
||||
"interpreter path. "
|
||||
"Correct the error then restart the server with ':YcmRestartServer'." )
|
||||
|
||||
post_vim_message.reset_mock()
|
||||
|
||||
with patch( 'ycm.tests.test_utils.server_python_interpreter',
|
||||
_PathToPythonUsedDuringBuild() ):
|
||||
ycm.RestartServer()
|
||||
|
||||
assert_that( ycm.IsServerAlive(), equal_to( True ) )
|
||||
post_vim_message.assert_called_once_with( 'Restarting ycmd server...' )
|
||||
finally:
|
||||
WaitUntilReady()
|
||||
StopServer( ycm )
|
||||
|
||||
|
||||
@YouCompleteMeInstance()
|
||||
@patch( 'ycm.vimsupport.PostVimMessage', new_callable = ExtendedMock )
|
||||
def RunNotifyUserIfServerCrashed( ycm, test, post_vim_message ):
|
||||
@ -271,7 +328,6 @@ def YouCompleteMe_GetDefinedSubcommands_ErrorFromServer_test( ycm,
|
||||
assert_that( result, empty() )
|
||||
|
||||
|
||||
|
||||
@YouCompleteMeInstance()
|
||||
@patch( 'ycm.vimsupport.PostVimMessage', new_callable = ExtendedMock )
|
||||
def YouCompleteMe_ShowDetailedDiagnostic_MessageFromServer_test(
|
||||
|
@ -150,7 +150,21 @@ class YouCompleteMe( object ):
|
||||
json.dump( options_dict, options_file )
|
||||
options_file.flush()
|
||||
|
||||
args = [ paths.PathToPythonInterpreter(),
|
||||
BaseRequest.server_location = 'http://127.0.0.1:' + str( server_port )
|
||||
BaseRequest.hmac_secret = hmac_secret
|
||||
|
||||
try:
|
||||
python_interpreter = paths.PathToPythonInterpreter()
|
||||
except RuntimeError as error:
|
||||
error_message = (
|
||||
"Unable to start the ycmd server. {0}. "
|
||||
"Correct the error then restart the server "
|
||||
"with ':YcmRestartServer'.".format( str( error ).rstrip( '.' ) ) )
|
||||
self._logger.exception( error_message )
|
||||
vimsupport.PostVimMessage( error_message )
|
||||
return
|
||||
|
||||
args = [ python_interpreter,
|
||||
paths.PathToServerScript(),
|
||||
'--port={0}'.format( server_port ),
|
||||
'--options_file={0}'.format( options_file.name ),
|
||||
@ -170,8 +184,6 @@ class YouCompleteMe( object ):
|
||||
|
||||
self._server_popen = utils.SafePopen( args, stdin_windows = PIPE,
|
||||
stdout = PIPE, stderr = PIPE )
|
||||
BaseRequest.server_location = 'http://127.0.0.1:' + str( server_port )
|
||||
BaseRequest.hmac_secret = hmac_secret
|
||||
|
||||
self._NotifyUserIfServerCrashed()
|
||||
|
||||
@ -215,9 +227,8 @@ class YouCompleteMe( object ):
|
||||
|
||||
|
||||
def IsServerAlive( self ):
|
||||
return_code = self._server_popen.poll()
|
||||
# When the process hasn't finished yet, poll() returns None.
|
||||
return return_code is None
|
||||
return bool( self._server_popen ) and self._server_popen.poll() is None
|
||||
|
||||
|
||||
def CheckIfServerIsReady( self ):
|
||||
@ -233,7 +244,8 @@ class YouCompleteMe( object ):
|
||||
|
||||
|
||||
def _NotifyUserIfServerCrashed( self ):
|
||||
if self._user_notified_about_crash or self.IsServerAlive():
|
||||
if ( not self._server_popen or self._user_notified_about_crash or
|
||||
self.IsServerAlive() ):
|
||||
return
|
||||
self._user_notified_about_crash = True
|
||||
|
||||
@ -589,10 +601,10 @@ class YouCompleteMe( object ):
|
||||
extra_data = {}
|
||||
self._AddExtraConfDataIfNeeded( extra_data )
|
||||
debug_info += FormatDebugInfoResponse( SendDebugInfoRequest( extra_data ) )
|
||||
debug_info += (
|
||||
'Server running at: {0}\n'
|
||||
'Server process ID: {1}\n'.format( BaseRequest.server_location,
|
||||
self._server_popen.pid ) )
|
||||
debug_info += 'Server running at: {0}\n'.format(
|
||||
BaseRequest.server_location )
|
||||
if self._server_popen:
|
||||
debug_info += 'Server process ID: {0}\n'.format( self._server_popen.pid )
|
||||
if self._server_stdout and self._server_stderr:
|
||||
debug_info += ( 'Server logfiles:\n'
|
||||
' {0}\n'
|
||||
|
Loading…
Reference in New Issue
Block a user