rust: Don't Link non-cdylib/staticlib Rust libraries into non-Rust ta…

[sdroege]

Fixes #11721

---

CC @dcbaker @xclaesse

This is probably wrong but not sure at which other place this should be. This seems to work though.

Another option would be

diff --git a/mesonbuild/backend/backends.py b/mesonbuild/backend/backends.py
index 4972eabad..3c2137b78 100644
--- a/mesonbuild/backend/backends.py
+++ b/mesonbuild/backend/backends.py
@@ -1034,6 +1034,8 @@ class Backend:
         for d in deps:
             if not d.is_linkable_target():
                 raise RuntimeError(f'Tried to link with a non-library target "{d.get_basename()}".')
+            if d.uses_rust() and d.rust_crate_type not in {'staticlib', 'cdylib'}:
+                continue
             arg = self.get_target_filename_for_linking(d)
             if not arg:
                 continue

[xclaesse]

Another alternative is to promote link to link_whole there: https://github.com/mesonbuild/meson/blob/master/mesonbuild/build.py#L1391. I think that reflects better what actually happens, and we already do similar stuff there. Does that work?

@sdroege see the comment in the link above, seems @dcbaker already had to deal with similar case.

[sdroege]

You mean adding a or (t.uses_rust() and t.rust_crate_type == 'staticlib') at the end of that existing condition?

[xclaesse]

Not sure I understood exactly the use-case actually. Do I understand correctly that when rustc links rlib A to another rlib B, it does it whole-archive, meaning that when an executable links to A we don't need B and its dependencies anymore?

We definitely need a unit test for this, and I'm sure that will also clarify what exactly you're fixing.

[sdroege]

Not sure I understood exactly the use-case actually. Do I understand correctly that when rustc links rlib A to another rlib B, it does it whole-archive, meaning that when an executable links to A we don't need B and its dependencies anymore?

The problem is that if you build a staticlib then all its rlib dependencies are like "whole-archive", i.e. they're included in the staticlib. So if a C/C++ compiler/linker wants to make use of such a staticlib it only needs that but none of its rlib dependencies passed in. Currently all the rlib dependencies are passed in, e.g. in the example from the linked issue

[13/13] c++  -o cpp/hltas-cpp-tests cpp/hltas-cpp-tests.p/src_tests.cpp.o -Wl,--as-needed -Wl,--no-undefined -Wl,--start-group cpp/libhltas_cpp.a hltas-cpp-bridge/libhltas_cpp_bridge.a -Wl,--end-group libhltas.rlib subprojects/nom-7.1.3/libnom.rlib subprojects/memchr-2.5.0/libmemchr.rlib subprojects/minimal-lexical-0.2.1/libminimal_lexical.rlib subprojects/cookie-factory-0.3.2/libcookie_factory.rlib subprojects/cfg-if-1.0.0/libcfg_if.rlib

while it should be just

[13/13] c++  -o cpp/hltas-cpp-tests cpp/hltas-cpp-tests.p/src_tests.cpp.o -Wl,--as-needed -Wl,--no-undefined -Wl,--start-group cpp/libhltas_cpp.a hltas-cpp-bridge/libhltas_cpp_bridge.a -Wl,--end-group

hltas_cpp_bridge is the staticlib here. All the .rlibs are already part of it (and arguably passing them to a C++ compiler/linker should give at least a warning, see #11722).

The change in this PR and also the alternative are both changing it from the first with all the .rlibs to the second without.

We definitely need a unit test for this, and I'm sure that will also clarify what exactly you're fixing.

I don't know how to make a unit test for this because it works fine and just passes too much to the linker. Do you have a suggestion?

[xclaesse]

I don't know how to make a unit test for this because it works fine and just passes too much to the linker. Do you have a suggestion?

I don't know any nice way of doing this, I initially thought that linker would error out if getting .rlib files. I see we have a few unit tests that parse build.ninja with some regex, that would work. See linuxliketests.py.

[sdroege]

Not beautiful but I guess that will do. Do you have a preferred approach/place for solving the bug though?

[xclaesse]

@sdroege I think test cases/rust/17 staticlib link staticlib already reproduce this issue actually:

[4/4] cc  -o prog prog.p/prog.c.o -Wl,--as-needed -Wl,--no-undefined -Wl,--start-group libbranch.a -Wl,--end-group libleaf.rlib

libleaf.rlib is not needed there.

[xclaesse]

As always, when investigating more closely the root cause I discover that the problem is bigger than we thought.

a = static_library('foo', 'foo.c')
b = static_library('bar', 'bar.c', link_whole: a)
e = executable('prog', 'main.c', link_with: b)

This is a similar pure C example, libfoo.a is not needed to link prog, but it is given to command line too.

[sdroege]

That seems like a separate but similar issue.

As discussed on IRC, @xclaesse thinks that the best approach here would be to explicitly use link_whole for staticlib / cdylib as that's how it conceptually works, and to keep using link_with in the other cases. My main concern with that is

• It requires different handling of dependencies by the user depending on the crate type, which seems error prone. meson should probably detect various "wrong" combinations and error out (or at least warn) if they happen, like everything that would involve linking an rlib/dylib/proc-macro with a non-Rust compiler/linker
• proc-macro needs special handling too as it is basically a "build dependency". It only needs to be passed to build targets that themselves directly use the proc-macro (as --extern), and indirectly (as -L only) if coming from an rlib dependency