From 92991db8f2938d35bbca3056b64cd13bd64133da Mon Sep 17 00:00:00 2001 From: micbou Date: Mon, 17 Jul 2017 18:43:44 +0200 Subject: [PATCH] Make relative paths in flags from extra conf absolute --- README.md | 4 + cpp/ycm/.ycm_extra_conf.py | 47 ++-------- ycmd/completers/cpp/flags.py | 12 ++- ycmd/tests/clang/flags_test.py | 152 +++++++++++++++++++++++---------- 4 files changed, 125 insertions(+), 90 deletions(-) diff --git a/README.md b/README.md index 158ddb9c1b..00e2339b76 100644 --- a/README.md +++ b/README.md @@ -220,6 +220,10 @@ The return value must be one of the following: - `flags`: (mandatory) a list of compiler flags. + - `working_directory': (optional) the directory used to convert relative paths + in the list of flags to absolute. Defaults to the directory containing the + `.ycm_extra_conf.py` file. + - `do_cache`: (optional) a boolean indicating whether or not the result of this call (i.e. the list of flags) should be cached for this file name. Defaults to `True`. If unsure, the default is almost always correct. diff --git a/cpp/ycm/.ycm_extra_conf.py b/cpp/ycm/.ycm_extra_conf.py index 93c3617fd8..8561c6ebb2 100644 --- a/cpp/ycm/.ycm_extra_conf.py +++ b/cpp/ycm/.ycm_extra_conf.py @@ -102,39 +102,6 @@ SOURCE_EXTENSIONS = [ '.cpp', '.cxx', '.cc', '.c', '.m', '.mm' ] -def DirectoryOfThisScript(): - return os.path.dirname( os.path.abspath( __file__ ) ) - - -def MakeRelativePathsInFlagsAbsolute( flags, working_directory ): - if not working_directory: - return list( flags ) - new_flags = [] - make_next_absolute = False - path_flags = [ '-isystem', '-I', '-iquote', '--sysroot=' ] - for flag in flags: - new_flag = flag - - if make_next_absolute: - make_next_absolute = False - if not flag.startswith( '/' ): - new_flag = os.path.join( working_directory, flag ) - - for path_flag in path_flags: - if flag == path_flag: - make_next_absolute = True - break - - if flag.startswith( path_flag ): - path = flag[ len( path_flag ): ] - new_flag = path_flag + os.path.join( working_directory, path ) - break - - if new_flag: - new_flags.append( new_flag ) - return new_flags - - def IsHeaderFile( filename ): extension = os.path.splitext( filename )[ 1 ] return extension in [ '.h', '.hxx', '.hpp', '.hh' ] @@ -166,10 +133,6 @@ def FlagsForFile( filename, **kwargs ): if not compilation_info: return None - final_flags = MakeRelativePathsInFlagsAbsolute( - compilation_info.compiler_flags_, - compilation_info.compiler_working_dir_ ) - # NOTE: This is just for YouCompleteMe; it's highly likely that your project # does NOT need to remove the stdlib flag. DO NOT USE THIS IN YOUR # ycm_extra_conf IF YOU'RE NOT 100% SURE YOU NEED IT. @@ -177,8 +140,10 @@ def FlagsForFile( filename, **kwargs ): final_flags.remove( '-stdlib=libc++' ) except ValueError: pass - else: - relative_to = DirectoryOfThisScript() - final_flags = MakeRelativePathsInFlagsAbsolute( flags, relative_to ) - return { 'flags': final_flags } + return { + 'flags': compilation_info.compiler_flags_, + 'working_directory': compilation_info.compiler_working_dir_ + } + + return { 'flags': flags } diff --git a/ycmd/completers/cpp/flags.py b/ycmd/completers/cpp/flags.py index e85e6093fb..eb1d52ada2 100644 --- a/ycmd/completers/cpp/flags.py +++ b/ycmd/completers/cpp/flags.py @@ -290,9 +290,17 @@ def _CallExtraConfFlagsForFile( module, filename, client_data ): # For the sake of backwards compatibility, we need to first check whether the # FlagsForFile function in the extra conf module even allows keyword args. if inspect.getargspec( module.FlagsForFile ).keywords: - return module.FlagsForFile( filename, client_data = client_data ) + flags = module.FlagsForFile( filename, client_data = client_data ) else: - return module.FlagsForFile( filename ) + flags = module.FlagsForFile( filename ) + + # If no working directory is specified, consider the paths to be relative to + # the directory containing the .ycm_extra_conf.py file. + working_directory = flags.get( 'working_directory', + os.path.dirname( inspect.getfile( module ) ) ) + flags[ 'flags' ] = _MakeRelativePathsInFlagsAbsolute( flags[ 'flags' ], + working_directory ) + return flags def _SysRootSpecifedIn( flags ): diff --git a/ycmd/tests/clang/flags_test.py b/ycmd/tests/clang/flags_test.py index f72c0a0073..27d5ffaaa6 100644 --- a/ycmd/tests/clang/flags_test.py +++ b/ycmd/tests/clang/flags_test.py @@ -22,11 +22,13 @@ # Not installing aliases from python-future; it's unreliable and slow. from builtins import * # noqa +import contextlib import os from nose.tools import eq_, ok_ from ycmd.completers.cpp import flags -from mock import patch, Mock +from mock import patch, MagicMock, Mock +from types import ModuleType from ycmd.tests.test_utils import MacOnly from ycmd.responses import NoExtraConfDetected from ycmd.tests.clang import TemporaryClangProject, TemporaryClangTestDir @@ -34,90 +36,146 @@ from hamcrest import assert_that, calling, contains, has_item, not_, raises -@patch( 'ycmd.extra_conf_store.ModuleForSourceFile', return_value = Mock() ) -def FlagsForFile_FlagsNotReady_test( *args ): - fake_flags = { - 'flags': [ ], - 'flags_ready': False - } +@contextlib.contextmanager +def MockExtraConfModule( flags_for_file_function, + pathname = '.ycm_extra_conf.py' ): + module = MagicMock( spec = ModuleType ) + module.__file__ = pathname + module.FlagsForFile = flags_for_file_function + with patch( 'ycmd.extra_conf_store.ModuleForSourceFile', + return_value = module ): + yield + + +def FlagsForFile_FlagsNotReady_test(): + flags_object = flags.Flags() + + def FlagsForFile( filename ): + return { + 'flags': [], + 'flags_ready': False + } - with patch( 'ycmd.completers.cpp.flags._CallExtraConfFlagsForFile', - return_value = fake_flags ): - flags_object = flags.Flags() + with MockExtraConfModule( FlagsForFile ): flags_list = flags_object.FlagsForFile( '/foo', False ) - eq_( list( flags_list ), [ ] ) + eq_( list( flags_list ), [] ) @patch( 'ycmd.extra_conf_store.ModuleForSourceFile', return_value = Mock() ) def FlagsForFile_BadNonUnicodeFlagsAreAlsoRemoved_test( *args ): - fake_flags = { - 'flags': [ bytes( b'-c' ), '-c', bytes( b'-foo' ), '-bar' ] - } + flags_object = flags.Flags() - with patch( 'ycmd.completers.cpp.flags._CallExtraConfFlagsForFile', - return_value = fake_flags ): - flags_object = flags.Flags() + def FlagsForFile( filename ): + return { + 'flags': [ bytes( b'-c' ), '-c', bytes( b'-foo' ), '-bar' ] + } + + with MockExtraConfModule( FlagsForFile ): flags_list = flags_object.FlagsForFile( '/foo', False ) eq_( list( flags_list ), [ '-foo', '-bar' ] ) -@patch( 'ycmd.extra_conf_store.ModuleForSourceFile', return_value = Mock() ) -def FlagsForFile_FlagsCachedByDefault_test( *args ): +def FlagsForFile_FlagsCachedByDefault_test(): flags_object = flags.Flags() - results = { 'flags': [ '-x', 'c' ] } - with patch( 'ycmd.completers.cpp.flags._CallExtraConfFlagsForFile', - return_value = results ): + def FlagsForFile( filename ): + return { + 'flags': [ '-x', 'c' ] + } + + with MockExtraConfModule( FlagsForFile ): flags_list = flags_object.FlagsForFile( '/foo', False ) assert_that( flags_list, contains( '-x', 'c' ) ) - results[ 'flags' ] = [ '-x', 'c++' ] - with patch( 'ycmd.completers.cpp.flags._CallExtraConfFlagsForFile', - return_value = results ): + def FlagsForFile( filename ): + return { + 'flags': [ '-x', 'c++' ] + } + + with MockExtraConfModule( FlagsForFile ): flags_list = flags_object.FlagsForFile( '/foo', False ) assert_that( flags_list, contains( '-x', 'c' ) ) -@patch( 'ycmd.extra_conf_store.ModuleForSourceFile', return_value = Mock() ) -def FlagsForFile_FlagsNotCachedWhenDoCacheIsFalse_test( *args ): +def FlagsForFile_FlagsNotCachedWhenDoCacheIsFalse_test(): flags_object = flags.Flags() - results = { - 'flags': [ '-x', 'c' ], - 'do_cache': False - } - with patch( 'ycmd.completers.cpp.flags._CallExtraConfFlagsForFile', - return_value = results ): + def FlagsForFile( filename ): + return { + 'flags': [ '-x', 'c' ], + 'do_cache': False + } + + with MockExtraConfModule( FlagsForFile ): flags_list = flags_object.FlagsForFile( '/foo', False ) assert_that( flags_list, contains( '-x', 'c' ) ) - results[ 'flags' ] = [ '-x', 'c++' ] - with patch( 'ycmd.completers.cpp.flags._CallExtraConfFlagsForFile', - return_value = results ): + def FlagsForFile( filename ): + return { + 'flags': [ '-x', 'c++' ] + } + + with MockExtraConfModule( FlagsForFile ): flags_list = flags_object.FlagsForFile( '/foo', False ) assert_that( flags_list, contains( '-x', 'c++' ) ) -@patch( 'ycmd.extra_conf_store.ModuleForSourceFile', return_value = Mock() ) -def FlagsForFile_FlagsCachedWhenDoCacheIsTrue_test( *args ): +def FlagsForFile_FlagsCachedWhenDoCacheIsTrue_test(): flags_object = flags.Flags() - results = { - 'flags': [ '-x', 'c' ], - 'do_cache': True - } - with patch( 'ycmd.completers.cpp.flags._CallExtraConfFlagsForFile', - return_value = results ): + def FlagsForFile( filename ): + return { + 'flags': [ '-x', 'c' ], + 'do_cache': True + } + + with MockExtraConfModule( FlagsForFile ): flags_list = flags_object.FlagsForFile( '/foo', False ) assert_that( flags_list, contains( '-x', 'c' ) ) - results[ 'flags' ] = [ '-x', 'c++' ] - with patch( 'ycmd.completers.cpp.flags._CallExtraConfFlagsForFile', - return_value = results ): + def FlagsForFile( filename ): + return { + 'flags': [ '-x', 'c++' ] + } + + with MockExtraConfModule( FlagsForFile ): flags_list = flags_object.FlagsForFile( '/foo', False ) assert_that( flags_list, contains( '-x', 'c' ) ) +def FlagsForFile_FlagsRelativeToExtraConfDirectory_test(): + flags_object = flags.Flags() + + def FlagsForFile( filename ): + return { + 'flags': [ '-x', 'c', '-I', 'header' ] + } + + with MockExtraConfModule( FlagsForFile, + pathname = '/project_root/.ycm_extra_conf.py' ): + flags_list = flags_object.FlagsForFile( '/foo', False ) + assert_that( flags_list, + contains( '-x', 'c', + '-I', os.path.normpath( '/project_root/header' ) ) ) + + +def FlagsForFile_FlagsRelativeToWorkingDirectory_test(): + flags_object = flags.Flags() + + def FlagsForFile( filename ): + return { + 'flags': [ '-x', 'c', '-I', 'header' ], + 'working_directory': '/working_dir/' + } + + with MockExtraConfModule( FlagsForFile, + pathname = '/project_root/.ycm_extra_conf.py' ): + flags_list = flags_object.FlagsForFile( '/foo', False ) + assert_that( flags_list, + contains( '-x', 'c', + '-I', os.path.normpath( '/working_dir/header' ) ) ) + + def RemoveUnusedFlags_Passthrough_test(): eq_( [ '-foo', '-bar' ], flags._RemoveUnusedFlags( [ '-foo', '-bar' ], 'file' ) )