Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix g:ycm_roslyn_binary_path being ignored #1721

Merged
merged 1 commit into from
Dec 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 22 additions & 13 deletions ycmd/completers/cs/cs_completer.py
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ def DebugInfo( self, request_data ):
omnisharp_server = responses.DebugInfoServer(
name = 'OmniSharp',
handle = completer._omnisharp_phandle,
executable = PATH_TO_ROSLYN_OMNISHARP,
executable = ' '.join( completer._ConstructOmnisharpCommand() ),
address = 'localhost',
port = completer._omnisharp_port,
logfiles = [ completer._filename_stdout, completer._filename_stderr ],
Expand Down Expand Up @@ -398,6 +398,7 @@ def __init__( self,
self._keep_logfiles = keep_logfiles
self._filename_stderr = None
self._filename_stdout = None
self._omnisharp_command = None
self._omnisharp_port = None
self._omnisharp_phandle = None
self._desired_omnisharp_port = desired_omnisharp_port
Expand All @@ -420,6 +421,24 @@ def _StartServer( self ):
return self._StartServerNoLock()


def _ConstructOmnisharpCommand( self ):
if self._omnisharp_command:
return self._omnisharp_command

self._ChooseOmnisharpPort()
self._omnisharp_command = [ self._roslyn_path,
'-p',
str( self._omnisharp_port ),
'-s',
str( self._solution_path ) ]

if ( not utils.OnWindows()
and self._roslyn_path.endswith( '.exe' ) ):
self._omnisharp_command.insert( 0, self._mono_path )

return self._omnisharp_command


def _StartServerNoLock( self ):
""" Start the OmniSharp server if not already running. Use a lock to avoid
starting the server multiple times for the same solution. """
Expand All @@ -429,18 +448,7 @@ def _StartServerNoLock( self ):
LOGGER.info( 'Starting OmniSharp server' )
LOGGER.info( 'Loading solution file %s', self._solution_path )

self._ChooseOmnisharpPort()

command = [ PATH_TO_OMNISHARP_ROSLYN_BINARY,
'-p',
str( self._omnisharp_port ),
'-s',
str( self._solution_path ) ]

if ( not utils.OnWindows()
and self._roslyn_path.endswith( '.exe' ) ):
command.insert( 0, self._mono_path )

command = self._ConstructOmnisharpCommand()
LOGGER.info( 'Starting OmniSharp server with: %s', command )

solutionfile = os.path.basename( self._solution_path )
Expand Down Expand Up @@ -513,6 +521,7 @@ def _ForceStopServer( self ):


def _CleanUp( self ):
self._omnisharp_command = None
self._omnisharp_port = None
self._omnisharp_phandle = None
if not self._keep_logfiles:
Expand Down
33 changes: 25 additions & 8 deletions ycmd/tests/cs/debug_info_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,19 @@

from hamcrest import ( assert_that, contains_exactly, empty, equal_to,
has_entries, has_entry, instance_of )

from subprocess import Popen as _mockable_popen

from unittest.mock import patch
from unittest import TestCase

from ycmd.completers.cs.cs_completer import PATH_TO_OMNISHARP_ROSLYN_BINARY
from ycmd.completers.cs.hook import GetCompleter
from ycmd.tests.cs import setUpModule, tearDownModule # noqa
from ycmd.tests.cs import PathToTestFile, SharedYcmd
from ycmd.tests.cs import ( PathToTestFile,
SharedYcmd,
IsolatedYcmd,
WrapOmniSharpServer )
from ycmd.tests.test_utils import ( BuildRequest,
WaitUntilCompleterServerReady )
from ycmd import user_options_store
Expand Down Expand Up @@ -197,14 +204,24 @@ def test_GetCompleter_RoslynNotFound( *args ):
assert_that( not GetCompleter( user_options_store.GetAll() ) )


@patch( 'ycmd.completers.cs.cs_completer.FindExecutableWithFallback',
wraps = lambda x, fb: x if x == 'roslyn' else fb )
@patch( 'os.path.isfile', return_value = True )
def test_GetCompleter_RoslynFromUserOption( *args ):
user_options = user_options_store.GetAll().copy(
roslyn_binary_path = 'roslyn' )
assert_that( GetCompleter( user_options )._roslyn_path,
equal_to( 'roslyn' ) )
@IsolatedYcmd( { 'roslyn_binary_path': 'my_roslyn.exe' } )
def test_GetCompleter_RoslynFromUserOption( self, app, *args ):
# `@patch` does not play nice with functions defined at class scope
def _popen_mock( cmdline, **kwargs ):
exe_index = 1 if cmdline[ 0 ].endswith( 'mono' ) else 0
assert_that( cmdline[ exe_index ], equal_to( 'my_roslyn.exe' ) )
# Need to redirect to real binary to allow test to pass
cmdline[ exe_index ] = PATH_TO_OMNISHARP_ROSLYN_BINARY
return _mockable_popen( cmdline, **kwargs )

filepath = PathToTestFile( 'testy', 'Program.cs' )
with patch( 'subprocess.Popen', wraps = _popen_mock ) as popen_mock:
with WrapOmniSharpServer( app, filepath ):
request = BuildRequest( filepath = filepath, filetype = 'cs' )
app.post_json( '/debug_info', request )

popen_mock.assert_called()


@patch( 'os.path.isfile', return_value = False )
Expand Down