Merge pull request #1496 from centricular/fix-internal-dep-order

Preserve internal-dep include order in build target dependencies

9 files changed, 81 insertions, 30 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 ede0ef7..a167c46 100644
--- a/mesonbuild/backend/ninjabackend.py
+++ b/mesonbuild/backend/ninjabackend.py
@@ -1822,10 +1822,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 07306aa..929fa93 100644
--- a/mesonbuild/backend/vs2010backend.py
+++ b/mesonbuild/backend/vs2010backend.py
@@ -751,12 +751,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
@@ -806,7 +809,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/mesonbuild/compilers.py b/mesonbuild/compilers.py
index 519a7d5..85b56a6 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
diff --git a/run_unittests.py b/run_unittests.py
index d2be970..a11e3a5 100755
--- a/run_unittests.py
+++ b/run_unittests.py
@@ -433,7 +433,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:
@@ -713,20 +726,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")
@@ -744,6 +755,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):
         '''
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('.'))