Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Building an installed static library that links with an internal static library in rust doesn’t work #11247

Closed
bpeel opened this issue Jan 5, 2023 · 6 comments · Fixed by #11250
Assignees

Comments

@bpeel
Copy link

bpeel commented Jan 5, 2023

I am trying to make an installable static library that is a mix of rust and C code. As far as I understand, the rust code needs to be in its own static_library because you can’t combine C and rust sources directly. If I do this then the resulting installed library doesn’t include the rust parts and then when I try to use it in an external application it gets link errors.

To reproduce

Here’s a example project:

meson.build

project('coollib', ['c', 'rust'],
        version : '0.1',
        default_options : 'default_library=static')

internal_lib = static_library('internal',
                              rust_crate_type : 'staticlib',
                              sources : 'internal.rs')

cool_lib = library('cool',
                   sources : 'cool.c',
                   install : true,
                   link_with : internal_lib)

pkg = import('pkgconfig')
pkg.generate(cool_lib)

internal.rs

#[no_mangle]
extern "C" fn multiply_by_two(number: i32) -> i32
{
        number * 2
}

cool.c

int
multiply_by_two(int);

int
get_cool_number(void)
{
        return multiply_by_two(21);
}

Building

Build the project like this:

meson setup -Dprefix=$PWD/install build
ninja -C build install

app.c

We can test with a sample app that uses the library like this:

#include <stdio.h>

int
get_cool_number(void);

int
main(int argc, char *argv)
{
        printf("cool number is %i\n", get_cool_number());

        return 0;
}

And then build it like this:

cc -o app app.c $(PKG_CONFIG_PATH=$PWD/install/lib64/pkgconfig pkg-config cool --libs --cflags)

This fails:

/usr/bin/ld: /home/neil/c/test-library/install/lib64/libcool.a(cool.c.o): in function `get_cool_number':
/home/neil/c/test-library/build/../cool.c:7: undefined reference to `multiply_by_two'
collect2: error: ld returned 1 exit status

You can see that internal.a was not included in the installed archive:

$ nm install/lib64/libcool.a

cool.c.o:
0000000000000000 T get_cool_number
                 U multiply_by_two

If I instead build a dynamic library by passing -Ddefault_library=shared to meson setup then it works fine.

Actual use case

My actual use case is trying to make a port of vkrunner to rust. If you run the test-build.sh script in that repo one of things it tries to do is link an application to the installed library.

System parameters

  • Native build
  • Fedora 37
  • Python 3.11.1
  • meson git head 25e73b6
  • ninja 1.10.2
@dcbaker
Copy link
Member

dcbaker commented Jan 5, 2023

I can reproduce this, and it seems like a legitimate bug of some sort, as a workaround I was able to test inversing the building, and that worked:

internal_lib = static_library('internal', 'cool.c')
cool_lib = static_library('cool', 'internal.rs', link_with : internal_lib, install : true, rust_crate_type : 'staticlib')
executable('app', 'app.c', link_with : cool_lib)

@dcbaker
Copy link
Member

dcbaker commented Jan 5, 2023

The problem is that for whatever reason when linked in the order posted in the report, there is no dependency created between the rust library and the internal library, so they don't actually get linked even though they should. Further, there's another bug which causes using link_whole : internal_lib (the first workaround I tried) to also error.

@dcbaker
Copy link
Member

dcbaker commented Jan 5, 2023

oh, this might not be entirely our fault. What version of rustc are you using?

@bpeel
Copy link
Author

bpeel commented Jan 5, 2023

I’m using the rustc that comes with Fedora, 1.65.0. I’ll try with the latest rust too.

I’m not sure if I understand the workaround. In the example I’m linking to the installed library without using meson. In fact the actual use case in vkrunner I’m also building the vkrunner executable with meson and that links fine without changing anything. I don’t know anything about how meson works but I was thinking it might be something like if you use link_with to link to a library that meson knows about then it knows that the internal static library is a dependency so it links it all together when you make the final executable. But when the static library is installed it’s going to be linked into an executable outside of meson so it needs to link the internal library into the static library, ie, before linking the executable.

@dcbaker
Copy link
Member

dcbaker commented Jan 5, 2023

So this is a bit our issue, a bit rustc's issue.

in Rustc < 1.61 linking two static libraries together was equivalent to gcc's --whole-archive, in rustc >= 1.61, you need to tell rustc that's what you it to do. I knew that was coming, but kinda forgot about it. I have a patch that fixes this, and it's pretty small so I'll propose it as a backport. I need to write some tests for it before I send it out though.

dcbaker added a commit to dcbaker/meson that referenced this issue Jan 5, 2023
Rustc as of version 1.61.0 has support for controlling when
whole-archive linking takes place, previous to this it tried to make a
good guess about what you wanted, which worked most of the time. This is
now implemented. Additionally, because meson breaks some rustc
assumption about library naming on windows, we have to manually pass
whole-archive libraries, bypassing rustc's interface and talking
directly to the linker for MSVC (and those imitating it), which we were
doing incorrectly. This has been fixed.

Fixes: mesonbuild#10723
Fixes: mesonbuild#11247
@dcbaker
Copy link
Member

dcbaker commented Jan 5, 2023

@bpeel I've opened a PR with a patch to fix this, at least it fixes your example code (I've modified one of our tests to cover it), and I've proposed it for backport

dcbaker added a commit to dcbaker/meson that referenced this issue Jan 5, 2023
Rustc as of version 1.61.0 has support for controlling when
whole-archive linking takes place, previous to this it tried to make a
good guess about what you wanted, which worked most of the time. This is
now implemented. Additionally, because meson breaks some rustc
assumption about library naming on windows, we have to manually pass
whole-archive libraries, bypassing rustc's interface and talking
directly to the linker for MSVC (and those imitating it), which we were
doing incorrectly. This has been fixed.

Fixes: mesonbuild#10723
Fixes: mesonbuild#11247
dcbaker added a commit to dcbaker/meson that referenced this issue Jan 5, 2023
Rustc as of version 1.61.0 has support for controlling when
whole-archive linking takes place, previous to this it tried to make a
good guess about what you wanted, which worked most of the time. This is
now implemented. Additionally, because meson breaks some rustc
assumption about library naming on windows, we have to manually pass
whole-archive libraries, bypassing rustc's interface and talking
directly to the linker for MSVC (and those imitating it), which we were
doing incorrectly. This has been fixed.

Fixes: mesonbuild#10723
Fixes: mesonbuild#11247
dcbaker added a commit to dcbaker/meson that referenced this issue Feb 7, 2023
Rustc as of version 1.61.0 has support for controlling when
whole-archive linking takes place, previous to this it tried to make a
good guess about what you wanted, which worked most of the time. This is
now implemented. Additionally, because meson breaks some rustc
assumption about library naming on windows, we have to manually pass
whole-archive libraries, bypassing rustc's interface and talking
directly to the linker for MSVC (and those imitating it), which we were
doing incorrectly. This has been fixed.

Fixes: mesonbuild#10723
Fixes: mesonbuild#11247
dcbaker added a commit to dcbaker/meson that referenced this issue Feb 7, 2023
Rustc as of version 1.61.0 has support for controlling when
whole-archive linking takes place, previous to this it tried to make a
good guess about what you wanted, which worked most of the time. This is
now implemented. Additionally, because meson breaks some rustc
assumption about library naming on windows, we have to manually pass
whole-archive libraries, bypassing rustc's interface and talking
directly to the linker for MSVC (and those imitating it), which we were
doing incorrectly. This has been fixed.

Fixes: mesonbuild#10723
Fixes: mesonbuild#11247
dcbaker added a commit to dcbaker/meson that referenced this issue Feb 7, 2023
Rustc as of version 1.61.0 has support for controlling when
whole-archive linking takes place, previous to this it tried to make a
good guess about what you wanted, which worked most of the time. This is
now implemented. Additionally, because meson breaks some rustc
assumption about library naming on windows, we have to manually pass
whole-archive libraries, bypassing rustc's interface and talking
directly to the linker for MSVC (and those imitating it), which we were
doing incorrectly. This has been fixed.

Fixes: mesonbuild#10723
Fixes: mesonbuild#11247
dcbaker added a commit to dcbaker/meson that referenced this issue Feb 8, 2023
Rustc as of version 1.61.0 has support for controlling when
whole-archive linking takes place, previous to this it tried to make a
good guess about what you wanted, which worked most of the time. This is
now implemented. Additionally, because meson breaks some rustc
assumption about library naming on windows, we have to manually pass
whole-archive libraries, bypassing rustc's interface and talking
directly to the linker for MSVC (and those imitating it), which we were
doing incorrectly. This has been fixed.

Fixes: mesonbuild#10723
Fixes: mesonbuild#11247
dcbaker added a commit to dcbaker/meson that referenced this issue Feb 10, 2023
Rustc as of version 1.61.0 has support for controlling when
whole-archive linking takes place, previous to this it tried to make a
good guess about what you wanted, which worked most of the time. This is
now implemented. Additionally, because meson breaks some rustc
assumption about library naming on windows, we have to manually pass
whole-archive libraries, bypassing rustc's interface and talking
directly to the linker for MSVC (and those imitating it), which we were
doing incorrectly. This has been fixed.

Fixes: mesonbuild#10723
Fixes: mesonbuild#11247

Co-authored-by: Nirbheek Chauhan <nirbheek@centricular.com>
dcbaker added a commit to dcbaker/meson that referenced this issue Feb 13, 2023
Rustc as of version 1.61.0 has support for controlling when
whole-archive linking takes place, previous to this it tried to make a
good guess about what you wanted, which worked most of the time. This is
now implemented. Additionally, because meson breaks some rustc
assumption about library naming on windows, we have to manually pass
whole-archive libraries, bypassing rustc's interface and talking
directly to the linker for MSVC (and those imitating it), which we were
doing incorrectly. This has been fixed.

Fixes: mesonbuild#10723
Fixes: mesonbuild#11247

Co-authored-by: Nirbheek Chauhan <nirbheek@centricular.com>
dcbaker added a commit to dcbaker/meson that referenced this issue Feb 13, 2023
Rustc as of version 1.61.0 has support for controlling when
whole-archive linking takes place, previous to this it tried to make a
good guess about what you wanted, which worked most of the time. This is
now implemented. Additionally, because meson breaks some rustc
assumption about library naming on windows, we have to manually pass
whole-archive libraries, bypassing rustc's interface and talking
directly to the linker for MSVC (and those imitating it), which we were
doing incorrectly. This has been fixed.

Fixes: mesonbuild#10723
Fixes: mesonbuild#11247

Co-authored-by: Nirbheek Chauhan <nirbheek@centricular.com>
dcbaker added a commit to dcbaker/meson that referenced this issue Feb 13, 2023
Rustc as of version 1.61.0 has support for controlling when
whole-archive linking takes place, previous to this it tried to make a
good guess about what you wanted, which worked most of the time. This is
now implemented. Additionally, because meson breaks some rustc
assumption about library naming on windows, we have to manually pass
whole-archive libraries, bypassing rustc's interface and talking
directly to the linker for MSVC (and those imitating it), which we were
doing incorrectly. This has been fixed.

Fixes: mesonbuild#10723
Fixes: mesonbuild#11247

Co-authored-by: Nirbheek Chauhan <nirbheek@centricular.com>
dcbaker added a commit to dcbaker/meson that referenced this issue Feb 15, 2023
Rustc as of version 1.61.0 has support for controlling when
whole-archive linking takes place, previous to this it tried to make a
good guess about what you wanted, which worked most of the time. This is
now implemented. Additionally, because meson breaks some rustc
assumption about library naming on windows, we have to manually pass
whole-archive libraries, bypassing rustc's interface and talking
directly to the linker for MSVC (and those imitating it), which we were
doing incorrectly. This has been fixed.

Fixes: mesonbuild#10723
Fixes: mesonbuild#11247

Co-authored-by: Nirbheek Chauhan <nirbheek@centricular.com>
dcbaker added a commit to dcbaker/meson that referenced this issue Feb 22, 2023
Rustc as of version 1.61.0 has support for controlling when
whole-archive linking takes place, previous to this it tried to make a
good guess about what you wanted, which worked most of the time. This is
now implemented. Additionally, because meson breaks some rustc
assumption about library naming on windows, we have to manually pass
whole-archive libraries, bypassing rustc's interface and talking
directly to the linker for MSVC (and those imitating it), which we were
doing incorrectly. This has been fixed.

Fixes: mesonbuild#10723
Fixes: mesonbuild#11247

Co-authored-by: Nirbheek Chauhan <nirbheek@centricular.com>
dcbaker added a commit to dcbaker/meson that referenced this issue Feb 22, 2023
Rustc as of version 1.61.0 has support for controlling when
whole-archive linking takes place, previous to this it tried to make a
good guess about what you wanted, which worked most of the time. This is
now implemented.

Additionally, rustc makes some assumptions about library names
(specifically static names), that meson does not keep. This can be fixed
with rustc 1.67, where a new +verbatim modifier has been added. We can
then force rustc to use the name we give it. Before that, we can sneak
through `/WHOELARCHIVE:` in cases of dynamic linking (into a dll or
exe), but we can't force the archiver to do what we want (rustc
considers the archiver to be an implementation detail). The only
solution I can come up with is to copy the library to the format that
rustc expects. I've run into some issues with that as well, so we warn
in that case.

The decisions to leave static into static broken on MSVC for 1.61–1.66
was made because:

 1) The work around is non-trivial, and we would have to support that
    workaround for a long time
 2) The number of users of Rust in Meson is small
 3) The number of users of Rust in Meson on Windows, with MSVC is tiny
 4) Using rustup to update rustc on windows is trivial, and solves the
    problem completely

Fixes: mesonbuild#10723
Fixes: mesonbuild#11247

Co-authored-by: Nirbheek Chauhan <nirbheek@centricular.com>
dcbaker added a commit to dcbaker/meson that referenced this issue Feb 22, 2023
Rustc as of version 1.61.0 has support for controlling when
whole-archive linking takes place, previous to this it tried to make a
good guess about what you wanted, which worked most of the time. This is
now implemented.

Additionally, rustc makes some assumptions about library names
(specifically static names), that meson does not keep. This can be fixed
with rustc 1.67, where a new +verbatim modifier has been added. We can
then force rustc to use the name we give it. Before that, we can sneak
through `/WHOELARCHIVE:` in cases of dynamic linking (into a dll or
exe), but we can't force the archiver to do what we want (rustc
considers the archiver to be an implementation detail). The only
solution I can come up with is to copy the library to the format that
rustc expects. I've run into some issues with that as well, so we warn
in that case.

The decisions to leave static into static broken on MSVC for 1.61–1.66
was made because:

 1) The work around is non-trivial, and we would have to support that
    workaround for a long time
 2) The number of users of Rust in Meson is small
 3) The number of users of Rust in Meson on Windows, with MSVC is tiny
 4) Using rustup to update rustc on windows is trivial, and solves the
    problem completely

Fixes: mesonbuild#10723
Fixes: mesonbuild#11247

Co-authored-by: Nirbheek Chauhan <nirbheek@centricular.com>
nirbheek added a commit that referenced this issue Feb 23, 2023
Rustc as of version 1.61.0 has support for controlling when
whole-archive linking takes place, previous to this it tried to make a
good guess about what you wanted, which worked most of the time. This is
now implemented.

Additionally, rustc makes some assumptions about library names
(specifically static names), that meson does not keep. This can be fixed
with rustc 1.67, where a new +verbatim modifier has been added. We can
then force rustc to use the name we give it. Before that, we can sneak
through `/WHOELARCHIVE:` in cases of dynamic linking (into a dll or
exe), but we can't force the archiver to do what we want (rustc
considers the archiver to be an implementation detail). The only
solution I can come up with is to copy the library to the format that
rustc expects. I've run into some issues with that as well, so we warn
in that case.

The decisions to leave static into static broken on MSVC for 1.61–1.66
was made because:

 1) The work around is non-trivial, and we would have to support that
    workaround for a long time
 2) The number of users of Rust in Meson is small
 3) The number of users of Rust in Meson on Windows, with MSVC is tiny
 4) Using rustup to update rustc on windows is trivial, and solves the
    problem completely

Fixes: #10723
Fixes: #11247

Co-authored-by: Nirbheek Chauhan <nirbheek@centricular.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants