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

libfdt: allow building only static or shared libraries #141

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

blmaier
Copy link
Contributor

@blmaier blmaier commented Jun 30, 2024

When using the meson-python backend to compile libfdt, it throws an installation error that it does not know where to install static libraries. We would disable the static libraries, but Meson is always building and installing both the shared and static libraries.

Right now we always build both shared and static libraries because our 'static-build' option needs us to link the dtc binaries against the static libfdt. So we need to make sure static is always built even if the build is configured for shared only.

So instead lets not link against the static library but link all of libfdt's objects directly. We will always have access to the library objects regardless if the build is configured for static or shared.

@blmaier blmaier force-pushed the meson-allow-building-only-static-or-shared-libraries branch from 74fabd8 to 1b75550 Compare October 1, 2024 20:00
@blmaier blmaier closed this Oct 1, 2024
@blmaier blmaier deleted the meson-allow-building-only-static-or-shared-libraries branch October 1, 2024 20:09
@blmaier blmaier restored the meson-allow-building-only-static-or-shared-libraries branch October 1, 2024 20:10
@blmaier
Copy link
Contributor Author

blmaier commented Oct 1, 2024

PR closed as I accidentally deleted branch, re-opening

@blmaier blmaier reopened this Oct 1, 2024
Copy link
Owner

@dgibson dgibson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry it's taken me so long to look at this.

@@ -26,27 +26,33 @@ else
endif

link_args += version_script
libfdt = both_libraries(
libfdt = library(
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love the idea of avoiding both_libraries which seems to have some builtin logic to make our life easier. I thought both_libraries already had support for selecting just one or the other libraro to build.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That fixes the static builds of dtc on NixOS.
The patch also works for dynamic builds.

I'm not sure I understand how both_libraries is supposed to work though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the main difference between library() and both_libraries() is that both_libraries() always builds both the static and dynamic versions of a library. Ultimately that is what I run into problems with, meson-python tries to map all the build artifacts into a Python package, but a static archive doesn't map. I'm guessing NixOS has a similar issue that it doesn't like the static archive.

One reason to use both_libraries() is it makes it simpler to force Meson to statically link your binaries. Since it always builds a static archive, both_libraries() returns the both_libs type which lets you do lib.get_static_lib() to get a direct handle. That is used later in this file to change which library is linked based on the custom static-build option. With just library() you can't access the type, and Meson decides whether to use the static or shared library.

Meson does provide some core options that let the builder influence the library type. --default-library {shared,static,both} and --prefer-static, but neither of them let you enforce a static build. I'm not sure what the value is of an option to require static build, but it does complicate things.

library() function: https://mesonbuild.com/Reference-manual_functions.html#library
both_library() function: https://mesonbuild.com/Reference-manual_functions.html#both_libraries
--default-library/--prefer-static: https://mesonbuild.com/Builtin-options.html#core-options

)

if static_build
libfdt_static_dep = declare_dependency(
include_directories: libfdt_inc,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the include directories me listed in both static and dynamic cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the dynamic case it uses the libfdt_dep which is itself a dependency that has already included libfdt_inc.

libfdt_static_dep = declare_dependency(
include_directories: libfdt_inc,
)
libfdt_static_objs = libfdt.extract_all_objects()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, this is doing static linking by individually linking against every constituent .o file. I guess that will work, but it seems convoluted, and I think we still want the ability to build a .a static library.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't happy with this approach either, it was the only way I could think to allow for static-build=true to require static linking, while also not requiring Meson compiles a static archive when static-build=false.

Switching from both_libraries() to library() doesn't stop us from building a .a static archive. It just changes it so Meson won't always build it, but instead build one or the other depending on the build configuration.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking over this with fresh eyes, I think a much easier solution would be to switch between library() and static_library(). I pushed an update with that change.

Some build systems like meson-python and NixOS have difficulties with
static archives. The `both_library()` Meson function always builds both
shared and static libraries. Instead of using `both_libraries()` to
get a handle on a static lib, use `static_library()` directly.

Signed-off-by: Brandon Maier <[email protected]>
@blmaier blmaier force-pushed the meson-allow-building-only-static-or-shared-libraries branch from 1b75550 to 56a7d0c Compare October 17, 2024 23:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants