Merge pull request #1545 from centricular/dont-link-recursively

Don't add dependencies recursively while linking

3 files changed, 78 insertions, 42 deletions

diff --git a/mesonbuild/backend/backends.py b/mesonbuild/backend/backends.py
index 4cfdd9e..3044ce6 100644
--- a/mesonbuild/backend/backends.py
+++ b/mesonbuild/backend/backends.py
@@ -453,15 +453,11 @@ class Backend:
         for d in deps:
             if not isinstance(d, (build.StaticLibrary, build.SharedLibrary)):
                 raise RuntimeError('Tried to link with a non-library target "%s".' % d.get_basename())
-            if isinstance(compiler, compilers.LLVMDCompiler) or isinstance(compiler, compilers.DmdDCompiler):
-                args += ['-L' + self.get_target_filename_for_linking(d)]
+            if isinstance(compiler, (compilers.LLVMDCompiler, compilers.DmdDCompiler)):
+                d_arg = '-L' + self.get_target_filename_for_linking(d)
             else:
-                args.append(self.get_target_filename_for_linking(d))
-            # If you have executable e that links to shared lib s1 that links to shared library s2
-            # you have to specify s2 as well as s1 when linking e even if e does not directly use
-            # s2. Gcc handles this case fine but Clang does not for some reason. Thus we need to
-            # explictly specify all libraries every time.
-            args += self.build_target_link_arguments(compiler, d.get_dependencies())
+                d_arg = self.get_target_filename_for_linking(d)
+            args.append(d_arg)
         return args
 
     def determine_windows_extra_paths(self, target):
diff --git a/mesonbuild/backend/ninjabackend.py b/mesonbuild/backend/ninjabackend.py
index 8624d66..40a05bf 100644
--- a/mesonbuild/backend/ninjabackend.py
+++ b/mesonbuild/backend/ninjabackend.py
@@ -12,6 +12,11 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import shlex
+import os, sys, pickle, re
+import subprocess, shutil
+from collections import OrderedDict
+
 from . import backends
 from .. import modules
 from .. import environment, mesonlib
@@ -24,16 +29,13 @@ from ..mesonlib import File, MesonException, OrderedSet
 from ..mesonlib import get_meson_script, get_compiler_for_source
 from .backends import CleanTrees, InstallData
 from ..build import InvalidArguments
-import os, sys, pickle, re
-import subprocess, shutil
-from collections import OrderedDict
 
 if mesonlib.is_windows():
-    quote_char = '"'
+    quote_func = lambda s: '"{}"'.format(s)
     execute_wrapper = 'cmd /c'
     rmfile_prefix = 'del /f /s /q {} &&'
 else:
-    quote_char = "'"
+    quote_func = shlex.quote
     execute_wrapper = ''
     rmfile_prefix = 'rm -f {} &&'
 
@@ -58,22 +60,22 @@ class NinjaBuildElement:
             self.infilenames = [infilenames]
         else:
             self.infilenames = infilenames
-        self.deps = []
-        self.orderdeps = []
+        self.deps = set()
+        self.orderdeps = set()
         self.elems = []
         self.all_outputs = all_outputs
 
     def add_dep(self, dep):
         if isinstance(dep, list):
-            self.deps += dep
+            self.deps.update(dep)
         else:
-            self.deps.append(dep)
+            self.deps.add(dep)
 
     def add_orderdep(self, dep):
         if isinstance(dep, list):
-            self.orderdeps += dep
+            self.orderdeps.update(dep)
         else:
-            self.orderdeps.append(dep)
+            self.orderdeps.add(dep)
 
     def add_item(self, name, elems):
         if isinstance(elems, str):
@@ -105,18 +107,17 @@ class NinjaBuildElement:
             (name, elems) = e
             should_quote = name not in raw_names
             line = ' %s = ' % name
-            q_templ = quote_char + "%s" + quote_char
             noq_templ = "%s"
             newelems = []
             for i in elems:
                 if not should_quote or i == '&&': # Hackety hack hack
-                    templ = noq_templ
+                    quoter = ninja_quote
                 else:
-                    templ = q_templ
+                    quoter = lambda x: ninja_quote(quote_func(x))
                 i = i.replace('\\', '\\\\')
-                if quote_char == '"':
+                if quote_func('') == '""':
                     i = i.replace('"', '\\"')
-                newelems.append(templ % ninja_quote(i))
+                newelems.append(quoter(i))
             line += ' '.join(newelems)
             line += '\n'
             outfile.write(line)
@@ -854,12 +855,12 @@ int dummy;
         outfile.write(' depfile = $DEPFILE\n')
         outfile.write(' restat = 1\n\n')
         outfile.write('rule REGENERATE_BUILD\n')
-        c = (quote_char + ninja_quote(sys.executable) + quote_char,
-             quote_char + ninja_quote(self.environment.get_build_command()) + quote_char,
+        c = (ninja_quote(quote_func(sys.executable)),
+             ninja_quote(quote_func(self.environment.get_build_command())),
              '--internal',
              'regenerate',
-             quote_char + ninja_quote(self.environment.get_source_dir()) + quote_char,
-             quote_char + ninja_quote(self.environment.get_build_dir()) + quote_char)
+             ninja_quote(quote_func(self.environment.get_source_dir())),
+             ninja_quote(quote_func(self.environment.get_build_dir())))
         outfile.write(" command = %s %s %s %s %s %s --backend ninja\n" % c)
         outfile.write(' description = Regenerating build files.\n')
         outfile.write(' generator = 1\n\n')
@@ -1515,7 +1516,7 @@ rule FORTRAN_DEP_HACK
                 pass
         return []
 
-    def generate_compile_rule_for(self, langname, compiler, qstr, is_cross, outfile):
+    def generate_compile_rule_for(self, langname, compiler, is_cross, outfile):
         if langname == 'java':
             if not is_cross:
                 self.generate_java_compile_rule(compiler, outfile)
@@ -1547,7 +1548,7 @@ rule FORTRAN_DEP_HACK
         quoted_depargs = []
         for d in depargs:
             if d != '$out' and d != '$in':
-                d = qstr % d
+                d = quote_func(d)
             quoted_depargs.append(d)
         cross_args = self.get_cross_info_lang_args(langname, is_cross)
         if mesonlib.is_windows():
@@ -1576,7 +1577,7 @@ rule FORTRAN_DEP_HACK
         outfile.write(description)
         outfile.write('\n')
 
-    def generate_pch_rule_for(self, langname, compiler, qstr, is_cross, outfile):
+    def generate_pch_rule_for(self, langname, compiler, is_cross, outfile):
         if langname != 'c' and langname != 'cpp':
             return
         if is_cross:
@@ -1595,7 +1596,7 @@ rule FORTRAN_DEP_HACK
         quoted_depargs = []
         for d in depargs:
             if d != '$out' and d != '$in':
-                d = qstr % d
+                d = quote_func(d)
             quoted_depargs.append(d)
         if compiler.get_id() == 'msvc':
             output = ''
@@ -1621,12 +1622,11 @@ rule FORTRAN_DEP_HACK
         outfile.write('\n')
 
     def generate_compile_rules(self, outfile):
-        qstr = quote_char + "%s" + quote_char
         for langname, compiler in self.build.compilers.items():
             if compiler.get_id() == 'clang':
                 self.generate_llvm_ir_compile_rule(compiler, False, outfile)
-            self.generate_compile_rule_for(langname, compiler, qstr, False, outfile)
-            self.generate_pch_rule_for(langname, compiler, qstr, False, outfile)
+            self.generate_compile_rule_for(langname, compiler, False, outfile)
+            self.generate_pch_rule_for(langname, compiler, False, outfile)
         if self.environment.is_cross_build():
             # In case we are going a target-only build, make the native compilers
             # masquerade as cross compilers.
@@ -1637,8 +1637,8 @@ rule FORTRAN_DEP_HACK
             for langname, compiler in cclist.items():
                 if compiler.get_id() == 'clang':
                     self.generate_llvm_ir_compile_rule(compiler, True, outfile)
-                self.generate_compile_rule_for(langname, compiler, qstr, True, outfile)
-                self.generate_pch_rule_for(langname, compiler, qstr, True, outfile)
+                self.generate_compile_rule_for(langname, compiler, True, outfile)
+                self.generate_pch_rule_for(langname, compiler, True, outfile)
         outfile.write('\n')
 
     def generate_generator_list_rules(self, target, outfile):
diff --git a/mesonbuild/compilers.py b/mesonbuild/compilers.py
index 0340ed2..06b0a59 100644
--- a/mesonbuild/compilers.py
+++ b/mesonbuild/compilers.py
@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import re
 import shutil
 import contextlib
 import subprocess, os.path
@@ -385,10 +386,15 @@ class CompilerArgs(list):
     # Arg prefixes that override by prepending instead of appending
     prepend_prefixes = ('-I', '-L')
     # Arg prefixes and args that must be de-duped by returning 2
-    dedup2_prefixes = ('-I', '-L', '-D')
+    dedup2_prefixes = ('-I', '-L', '-D', '-U')
+    dedup2_suffixes = ()
     dedup2_args = ()
     # Arg prefixes and args that must be de-duped by returning 1
-    dedup1_prefixes = ()
+    dedup1_prefixes = ('-l',)
+    dedup1_suffixes = ('.lib', '.dll', '.so', '.dylib', '.a')
+    # Match a .so of the form path/to/libfoo.so.0.1.0
+    # Only UNIX shared libraries require this. Others have a fixed extension.
+    dedup1_regex = re.compile(r'([\/\\]|\A)lib.*\.so(\.[0-9]+)?(\.[0-9]+)?(\.[0-9]+)?$')
     dedup1_args = ('-c', '-S', '-E', '-pipe', '-pthread')
     compiler = None
 
@@ -426,7 +432,7 @@ class CompilerArgs(list):
     def _can_dedup(cls, arg):
         '''
         Returns whether the argument can be safely de-duped. This is dependent
-        on two things:
+        on three things:
 
         a) Whether an argument can be 'overriden' by a later argument.  For
            example, -DFOO defines FOO and -UFOO undefines FOO. In this case, we
@@ -440,10 +446,20 @@ class CompilerArgs(list):
            a particular argument is present. This can matter for symbol
            resolution in static or shared libraries, so we cannot de-dup or
            reorder them. For these we return `0`. This is the default.
+
+        In addition to these, we handle library arguments specially.
+        With GNU ld, we surround library arguments with -Wl,--start/end-group
+        to recursively search for symbols in the libraries. This is not needed
+        with other linkers.
         '''
-        if arg.startswith(cls.dedup2_prefixes) or arg in cls.dedup2_args:
+        if arg in cls.dedup2_args or \
+           arg.startswith(cls.dedup2_prefixes) or \
+           arg.endswith(cls.dedup2_suffixes):
             return 2
-        if arg.startswith(cls.dedup1_prefixes) or arg in cls.dedup1_args:
+        if arg in cls.dedup1_args or \
+           arg.startswith(cls.dedup1_prefixes) or \
+           arg.endswith(cls.dedup1_suffixes) or \
+           re.search(cls.dedup1_regex, arg):
             return 1
         return 0
 
@@ -454,6 +470,21 @@ class CompilerArgs(list):
         return False
 
     def to_native(self):
+        # Check if we need to add --start/end-group for circular dependencies
+        # between static libraries.
+        if get_compiler_uses_gnuld(self.compiler):
+            group_started = False
+            for each in self:
+                if not each.startswith('-l') and not each.endswith('.a'):
+                    continue
+                i = self.index(each)
+                if not group_started:
+                    # First occurance of a library
+                    self.insert(i, '-Wl,--start-group')
+                    group_started = True
+            # Last occurance of a library
+            if group_started:
+                self.insert(i + 1, '-Wl,--end-group')
         return self.compiler.unix_args_to_native(self)
 
     def __add__(self, args):
@@ -2412,6 +2443,15 @@ def get_compiler_is_linuxlike(compiler):
         return True
     return False
 
+def get_compiler_uses_gnuld(c):
+    # FIXME: Perhaps we should detect the linker in the environment?
+    # FIXME: Assumes that *BSD use GNU ld, but they might start using lld soon
+    if (getattr(c, 'gcc_type', None) in (GCC_STANDARD, GCC_MINGW, GCC_CYGWIN)) or \
+       (getattr(c, 'clang_type', None) in (CLANG_STANDARD, CLANG_WIN)) or \
+       (getattr(c, 'icc_type', None) in (ICC_STANDARD, ICC_WIN)):
+        return True
+    return False
+
 def get_largefile_args(compiler):
     '''
     Enable transparent large-file-support for 32-bit UNIX systems