From d4829f1ef941df59b2e28fe7d4f4b30e9767a5b7 Mon Sep 17 00:00:00 2001 From: nop00 Date: Wed, 4 Dec 2013 22:01:35 +0100 Subject: [PATCH 1/7] Fix OmniSharp launch under Windows --- python/ycm/completers/cs/cs_completer.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/python/ycm/completers/cs/cs_completer.py b/python/ycm/completers/cs/cs_completer.py index 9e2147bc..474c6289 100755 --- a/python/ycm/completers/cs/cs_completer.py +++ b/python/ycm/completers/cs/cs_completer.py @@ -150,14 +150,13 @@ class CsharpCompleter( Completer ): if not os.path.isfile( omnisharp ): raise RuntimeError( SERVER_NOT_FOUND_MSG.format( omnisharp ) ) - if not platform.startswith( 'win' ): - omnisharp = 'mono ' + omnisharp - path_to_solutionfile = os.path.join( folder, solutionfile ) - # command has to be provided as one string for some reason - command = [ omnisharp + ' -p ' + str( self._omnisharp_port ) + ' -s ' + + command = [ omnisharp, '-p', str( self._omnisharp_port ), '-s', path_to_solutionfile ] + if not platform.startswith( 'win' ): + command.insert(0, 'mono') + filename_format = os.path.join( utils.PathToTempDir(), 'omnisharp_{port}_{sln}_{std}.log' ) @@ -168,7 +167,7 @@ class CsharpCompleter( Completer ): with open( self._filename_stderr, 'w' ) as fstderr: with open( self._filename_stdout, 'w' ) as fstdout: - subprocess.Popen( command, stdout=fstdout, stderr=fstderr, shell=True ) + subprocess.Popen( command, stdout=fstdout, stderr=fstderr ) self._logger.info( 'Starting OmniSharp server' ) From 9b8781322ef3d18430c3da37e9d6b1be3b832f8b Mon Sep 17 00:00:00 2001 From: nop Date: Thu, 5 Dec 2013 10:25:55 +0100 Subject: [PATCH 2/7] Small refactoring Use the dedicated function to check if we're running on Windows instead of checking the platform directly. --- python/ycm/completers/cs/cs_completer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ycm/completers/cs/cs_completer.py b/python/ycm/completers/cs/cs_completer.py index 474c6289..01a26108 100755 --- a/python/ycm/completers/cs/cs_completer.py +++ b/python/ycm/completers/cs/cs_completer.py @@ -154,7 +154,7 @@ class CsharpCompleter( Completer ): command = [ omnisharp, '-p', str( self._omnisharp_port ), '-s', path_to_solutionfile ] - if not platform.startswith( 'win' ): + if utils.OnWindows(): command.insert(0, 'mono') filename_format = os.path.join( utils.PathToTempDir(), From ee90d9b09bbed8cbf69114709fffb9ae0fefc1cf Mon Sep 17 00:00:00 2001 From: nop Date: Tue, 10 Dec 2013 12:35:33 +0100 Subject: [PATCH 3/7] Fix faulty logic in previous commit Previous commit is 9b8781322ef3d18430c3da37e9d6b1be3b832f8b --- python/ycm/completers/cs/cs_completer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ycm/completers/cs/cs_completer.py b/python/ycm/completers/cs/cs_completer.py index 01a26108..fe5eff5a 100755 --- a/python/ycm/completers/cs/cs_completer.py +++ b/python/ycm/completers/cs/cs_completer.py @@ -154,7 +154,7 @@ class CsharpCompleter( Completer ): command = [ omnisharp, '-p', str( self._omnisharp_port ), '-s', path_to_solutionfile ] - if utils.OnWindows(): + if not utils.OnWindows(): command.insert(0, 'mono') filename_format = os.path.join( utils.PathToTempDir(), From fd6338fa88f4b7111fbad1e31da74681bfb5e7ab Mon Sep 17 00:00:00 2001 From: nop Date: Tue, 10 Dec 2013 12:46:05 +0100 Subject: [PATCH 4/7] Fixed OmniSharp launch under Windows (again) We pass shell=True to Popen so that OmniSharp is not started inside a new visible window under Windows. And since we use shell=True, we pass the command to execute as a string, as recommended by Python's docs (also, it won't work when passed as a sequence anyway :) ). --- python/ycm/completers/cs/cs_completer.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/python/ycm/completers/cs/cs_completer.py b/python/ycm/completers/cs/cs_completer.py index fe5eff5a..9e1e4eaf 100755 --- a/python/ycm/completers/cs/cs_completer.py +++ b/python/ycm/completers/cs/cs_completer.py @@ -151,11 +151,13 @@ class CsharpCompleter( Completer ): raise RuntimeError( SERVER_NOT_FOUND_MSG.format( omnisharp ) ) path_to_solutionfile = os.path.join( folder, solutionfile ) - command = [ omnisharp, '-p', str( self._omnisharp_port ), '-s', - path_to_solutionfile ] + # we need to pass the command to Popen as a string since we're passing + # shell=True (as recommended by Python's doc) + command = omnisharp + ' -p ' + str( self._omnisharp_port ) + ' -s ' + \ + path_to_solutionfile if not utils.OnWindows(): - command.insert(0, 'mono') + command = 'mono ' + command filename_format = os.path.join( utils.PathToTempDir(), 'omnisharp_{port}_{sln}_{std}.log' ) @@ -167,7 +169,9 @@ class CsharpCompleter( Completer ): with open( self._filename_stderr, 'w' ) as fstderr: with open( self._filename_stdout, 'w' ) as fstdout: - subprocess.Popen( command, stdout=fstdout, stderr=fstderr ) + # shell=True is needed for Windows so OmniSharp does not spawn + # in a new visible window + subprocess.Popen( command, stdout=fstdout, stderr=fstderr, shell=True ) self._logger.info( 'Starting OmniSharp server' ) From 96762e2a7ee13cc228cfb4da059c18779bcc2b75 Mon Sep 17 00:00:00 2001 From: nop Date: Tue, 10 Dec 2013 14:59:44 +0100 Subject: [PATCH 5/7] Removed an unused import --- python/ycm/completers/cs/cs_completer.py | 1 - 1 file changed, 1 deletion(-) diff --git a/python/ycm/completers/cs/cs_completer.py b/python/ycm/completers/cs/cs_completer.py index 9e1e4eaf..653fda22 100755 --- a/python/ycm/completers/cs/cs_completer.py +++ b/python/ycm/completers/cs/cs_completer.py @@ -19,7 +19,6 @@ # along with YouCompleteMe. If not, see . import os -from sys import platform import glob from ycm.completers.completer import Completer from ycm.server import responses From c8b0f466c0c61c2b2be0885cd4b7af8a26541c11 Mon Sep 17 00:00:00 2001 From: nop Date: Wed, 11 Dec 2013 10:27:29 +0100 Subject: [PATCH 6/7] Small stylistic change --- python/ycm/completers/cs/cs_completer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/ycm/completers/cs/cs_completer.py b/python/ycm/completers/cs/cs_completer.py index 653fda22..7553982d 100755 --- a/python/ycm/completers/cs/cs_completer.py +++ b/python/ycm/completers/cs/cs_completer.py @@ -152,8 +152,8 @@ class CsharpCompleter( Completer ): path_to_solutionfile = os.path.join( folder, solutionfile ) # we need to pass the command to Popen as a string since we're passing # shell=True (as recommended by Python's doc) - command = omnisharp + ' -p ' + str( self._omnisharp_port ) + ' -s ' + \ - path_to_solutionfile + command = ( omnisharp + ' -p ' + str( self._omnisharp_port ) + ' -s ' + + path_to_solutionfile ) if not utils.OnWindows(): command = 'mono ' + command From bc9a283be68b8522e90065583b7766fd1060aae0 Mon Sep 17 00:00:00 2001 From: nop Date: Wed, 11 Dec 2013 13:41:04 +0100 Subject: [PATCH 7/7] New wrapper function around subprocess.Popen New wrapper function around subprocess.Popen that handles stdin correctly when on Windows (see issue #637) --- python/ycm/completers/cs/cs_completer.py | 3 +-- python/ycm/utils.py | 16 ++++++++++++++++ python/ycm/youcompleteme.py | 12 ++++-------- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/python/ycm/completers/cs/cs_completer.py b/python/ycm/completers/cs/cs_completer.py index 7553982d..629b7eb6 100755 --- a/python/ycm/completers/cs/cs_completer.py +++ b/python/ycm/completers/cs/cs_completer.py @@ -27,7 +27,6 @@ import urllib2 import urllib import urlparse import json -import subprocess import logging SERVER_NOT_FOUND_MSG = ( 'OmniSharp server binary not found at {0}. ' + @@ -170,7 +169,7 @@ class CsharpCompleter( Completer ): with open( self._filename_stdout, 'w' ) as fstdout: # shell=True is needed for Windows so OmniSharp does not spawn # in a new visible window - subprocess.Popen( command, stdout=fstdout, stderr=fstderr, shell=True ) + utils.SafePopen( command, stdout=fstdout, stderr=fstderr, shell=True ) self._logger.info( 'Starting OmniSharp server' ) diff --git a/python/ycm/utils.py b/python/ycm/utils.py index 124f98fd..584dd7bc 100644 --- a/python/ycm/utils.py +++ b/python/ycm/utils.py @@ -25,6 +25,7 @@ import functools import socket import stat from distutils.spawn import find_executable +import subprocess WIN_PYTHON27_PATH = 'C:\python27\pythonw.exe' WIN_PYTHON26_PATH = 'C:\python26\pythonw.exe' @@ -162,3 +163,18 @@ def AddThirdPartyFoldersToSysPath(): def ForceSemanticCompletion( request_data ): return ( 'force_semantic' in request_data and bool( request_data[ 'force_semantic' ] ) ) + +def SafePopen( args, bufsize = 0, executable = None, stdin = None, + stdout = None, stderr = None, preexec_fn = None, + close_fds = False, shell = False, cwd = None, env = None, + universal_newlines = False, startupinfo = None, + creationflags = 0 ): + if stdin is None: + # We need this on Windows otherwise bad things happen. See issue #637. + stdin = subprocess.PIPE if OnWindows() else None + + return subprocess.Popen( args, bufsize, executable, stdin, stdout, stderr, + preexec_fn, close_fds, shell, cwd, env, + universal_newlines, startupinfo, creationflags ) + + diff --git a/python/ycm/youcompleteme.py b/python/ycm/youcompleteme.py index c84b350a..64c5edaf 100644 --- a/python/ycm/youcompleteme.py +++ b/python/ycm/youcompleteme.py @@ -19,7 +19,6 @@ import os import vim -import subprocess import tempfile import json from ycm import vimsupport @@ -93,7 +92,7 @@ class YouCompleteMe( object ): BaseRequest.server_location = 'http://localhost:' + str( server_port ) if self._user_options[ 'server_use_vim_stdout' ]: - self._server_popen = subprocess.Popen( args ) + self._server_popen = utils.SafePopen( args ) else: filename_format = os.path.join( utils.PathToTempDir(), 'server_{port}_{std}.log' ) @@ -102,15 +101,12 @@ class YouCompleteMe( object ): std = 'stdout' ) self._server_stderr = filename_format.format( port = server_port, std = 'stderr' ) - # We need this on Windows otherwise bad things happen. See issue #637. - stdin = subprocess.PIPE if utils.OnWindows() else None with open( self._server_stderr, 'w' ) as fstderr: with open( self._server_stdout, 'w' ) as fstdout: - self._server_popen = subprocess.Popen( args, - stdin = stdin, - stdout = fstdout, - stderr = fstderr ) + self._server_popen = utils.SafePopen( args, + stdout = fstdout, + stderr = fstderr ) self._NotifyUserIfServerCrashed()