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

static build: libnvme-mi depends on a shared libsystemd #1734

Closed
igaw opened this issue Nov 17, 2022 · 18 comments · Fixed by linux-nvme/libnvme#542
Closed

static build: libnvme-mi depends on a shared libsystemd #1734

igaw opened this issue Nov 17, 2022 · 18 comments · Fixed by linux-nvme/libnvme#542

Comments

@igaw
Copy link
Collaborator

igaw commented Nov 17, 2022

libsystemd seems to be only available as shared library. At least on Debian.

Building against a current version of libnvme with following configuration:

CFLAGS=-static meson setup .build --wrap-mode forcefallback --default-library=static

results in:

FAILED: subprojects/libnvme/test/test-mi-mctp 
cc  -o subprojects/libnvme/test/test-mi-mctp subprojects/libnvme/test/test-mi-mctp.p/mi-mctp.c.o subprojects/libnvme/test/test-mi-mctp.p/utils.c.o -Wl,--as-needed -Wl,--no-undefined -Wl,-O1 -static -Wl,--start-group subprojects/libnvme/src/libnvme-mi-test.a subprojects/libnvme/ccan/libccan.a -Wl,--end-group
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: subprojects/libnvme/src/libnvme-mi-test.a.p/nvme_mi-mctp.c.o: in function `nvme_mi_scan_mctp':
mi-mctp.c:(.text+0x804): undefined reference to `sd_bus_default_system'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0x84a): undefined reference to `sd_bus_call_method'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0x876): undefined reference to `sd_bus_message_enter_container'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0x89a): undefined reference to `sd_bus_message_peek_type'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0x8b8): undefined reference to `sd_bus_message_enter_container'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0x8ed): undefined reference to `sd_bus_message_read'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0x90b): undefined reference to `sd_bus_message_enter_container'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0x95e): undefined reference to `sd_bus_message_enter_container'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0x975): undefined reference to `sd_bus_message_read_array'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0x987): undefined reference to `sd_bus_message_exit_container'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0x9d4): undefined reference to `sd_bus_message_exit_container'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0x9e8): undefined reference to `sd_bus_message_peek_type'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0xa04): undefined reference to `sd_bus_message_enter_container'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0xa25): undefined reference to `sd_bus_message_read'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0xa6e): undefined reference to `sd_bus_message_peek_type'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0xa8a): undefined reference to `sd_bus_message_enter_container'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0xaa8): undefined reference to `sd_bus_message_peek_type'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0xad2): undefined reference to `sd_bus_message_enter_container'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0xaec): undefined reference to `sd_bus_message_read'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0xb27): undefined reference to `sd_bus_message_read'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0xb3b): undefined reference to `sd_bus_message_exit_container'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0xb90): undefined reference to `sd_bus_message_exit_container'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0xbcb): undefined reference to `sd_bus_message_skip'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0xbf7): undefined reference to `sd_bus_message_read'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0xc13): undefined reference to `sd_bus_message_skip'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0xc6a): undefined reference to `sd_bus_message_exit_container'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0xcf3): undefined reference to `sd_bus_message_exit_container'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0xd04): undefined reference to `sd_bus_message_exit_container'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0xd89): undefined reference to `sd_bus_message_exit_container'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0xe20): undefined reference to `sd_bus_error_free'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0xe2a): undefined reference to `sd_bus_message_unref'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0xe34): undefined reference to `sd_bus_unref'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0xf8d): undefined reference to `sd_bus_error_free'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0xf97): undefined reference to `sd_bus_message_unref'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0xfa1): undefined reference to `sd_bus_unref'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0x107b): undefined reference to `sd_bus_message_exit_container'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0x1089): undefined reference to `sd_bus_error_free'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0x1093): undefined reference to `sd_bus_message_unref'
/usr/lib64/gcc/x86_64-suse-linux/12/../../../../x86_64-suse-linux/bin/ld: mi-mctp.c:(.text+0x109d): undefined reference to `sd_bus_unref'
collect2: error: ld returned 1 exit status

That begs the question if we should stop trying to get a static build setup running or not.

@jk-ozlabs libsystemd is sucked in via the libnvme-mi library. As I understand we just need the D-Bus API. So I wonder how realistically you see it that we replace it with something else? If that even exists?

@igaw igaw added the question label Nov 17, 2022
@jk-ozlabs
Copy link
Collaborator

A couple of options:

  1. Use a non-dbus-enabled libnvme-mi for those builds. The libsystemd dependency is optional, and we only use that to query available MI endpoints on the system (which we haven't even plumbed that through to nvme-cli yet...).
  2. Use a different dbus library - the usage of the dbus API is pretty light, so changing shouldn't be much of an issue.

Either way works for me. Is there a particular use-case for the static builds that might guide this choice?

@jk-ozlabs
Copy link
Collaborator

We could always use the libdbus lower-level library, too. Just that:

If you use this low-level API directly, you're signing up for some pain.

@igaw
Copy link
Collaborator Author

igaw commented Nov 18, 2022

I also thought that the dependency to libsystemd is optional but my attempt to build nvme-cli statically went horrible wrong. That means something is still not working as expected.

I'd say we don't need to switch a to different D-Bus binding if it's optional. If someone really wants this feature I accept contributions.

(I know libdbus very well and it's not so bad as it sounds, it's just a lot of boiler plate code we have to add to make it work)

@jk-ozlabs
Copy link
Collaborator

ok, sounds like I need to do some testing here then, and ensure that we're disabling the correct parts without libsystemd. Will keep you posted.

@igaw
Copy link
Collaborator Author

igaw commented Nov 18, 2022

This might be related how I was passing the -static flag to the compiler. There is a different behavior if the environment has CLFAGS=-static or it is passed in via the --cross-file file.

@igaw
Copy link
Collaborator Author

igaw commented Nov 18, 2022

FWIW, here is my current draft version of my attempt: https://github.com/igaw/nvme-cli/tree/build-add-more-CI

@jk-ozlabs
Copy link
Collaborator

So it looks like having a dynamic libsystemd present still allow the libnvme pkg-config check to proceed, and we will happily link the .a with symbols that would still require resolving later:

$ objdump -t install.static/lib/x86_64-linux-gnu/libnvme-mi.a | grep UND | grep sd_
0000000000000000         *UND*	0000000000000000 sd_bus_default_system
0000000000000000         *UND*	0000000000000000 sd_bus_call_method
0000000000000000         *UND*	0000000000000000 sd_bus_message_enter_container
0000000000000000         *UND*	0000000000000000 sd_bus_message_peek_type
0000000000000000         *UND*	0000000000000000 sd_bus_message_read
0000000000000000         *UND*	0000000000000000 sd_bus_message_read_array
0000000000000000         *UND*	0000000000000000 sd_bus_message_exit_container
0000000000000000         *UND*	0000000000000000 sd_bus_message_skip
0000000000000000         *UND*	0000000000000000 sd_bus_error_free
0000000000000000         *UND*	0000000000000000 sd_bus_message_unref
0000000000000000         *UND*	0000000000000000 sd_bus_unref

(this isn't specific to systemd; the same applies for libc symbols, for example).

This isn't necessarily an invalid configuration, as those symbols may be later resolvable - either as a later link stage, or at runtime. For an eventual static binary though, we probably want all of those to be resolved, and cannot do at a later link stage, because there is no static libsystemd.

So, we probably want some way to explicitly disable the libsystemd dependency, and require that for a static nvme-cli. Manually disabling a dependency seems to be discouraged (see mesonbuild/meson#8224), so instead we may need a feature option. Perhaps a new option for libnvme:

 -DDBUS=no

- which we would apply to static builds of nvme-cli?

@jk-ozlabs
Copy link
Collaborator

... which is still a little ugly, as it somewhat complicates the user-interface to the libnvme build system. Also --default-library=both will not support dbus, even in the shared lib.

So maybe the best approach would be to switch to libdbus after all?

@jk-ozlabs
Copy link
Collaborator

(are you linking libc statically too?)

@igaw
Copy link
Collaborator Author

igaw commented Nov 21, 2022

What about making the discovery feature which depends on systemd optional? This would be way cleaner in my opinion.

@jk-ozlabs
Copy link
Collaborator

yep, it already is - but we would need an explicit option to disable it even if libsystemd is found (because the libsystemd that does get found is unsuitable to link against).

I've already done a patch to convert to libdbus though, just chasing down a memory leak. Shouldn't be too much more work!

@igaw
Copy link
Collaborator Author

igaw commented Nov 21, 2022

My idea was to use the option to make the dependency explicit. So if the user wants to use it we look for libsystemd and fail if we can't link. The static build for nvme-cli would just disable it and we would be fine.

But if you got libdbus working, than this is also cool :)

@jk-ozlabs
Copy link
Collaborator

My idea was to use the option to make the dependency explicit.

Not sure what you mean there - the dependency is already explicit in the libnvme meson.build? Is there some trickery we should be doing in the nvme-cli meson rules?

@jk-ozlabs
Copy link
Collaborator

Have just uploaded the WIP libdbus change. I'll do some testing, but let me know if you have any thoughts in the meantime.

@igaw
Copy link
Collaborator Author

igaw commented Nov 22, 2022

W00t! This looks great and pretty straightforward.

I'v just checked if libdbus is simple to build as a static build. And it turns out the project is using Meson and it was dead simple to get a static library build.

This means we should be able to add this to Meson's Wrap DB and can pull it via subprojects. Yay!

@igaw
Copy link
Collaborator Author

igaw commented Nov 22, 2022

@igaw igaw added enhancement and removed question labels Nov 22, 2022
jk-ozlabs added a commit to CodeConstruct/libnvme that referenced this issue Nov 23, 2022
libsystemd is not generally available as a static library, and we would
like to produce a static nvme-cli. This change switches to libdbus
instead.

This requires slightly more boilerplate code for the dbus
marshalling/unmarshalling as part of the MCTP endpoint scan, but means
we have a simpler upstream lib dependency.

Fixes: linux-nvme/nvme-cli#1734

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
@igaw
Copy link
Collaborator Author

igaw commented Nov 23, 2022

Let's keep this issue open until we have figured out how to pull in libdbus via subprojects.

@igaw igaw reopened this Nov 23, 2022
@igaw
Copy link
Collaborator Author

igaw commented Nov 23, 2022

On the second thought. No let's create a new issue for this :)

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