From 8482286236f6d91dfaa02e4254e0e15430e67eb2 Mon Sep 17 00:00:00 2001 From: Nirbheek Chauhan Date: Thu, 23 Mar 2017 08:28:23 +0530 Subject: Test whether internal-dep include order is preserved https://github.com/mesonbuild/meson/issues/1495 --- test cases/common/138 include order/meson.build | 8 ++++++++ test cases/common/138 include order/sub2/meson.build | 1 + test cases/common/138 include order/sub4/meson.build | 2 ++ 3 files changed, 11 insertions(+) diff --git a/test cases/common/138 include order/meson.build b/test cases/common/138 include order/meson.build index f744ae7..c79cb0a 100644 --- a/test cases/common/138 include order/meson.build +++ b/test cases/common/138 include order/meson.build @@ -19,4 +19,12 @@ subdir('sub3') # The directory where the target resides subdir('sub4') +# Test that the order in which internal dependencies are specified is +# preserved. This is needed especially when subprojects get involved and +# multiple build-root config.h files exist, and we must be sure that the +# correct one is found: https://github.com/mesonbuild/meson/issues/1495 +f = executable('somefxe', 'sub4/main.c', + dependencies : [correctinc, dep, wronginc]) + test('eh', e) +test('oh', f) diff --git a/test cases/common/138 include order/sub2/meson.build b/test cases/common/138 include order/sub2/meson.build index 7b49d6a..b1e6190 100644 --- a/test cases/common/138 include order/sub2/meson.build +++ b/test cases/common/138 include order/sub2/meson.build @@ -1 +1,2 @@ j = include_directories('.') +wronginc = declare_dependency(include_directories : j) diff --git a/test cases/common/138 include order/sub4/meson.build b/test cases/common/138 include order/sub4/meson.build index 538899a..ab4c455 100644 --- a/test cases/common/138 include order/sub4/meson.build +++ b/test cases/common/138 include order/sub4/meson.build @@ -2,3 +2,5 @@ e = executable('someexe', 'main.c', c_args : ['-I' + sub3], include_directories : j, dependencies : dep) + +correctinc = declare_dependency(include_directories : include_directories('.')) -- cgit v1.1 From 7f80f81ac9c00ed328566d9aec5140b4f9d1d21d Mon Sep 17 00:00:00 2001 From: Nirbheek Chauhan Date: Thu, 23 Mar 2017 08:32:01 +0530 Subject: Preserve the order of internal deps in a target We were adding them to the CompilerArgs instance in the order in which they are specified, which is wrong because later dependencies would override previous ones. Add them in the reverse order instead. Closes https://github.com/mesonbuild/meson/issues/1495 --- mesonbuild/backend/backends.py | 8 +++-- mesonbuild/backend/ninjabackend.py | 6 ++-- mesonbuild/backend/vs2010backend.py | 9 ++++-- mesonbuild/build.py | 13 ++++---- run_unittests.py | 60 ++++++++++++++++++++++++++++--------- 5 files changed, 68 insertions(+), 28 deletions(-) diff --git a/mesonbuild/backend/backends.py b/mesonbuild/backend/backends.py index 5939488..fa9a82f 100644 --- a/mesonbuild/backend/backends.py +++ b/mesonbuild/backend/backends.py @@ -375,9 +375,11 @@ class Backend: # Set -fPIC for static libraries by default unless explicitly disabled if isinstance(target, build.StaticLibrary) and target.pic: commands += compiler.get_pic_args() - # Add compile args needed to find external dependencies - # Link args are added while generating the link command - for dep in target.get_external_deps(): + # Add compile args needed to find external dependencies. Link args are + # added while generating the link command. + # NOTE: We must preserve the order in which external deps are + # specified, so we reverse the list before iterating over it. + for dep in reversed(target.get_external_deps()): commands += dep.get_compile_args() # Qt needs -fPIC for executables # XXX: We should move to -fPIC for all executables diff --git a/mesonbuild/backend/ninjabackend.py b/mesonbuild/backend/ninjabackend.py index 1acfe78..e0106b2 100644 --- a/mesonbuild/backend/ninjabackend.py +++ b/mesonbuild/backend/ninjabackend.py @@ -1831,10 +1831,12 @@ rule FORTRAN_DEP_HACK # and from `include_directories:` of internal deps of the target. # # Target include dirs should override internal deps include dirs. + # This is handled in BuildTarget.process_kwargs() # # Include dirs from internal deps should override include dirs from - # external deps. - for i in target.get_include_dirs(): + # external deps and must maintain the order in which they are specified. + # Hence, we must reverse the list so that the order is preserved. + for i in reversed(target.get_include_dirs()): basedir = i.get_curdir() for d in i.get_incdirs(): # Avoid superfluous '/.' at the end of paths when d is '.' diff --git a/mesonbuild/backend/vs2010backend.py b/mesonbuild/backend/vs2010backend.py index e1f7325..e512f0e 100644 --- a/mesonbuild/backend/vs2010backend.py +++ b/mesonbuild/backend/vs2010backend.py @@ -714,12 +714,15 @@ class Vs2010Backend(backends.Backend): # and from `include_directories:` of internal deps of the target. # # Target include dirs should override internal deps include dirs. + # This is handled in BuildTarget.process_kwargs() # # Include dirs from internal deps should override include dirs from - # external deps. + # external deps and must maintain the order in which they are + # specified. Hence, we must reverse so that the order is preserved. + # # These are per-target, but we still add them as per-file because we # need them to be looked in first. - for d in target.get_include_dirs(): + for d in reversed(target.get_include_dirs()): for i in d.get_incdirs(): curdir = os.path.join(d.get_curdir(), i) args.append('-I' + self.relpath(curdir, target.subdir)) # build dir @@ -769,7 +772,7 @@ class Vs2010Backend(backends.Backend): # Split compile args needed to find external dependencies # Link args are added while generating the link command - for d in target.get_external_deps(): + for d in reversed(target.get_external_deps()): # Cflags required by external deps might have UNIX-specific flags, # so filter them out if needed d_compile_args = compiler.unix_args_to_native(d.get_compile_args()) diff --git a/mesonbuild/build.py b/mesonbuild/build.py index c7e8f8e..2806331 100644 --- a/mesonbuild/build.py +++ b/mesonbuild/build.py @@ -598,16 +598,17 @@ class BuildTarget(Target): for i in self.link_depends: if not isinstance(i, str): raise InvalidArguments('Link_depends arguments must be strings.') - deplist = kwargs.get('dependencies', []) - if not isinstance(deplist, list): - deplist = [deplist] - self.add_deps(deplist) - # Target-specific include dirs must be added after include dirs from - # internal deps (added inside self.add_deps()) to override correctly. + # Target-specific include dirs must be added BEFORE include dirs from + # internal deps (added inside self.add_deps()) to override them. inclist = kwargs.get('include_directories', []) if not isinstance(inclist, list): inclist = [inclist] self.add_include_dirs(inclist) + # Add dependencies (which also have include_directories) + deplist = kwargs.get('dependencies', []) + if not isinstance(deplist, list): + deplist = [deplist] + self.add_deps(deplist) self.custom_install_dir = kwargs.get('install_dir', None) if self.custom_install_dir is not None: if not isinstance(self.custom_install_dir, str): diff --git a/run_unittests.py b/run_unittests.py index 9945057..bba5bb8 100755 --- a/run_unittests.py +++ b/run_unittests.py @@ -432,7 +432,20 @@ class BasePlatformTests(unittest.TestCase): def get_compdb(self): with open(os.path.join(self.builddir, 'compile_commands.json')) as ifile: - return json.load(ifile) + contents = json.load(ifile) + # If Ninja is using .rsp files, generate them, read their contents, and + # replace it as the command for all compile commands in the parsed json. + if len(contents) > 0 and contents[0]['command'].endswith('.rsp'): + # Pretend to build so that the rsp files are generated + self.build(['-d', 'keeprsp', '-n']) + for each in contents: + # Extract the actual command from the rsp file + compiler, rsp = each['command'].split(' @') + rsp = os.path.join(self.builddir, rsp) + # Replace the command with its contents + with open(rsp, 'r', encoding='utf-8') as f: + each['command'] = compiler + ' ' + f.read() + return contents def get_meson_log(self): with open(os.path.join(self.builddir, 'meson-logs', 'meson-log.txt')) as f: @@ -712,20 +725,18 @@ class AllPlatformTests(BasePlatformTests): def test_internal_include_order(self): testdir = os.path.join(self.common_test_dir, '138 include order') self.init(testdir) + execmd = fxecmd = None for cmd in self.get_compdb(): - if cmd['file'].endswith('/main.c'): - cmd = cmd['command'] - break - else: - raise Exception('Could not find main.c command') - if cmd.endswith('.rsp'): - # Pretend to build so that the rsp files are generated - self.build(['-d', 'keeprsp', '-n']) - # Extract the actual command from the rsp file - rsp = os.path.join(self.builddir, cmd.split('cl @')[1]) - with open(rsp, 'r', encoding='utf-8') as f: - cmd = f.read() - incs = [a for a in shlex.split(cmd) if a.startswith("-I")] + if 'someexe' in cmd['command']: + execmd = cmd['command'] + continue + if 'somefxe' in cmd['command']: + fxecmd = cmd['command'] + continue + if not execmd or not fxecmd: + raise Exception('Could not find someexe and somfxe commands') + # Check include order for 'someexe' + incs = [a for a in shlex.split(execmd) if a.startswith("-I")] self.assertEqual(len(incs), 8) # target private dir self.assertPathEqual(incs[0], "-Isub4/someexe@exe") @@ -743,6 +754,27 @@ class AllPlatformTests(BasePlatformTests): self.assertPathEqual(incs[6], "-Isub1") # target internal dependency include_directories: source dir self.assertPathBasenameEqual(incs[7], 'sub1') + # Check include order for 'somefxe' + incs = [a for a in shlex.split(fxecmd) if a.startswith('-I')] + self.assertEqual(len(incs), 9) + # target private dir + self.assertPathEqual(incs[0], '-Isomefxe@exe') + # target build dir + self.assertPathEqual(incs[1], '-I.') + # target source dir + self.assertPathBasenameEqual(incs[2], os.path.basename(testdir)) + # target internal dependency correct include_directories: build dir + self.assertPathEqual(incs[3], "-Isub4") + # target internal dependency correct include_directories: source dir + self.assertPathBasenameEqual(incs[4], 'sub4') + # target internal dependency dep include_directories: build dir + self.assertPathEqual(incs[5], "-Isub1") + # target internal dependency dep include_directories: source dir + self.assertPathBasenameEqual(incs[6], 'sub1') + # target internal dependency wrong include_directories: build dir + self.assertPathEqual(incs[7], "-Isub2") + # target internal dependency wrong include_directories: source dir + self.assertPathBasenameEqual(incs[8], 'sub2') def test_compiler_detection(self): ''' -- cgit v1.1 From b35a808972a4a51438e0c76f117c557fe3a1ffa1 Mon Sep 17 00:00:00 2001 From: Nirbheek Chauhan Date: Thu, 23 Mar 2017 08:34:42 +0530 Subject: compiler args: Also dedup -pthread since it can't be undone --- mesonbuild/compilers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mesonbuild/compilers.py b/mesonbuild/compilers.py index f8c3f6b..e814693 100644 --- a/mesonbuild/compilers.py +++ b/mesonbuild/compilers.py @@ -369,7 +369,7 @@ class CompilerArgs(list): dedup2_args = () # Arg prefixes and args that must be de-duped by returning 1 dedup1_prefixes = () - dedup1_args = ('-c', '-S', '-E', '-pipe') + dedup1_args = ('-c', '-S', '-E', '-pipe', '-pthread') compiler = None def _check_args(self, args): @@ -413,7 +413,7 @@ class CompilerArgs(list): can safely remove the previous occurance and add a new one. The same is true for include paths and library paths with -I and -L. For these we return `2`. See `dedup2_prefixes` and `dedup2_args`. - b) Arguments that once specifie cannot be undone, such as `-c` or + b) Arguments that once specified cannot be undone, such as `-c` or `-pipe`. New instances of these can be completely skipped. For these we return `1`. See `dedup1_prefixes` and `dedup1_args`. c) Whether it matters where or how many times on the command-line -- cgit v1.1