Merge pull request #12485 from xclaesse/rust-link-regress

rust: Fix linking with C libraries in subdir

16 files changed, 204 insertions, 107 deletions

diff --git a/mesonbuild/backend/ninjabackend.py b/mesonbuild/backend/ninjabackend.py
index 339952400..7ed6b443f 100644
--- a/mesonbuild/backend/ninjabackend.py
+++ b/mesonbuild/backend/ninjabackend.py
@@ -1962,23 +1962,22 @@ class NinjaBackend(backends.Backend):
             except KeyError:
                 pass
 
-        # Since 1.61.0 Rust has a special modifier for whole-archive linking,
-        # before that it would treat linking two static libraries as
-        # whole-archive linking. However, to make this work we have to disable
-        # bundling, which can't be done until 1.63.0… So for 1.61–1.62 we just
-        # have to hope that the default cases of +whole-archive are sufficient.
-        # See: https://github.com/rust-lang/rust/issues/99429
-        if mesonlib.version_compare(rustc.version, '>= 1.63.0'):
-            whole_archive = '+whole-archive,-bundle'
-        else:
-            whole_archive = ''
-
-        # FIXME: Seems broken on MacOS: https://github.com/rust-lang/rust/issues/116674
-        if mesonlib.version_compare(rustc.version, '>= 1.67.0') and not mesonlib.is_osx():
+        if mesonlib.version_compare(rustc.version, '>= 1.67.0'):
             verbatim = '+verbatim'
         else:
             verbatim = ''
 
+        def _link_library(libname: str, static: bool, bundle: bool = False):
+            type_ = 'static' if static else 'dylib'
+            modifiers = []
+            if not bundle and static:
+                modifiers.append('-bundle')
+            if verbatim:
+                modifiers.append(verbatim)
+            if modifiers:
+                type_ += ':' + ','.join(modifiers)
+            args.append(f'-l{type_}={libname}')
+
         linkdirs = mesonlib.OrderedSet()
         external_deps = target.external_deps.copy()
         target_deps = target.get_dependencies()
@@ -2001,72 +2000,54 @@ class NinjaBackend(backends.Backend):
             if isinstance(d, build.StaticLibrary):
                 external_deps.extend(d.external_deps)
 
-            lib = None
-            modifiers = []
+            # Pass native libraries directly to the linker with "-C link-arg"
+            # because rustc's "-l:+verbatim=" is not portable and we cannot rely
+            # on linker to find the right library without using verbatim filename.
+            # For example "-lfoo" won't find "foo.so" in the case name_prefix set
+            # to "", or would always pick the shared library when both "libfoo.so"
+            # and "libfoo.a" are available.
+            # See https://doc.rust-lang.org/rustc/command-line-arguments.html#linking-modifiers-verbatim.
+            #
+            # However, rustc static linker (rlib and staticlib) requires using
+            # "-l" argument and does not rely on platform specific dynamic linker.
+            lib = self.get_target_filename_for_linking(d)
             link_whole = d in target.link_whole_targets
-            if link_whole and whole_archive:
-                modifiers.append(whole_archive)
-            if verbatim:
-                modifiers.append(verbatim)
-                lib = self.get_target_filename_for_linking(d)
-            elif rustc.linker.id in {'link', 'lld-link'} and isinstance(d, build.StaticLibrary):
-                # Rustc doesn't follow Meson's convention that static libraries
-                # are called .a, and import libraries are .lib, so we have to
-                # manually handle that.
-                if link_whole:
-                    if isinstance(target, build.StaticLibrary):
-                        # If we don't, for static libraries the only option is
-                        # to make a copy, since we can't pass objects in, or
-                        # directly affect the archiver. but we're not going to
-                        # do that given how quickly rustc versions go out of
-                        # support unless there's a compelling reason to do so.
-                        # This only affects 1.61–1.66
-                        mlog.warning('Due to limitations in Rustc versions 1.61–1.66 and meson library naming,',
-                                     'whole-archive linking with MSVC may or may not work. Upgrade rustc to',
-                                     '>= 1.67. A best effort is being made, but likely won\'t work')
-                        lib = d.name
-                    else:
-                        # When doing dynamic linking (binaries and [c]dylibs),
-                        # we can instead just proxy the correct arguments to the linker
-                        for link_whole_arg in rustc.linker.get_link_whole_for([self.get_target_filename_for_linking(d)]):
-                            args += ['-C', f'link-arg={link_whole_arg}']
-                else:
-                    args += ['-C', f'link-arg={self.get_target_filename_for_linking(d)}']
+            if isinstance(target, build.StaticLibrary):
+                static = isinstance(d, build.StaticLibrary)
+                libname = os.path.basename(lib) if verbatim else d.name
+                _link_library(libname, static, bundle=link_whole)
+            elif link_whole:
+                link_whole_args = rustc.linker.get_link_whole_for([lib])
+                args += [f'-Clink-arg={a}' for a in link_whole_args]
             else:
-                lib = d.name
-
-            if lib:
-                _type = 'static' if isinstance(d, build.StaticLibrary) else 'dylib'
-                if modifiers:
-                    _type += ':' + ','.join(modifiers)
-                args += ['-l', f'{_type}={lib}']
+                args.append(f'-Clink-arg={lib}')
 
         for e in external_deps:
             for a in e.get_link_args():
                 if a in rustc.native_static_libs:
                     # Exclude link args that rustc already add by default
-                    continue
-                if a.endswith(('.dll', '.so', '.dylib', '.a', '.lib')):
-                    dir_, lib = os.path.split(a)
-                    linkdirs.add(dir_)
-                    lib, ext = os.path.splitext(lib)
-                    if lib.startswith('lib'):
-                        lib = lib[3:]
-                    _type = 'static' if a.endswith(('.a', '.lib')) else 'dylib'
-                    args.extend(['-l', f'{_type}={lib}'])
+                    pass
                 elif a.startswith('-L'):
                     args.append(a)
-                elif a.startswith('-l'):
-                    _type = 'static' if e.static else 'dylib'
-                    args.extend(['-l', f'{_type}={a[2:]}'])
+                elif a.endswith(('.dll', '.so', '.dylib', '.a', '.lib')) and isinstance(target, build.StaticLibrary):
+                    dir_, lib = os.path.split(a)
+                    linkdirs.add(dir_)
+                    if not verbatim:
+                        lib, ext = os.path.splitext(lib)
+                        if lib.startswith('lib'):
+                            lib = lib[3:]
+                    static = a.endswith(('.a', '.lib'))
+                    _link_library(lib, static)
+                else:
+                    args.append(f'-Clink-arg={a}')
+
         for d in linkdirs:
-            if d == '':
-                d = '.'
-            args += ['-L', d]
+            d = d or '.'
+            args.append(f'-L{d}')
 
         # Because of the way rustc links, this must come after any potential
         # library need to link with their stdlibs (C++ and Fortran, for example)
-        args.extend(target.get_used_stdlib_args('rust'))
+        args.extend(f'-Clink-arg={a}' for a in target.get_used_stdlib_args('rust'))
 
         has_shared_deps = any(isinstance(dep, build.SharedLibrary) for dep in target_deps)
         has_rust_shared_deps = any(dep.uses_rust()
diff --git a/mesonbuild/build.py b/mesonbuild/build.py
index 9283bca97..9c27b2361 100644
--- a/mesonbuild/build.py
+++ b/mesonbuild/build.py
@@ -1276,13 +1276,13 @@ class BuildTarget(Target):
         return self.extra_args[language]
 
     @lru_cache(maxsize=None)
-    def get_dependencies(self) -> OrderedSet[Target]:
+    def get_dependencies(self) -> OrderedSet[BuildTargetTypes]:
         # Get all targets needed for linking. This includes all link_with and
         # link_whole targets, and also all dependencies of static libraries
         # recursively. The algorithm here is closely related to what we do in
         # get_internal_static_libraries(): Installed static libraries include
         # objects from all their dependencies already.
-        result: OrderedSet[Target] = OrderedSet()
+        result: OrderedSet[BuildTargetTypes] = OrderedSet()
         for t in itertools.chain(self.link_targets, self.link_whole_targets):
             if t not in result:
                 result.add(t)
@@ -1290,7 +1290,7 @@ class BuildTarget(Target):
                     t.get_dependencies_recurse(result)
         return result
 
-    def get_dependencies_recurse(self, result: OrderedSet[Target], include_internals: bool = True) -> None:
+    def get_dependencies_recurse(self, result: OrderedSet[BuildTargetTypes], include_internals: bool = True) -> None:
         # self is always a static library because we don't need to pull dependencies
         # of shared libraries. If self is installed (not internal) it already
         # include objects extracted from all its internal dependencies so we can
@@ -1299,7 +1299,7 @@ class BuildTarget(Target):
         for t in self.link_targets:
             if t in result:
                 continue
-            if isinstance(t, SharedLibrary) and t.rust_crate_type == 'proc-macro':
+            if t.rust_crate_type == 'proc-macro':
                 continue
             if include_internals or not t.is_internal():
                 result.add(t)
@@ -1394,7 +1394,7 @@ class BuildTarget(Target):
     def is_internal(self) -> bool:
         return False
 
-    def link(self, targets):
+    def link(self, targets: T.List[BuildTargetTypes]) -> None:
         for t in targets:
             if not isinstance(t, (Target, CustomTargetIndex)):
                 if isinstance(t, dependencies.ExternalLibrary):
@@ -1420,7 +1420,7 @@ class BuildTarget(Target):
             self.check_can_link_together(t)
             self.link_targets.append(t)
 
-    def link_whole(self, targets, promoted: bool = False):
+    def link_whole(self, targets: T.List[BuildTargetTypes], promoted: bool = False) -> None:
         for t in targets:
             if isinstance(t, (CustomTarget, CustomTargetIndex)):
                 if not t.is_linkable_target():
@@ -1434,36 +1434,37 @@ class BuildTarget(Target):
                 msg += "Use the 'pic' option to static_library to build with PIC."
                 raise InvalidArguments(msg)
             self.check_can_link_together(t)
-            if isinstance(self, StaticLibrary) and not self.uses_rust():
+            if isinstance(self, StaticLibrary):
                 # When we're a static library and we link_whole: to another static
                 # library, we need to add that target's objects to ourselves.
-                self.check_can_extract_objects(t, origin=self, promoted=promoted)
-                self.objects += [t.extract_all_objects()]
+                self._bundle_static_library(t, promoted)
                 # If we install this static library we also need to include objects
                 # from all uninstalled static libraries it depends on.
                 if self.install:
-                    for lib in t.get_internal_static_libraries(origin=self):
-                        self.objects += [lib.extract_all_objects()]
+                    for lib in t.get_internal_static_libraries():
+                        self._bundle_static_library(lib, True)
             self.link_whole_targets.append(t)
 
     @lru_cache(maxsize=None)
-    def get_internal_static_libraries(self, origin: StaticLibrary) -> OrderedSet[Target]:
-        result: OrderedSet[Target] = OrderedSet()
-        self.get_internal_static_libraries_recurse(result, origin)
+    def get_internal_static_libraries(self) -> OrderedSet[BuildTargetTypes]:
+        result: OrderedSet[BuildTargetTypes] = OrderedSet()
+        self.get_internal_static_libraries_recurse(result)
         return result
 
-    def get_internal_static_libraries_recurse(self, result: OrderedSet[Target], origin: StaticLibrary) -> None:
+    def get_internal_static_libraries_recurse(self, result: OrderedSet[BuildTargetTypes]) -> None:
         for t in self.link_targets:
             if t.is_internal() and t not in result:
-                self.check_can_extract_objects(t, origin, promoted=True)
                 result.add(t)
-                t.get_internal_static_libraries_recurse(result, origin)
+                t.get_internal_static_libraries_recurse(result)
         for t in self.link_whole_targets:
             if t.is_internal():
-                t.get_internal_static_libraries_recurse(result, origin)
+                t.get_internal_static_libraries_recurse(result)
 
-    def check_can_extract_objects(self, t: T.Union[Target, CustomTargetIndex], origin: StaticLibrary, promoted: bool = False) -> None:
-        if isinstance(t, (CustomTarget, CustomTargetIndex)) or t.uses_rust():
+    def _bundle_static_library(self, t: T.Union[BuildTargetTypes], promoted: bool = False) -> None:
+        if self.uses_rust():
+            # Rustc can bundle static libraries, no need to extract objects.
+            self.link_whole_targets.append(t)
+        elif isinstance(t, (CustomTarget, CustomTargetIndex)) or t.uses_rust():
             # To extract objects from a custom target we would have to extract
             # the archive, WIP implementation can be found in
             # https://github.com/mesonbuild/meson/pull/9218.
@@ -1472,12 +1473,14 @@ class BuildTarget(Target):
             # https://github.com/mesonbuild/meson/issues/10722
             # https://github.com/mesonbuild/meson/issues/10723
             # https://github.com/mesonbuild/meson/issues/10724
-            m = (f'Cannot link_whole a custom or Rust target {t.name!r} into a static library {origin.name!r}. '
+            m = (f'Cannot link_whole a custom or Rust target {t.name!r} into a static library {self.name!r}. '
                  'Instead, pass individual object files with the "objects:" keyword argument if possible.')
             if promoted:
-                m += (f' Meson had to promote link to link_whole because {origin.name!r} is installed but not {t.name!r},'
+                m += (f' Meson had to promote link to link_whole because {self.name!r} is installed but not {t.name!r},'
                       f' and thus has to include objects from {t.name!r} to be usable.')
             raise InvalidArguments(m)
+        else:
+            self.objects.append(t.extract_all_objects())
 
     def check_can_link_together(self, t: BuildTargetTypes) -> None:
         links_with_rust_abi = isinstance(t, BuildTarget) and t.uses_rust_abi()
@@ -2524,7 +2527,26 @@ class CommandBase:
                 raise InvalidArguments(f'Argument {c!r} in "command" is invalid')
         return final_cmd
 
-class CustomTarget(Target, CommandBase):
+class CustomTargetBase:
+    ''' Base class for CustomTarget and CustomTargetIndex
+
+    This base class can be used to provide a dummy implementation of some
+    private methods to avoid repeating `isinstance(t, BuildTarget)` when dealing
+    with custom targets.
+    '''
+
+    rust_crate_type = ''
+
+    def get_dependencies_recurse(self, result: OrderedSet[BuildTargetTypes], include_internals: bool = True) -> None:
+        pass
+
+    def get_internal_static_libraries(self) -> OrderedSet[BuildTargetTypes]:
+        return OrderedSet()
+
+    def get_internal_static_libraries_recurse(self, result: OrderedSet[BuildTargetTypes]) -> None:
+        pass
+
+class CustomTarget(Target, CustomTargetBase, CommandBase):
 
     typename = 'custom'
 
@@ -2901,7 +2923,7 @@ class Jar(BuildTarget):
         return self.environment.get_jar_dir(), '{jardir}'
 
 @dataclass(eq=False)
-class CustomTargetIndex(HoldableObject):
+class CustomTargetIndex(CustomTargetBase, HoldableObject):
 
     """A special opaque object returned by indexing a CustomTarget. This object
     exists in Meson, but acts as a proxy in the backends, making targets depend
diff --git a/test cases/rust/15 polyglot sharedlib/adder.c b/test cases/rust/15 polyglot sharedlib/adder.c
index 66613edf5..1b5faa67e 100644
--- a/test cases/rust/15 polyglot sharedlib/adder.c
+++ b/test cases/rust/15 polyglot sharedlib/adder.c
@@ -11,7 +11,13 @@ adder* adder_create(int number) {
     return a;
 }
 
-// adder_add is implemented in the Rust file.
+// adder_add_r is implemented in the Rust file.
+int adder_add_r(adder *a, int number);
+
+int adder_add(adder *a, int number)
+{
+    return adder_add_r(a, number);
+}
 
 void adder_destroy(adder *a) {
     free(a);
diff --git a/test cases/rust/15 polyglot sharedlib/adder.rs b/test cases/rust/15 polyglot sharedlib/adder.rs
index 909535035..ec4d1cc13 100644
--- a/test cases/rust/15 polyglot sharedlib/adder.rs
+++ b/test cases/rust/15 polyglot sharedlib/adder.rs
@@ -3,7 +3,14 @@ pub struct Adder {
   pub number: i32
 }
 
+extern "C" {
+    pub fn zero() -> i32;
+    pub fn zero_static() -> i32;
+}
+
 #[no_mangle]
-pub extern fn adder_add(a: &Adder, number: i32) -> i32 {
-    return a.number + number;
+pub extern fn adder_add_r(a: &Adder, number: i32) -> i32 {
+  unsafe {
+    return a.number + number + zero() + zero_static();
+  }
 }
diff --git a/test cases/rust/15 polyglot sharedlib/meson.build b/test cases/rust/15 polyglot sharedlib/meson.build
index 13fc8fd47..fc3d53b67 100644
--- a/test cases/rust/15 polyglot sharedlib/meson.build
+++ b/test cases/rust/15 polyglot sharedlib/meson.build
@@ -1,20 +1,15 @@
 project('adder', 'c', 'rust', version: '1.0.0')
 
-if build_machine.system() != 'linux'
-    error('MESON_SKIP_TEST, this test only works on Linux. Patches welcome.')
-endif
+subdir('zero')
 
-thread_dep = dependency('threads')
-dl_dep = meson.get_compiler('c').find_library('dl', required: false)
-m_dep = meson.get_compiler('c').find_library('m', required: false)
-
-rl = static_library('radder', 'adder.rs', rust_crate_type: 'staticlib')
+rl = shared_library('radder', 'adder.rs',
+  rust_abi: 'c',
+  link_with: [zero_shared, zero_static])
 
 l = shared_library('adder', 'adder.c',
-    c_args: '-DBUILDING_ADDER',
-    link_with: rl,
-    version: '1.0.0',
-    soversion: '1',
-    link_args: '-Wl,-u,adder_add', # Ensure that Rust code is not removed as unused.
-    dependencies: [thread_dep, dl_dep, m_dep])
+  c_args: '-DBUILDING_ADDER',
+  link_with: rl,
+  version: '1.0.0',
+  soversion: '1',
+)
 test('adder', executable('addertest', 'addertest.c', link_with: l))
diff --git a/test cases/rust/15 polyglot sharedlib/zero/meson.build b/test cases/rust/15 polyglot sharedlib/zero/meson.build
new file mode 100644
index 000000000..ec7ecf7bb
--- /dev/null
+++ b/test cases/rust/15 polyglot sharedlib/zero/meson.build
@@ -0,0 +1,6 @@
+# They both have the same name, this tests we use +verbatim to distinguish them
+# using their filename. It also ensures we pass the importlib on Windows.
+# Those libs are in a subdir as regression test:
+# https://github.com/mesonbuild/meson/issues/12484
+zero_shared = shared_library('zero', 'zero.c')
+zero_static = static_library('zero', 'zero_static.c')
diff --git a/test cases/rust/15 polyglot sharedlib/zero/zero.c b/test cases/rust/15 polyglot sharedlib/zero/zero.c
new file mode 100644
index 000000000..02672f394
--- /dev/null
+++ b/test cases/rust/15 polyglot sharedlib/zero/zero.c
@@ -0,0 +1,11 @@
+#if defined _WIN32 || defined __CYGWIN__
+#define EXPORT __declspec(dllexport)
+#else
+#define EXPORT
+#endif
+
+EXPORT int zero(void);
+
+int zero(void) {
+    return 0;
+}
diff --git a/test cases/rust/15 polyglot sharedlib/zero/zero_static.c b/test cases/rust/15 polyglot sharedlib/zero/zero_static.c
new file mode 100644
index 000000000..7f14fb4d6
--- /dev/null
+++ b/test cases/rust/15 polyglot sharedlib/zero/zero_static.c
@@ -0,0 +1,6 @@
+int zero_static(void);
+
+int zero_static(void)
+{
+    return 0;
+}
diff --git a/test cases/rust/20 transitive dependencies/diamond/func.c b/test cases/rust/20 transitive dependencies/diamond/func.c
new file mode 100644
index 000000000..c07ab728d
--- /dev/null
+++ b/test cases/rust/20 transitive dependencies/diamond/func.c
@@ -0,0 +1,4 @@
+int c_func(void);
+int c_func(void) {
+    return 123;
+}
diff --git a/test cases/rust/20 transitive dependencies/diamond/main.c b/test cases/rust/20 transitive dependencies/diamond/main.c
new file mode 100644
index 000000000..c633e9ae5
--- /dev/null
+++ b/test cases/rust/20 transitive dependencies/diamond/main.c
@@ -0,0 +1,5 @@
+int r3(void);
+
+int main_func(void) {
+    return r3() == 246 ? 0 : 1;
+}
diff --git a/test cases/rust/20 transitive dependencies/diamond/meson.build b/test cases/rust/20 transitive dependencies/diamond/meson.build
new file mode 100644
index 000000000..dc48d4523
--- /dev/null
+++ b/test cases/rust/20 transitive dependencies/diamond/meson.build
@@ -0,0 +1,25 @@
+# Regression test for a diamond dependency graph:
+#          ┌►R1┐
+# main-►R3─┤   ├─►C1
+#          └►R2┘
+# Both libr1.rlib and libr2.rlib used to contain func.c.o. That was causing
+# libr3.rlib to have duplicated func.c.o and then libmain.so failed to link:
+# multiple definition of `c_func'.
+
+libc1 = static_library('c1', 'func.c')
+libr1 = static_library('r1', 'r1.rs', link_with: libc1)
+libr2 = static_library('r2', 'r2.rs', link_with: libc1)
+libr3 = static_library('r3', 'r3.rs',
+  link_with: [libr1, libr2],
+  rust_abi: 'c',
+)
+shared_library('main', 'main.c', link_whole: [libr3])
+
+# Same dependency graph, but r3 is now installed. Since c1, r1 and r2 are
+# not installed, r3 must contain them.
+libr3 = static_library('r3-installed', 'r3.rs',
+  link_with: [libr1, libr2],
+  rust_abi: 'c',
+  install: true,
+)
+shared_library('main-installed', 'main.c', link_with: [libr3])
diff --git a/test cases/rust/20 transitive dependencies/diamond/r1.rs b/test cases/rust/20 transitive dependencies/diamond/r1.rs
new file mode 100644
index 000000000..7afb71109
--- /dev/null
+++ b/test cases/rust/20 transitive dependencies/diamond/r1.rs
@@ -0,0 +1,9 @@
+extern "C" {
+    fn c_func() -> i32;
+}
+
+pub fn r1() -> i32 {
+    unsafe {
+        c_func()
+    }
+}
diff --git a/test cases/rust/20 transitive dependencies/diamond/r2.rs b/test cases/rust/20 transitive dependencies/diamond/r2.rs
new file mode 100644
index 000000000..ee73ee2e2
--- /dev/null
+++ b/test cases/rust/20 transitive dependencies/diamond/r2.rs
@@ -0,0 +1,9 @@
+extern "C" {
+    fn c_func() -> i32;
+}
+
+pub fn r2() -> i32 {
+    unsafe {
+        c_func()
+    }
+}
diff --git a/test cases/rust/20 transitive dependencies/diamond/r3.rs b/test cases/rust/20 transitive dependencies/diamond/r3.rs
new file mode 100644
index 000000000..9794b7eda
--- /dev/null
+++ b/test cases/rust/20 transitive dependencies/diamond/r3.rs
@@ -0,0 +1,4 @@
+#[no_mangle]
+pub fn r3() -> i32 {
+    r1::r1() + r2::r2()
+}
diff --git a/test cases/rust/20 transitive dependencies/meson.build b/test cases/rust/20 transitive dependencies/meson.build
index e5354b8f6..b786e6494 100644
--- a/test cases/rust/20 transitive dependencies/meson.build
+++ b/test cases/rust/20 transitive dependencies/meson.build
@@ -25,3 +25,5 @@ exe = executable('footest', 'foo.c',
   link_with: foo,
 )
 test('footest', exe)
+
+subdir('diamond')
diff --git a/test cases/rust/20 transitive dependencies/test.json b/test cases/rust/20 transitive dependencies/test.json
new file mode 100644
index 000000000..0d98c230a
--- /dev/null
+++ b/test cases/rust/20 transitive dependencies/test.json
@@ -0,0 +1,5 @@
+{
+  "installed": [
+    {"type": "file", "file": "usr/lib/libr3-installed.a"}
+  ]
+}