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

Improve the use of both_libraries #12632

Merged
merged 4 commits into from
Sep 6, 2024
Merged

Conversation

bruchar1
Copy link
Member

@bruchar1 bruchar1 commented Dec 13, 2023

FIxes #5452

  • Internal dependency objects now carry both_libraries objects, allowing to choose either the static or the shared version of a library, at a later stage. There is now as_static and as_shared methods on dependencies for that (otherwise the default is automatically used)
  • Added a default_both_libraries to configure the default behavior of both_dependencies. Previously, it was always the shared version by default.
  • Using the same option with 'auto' value, it is now possible to build hierarchies of both_libraries, with static dependencies for static libs, and shared dependencies for shared libs (internal dependencies only).

In my monorepo, I need to compile static and shared versions of many common libs, for different uses. For instance, I usually prefer the shared version, but for plugins, I need to compile them with static dependencies to make them independent from the version of the common libs used in the main app. With the proposed changes, I can remove a lot of duplication in my build files, by using both_libraries instead of having to duplicate shared_library and static_library, along with their respective declare_dependency.

@bruchar1 bruchar1 changed the title Both lib deps Improve the use of both_libraries Dec 14, 2023
@bruchar1 bruchar1 marked this pull request as ready for review December 14, 2023 13:15
@bruchar1 bruchar1 requested a review from jpakkane as a code owner December 14, 2023 13:15
@bruchar1 bruchar1 requested a review from dcbaker April 5, 2024 18:08
@bruchar1 bruchar1 force-pushed the both-lib-deps branch 2 times, most recently from f455e58 to 1682604 Compare September 5, 2024 20:09
Copy link
Member

@dcbaker dcbaker left a comment

Choose a reason for hiding this comment

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

I have some implementation suggestions, but overall this looks like a nice change to me.

mesonbuild/build.py Outdated Show resolved Hide resolved
mesonbuild/build.py Outdated Show resolved Hide resolved
mesonbuild/build.py Outdated Show resolved Hide resolved
mesonbuild/build.py Outdated Show resolved Hide resolved
mesonbuild/dependencies/base.py Outdated Show resolved Hide resolved
@bruchar1 bruchar1 force-pushed the both-lib-deps branch 2 times, most recently from 65d0fbe to b8beae4 Compare September 6, 2024 13:48
@bruchar1
Copy link
Member Author

bruchar1 commented Sep 6, 2024

@dcbaker I think I addressed all your comments. typing in build.py is not easy, as this file is not ready for typing yet, so I didn't tried to run mypy on it...

@bruchar1 bruchar1 requested a review from dcbaker September 6, 2024 17:24
Copy link
Member

@dcbaker dcbaker left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me

@dcbaker
Copy link
Member

dcbaker commented Sep 6, 2024

I think I addressed all your comments. typing in build.py is not easy, as this file is not ready for typing yet, so I didn't tried to run mypy on it...

Yeah. Don't. I've tried to fix it several times and it's a huge amount of work to get it all right. I'm hoping to get back to it after I get the machinefile, options, and coredata done.

@dcbaker dcbaker merged commit ce1602c into mesonbuild:master Sep 6, 2024
36 checks passed
@xclaesse
Copy link
Member

I suspect this is breaking gobject-introspection when building GStreamer with default_library=both.

FAILED: subprojects/gstreamer/libs/gst/base/GstBase-1.0.gir 
env PKG_CONFIG_PATH=/home/xclaesse/programmation/gstreamer/builddir/meson-uninstalled PKG_CONFIG=/usr/bin/pkg-config 'CC=ccache cc' /usr/bin/x86_64-linux-gnu-g-ir-scanner --quiet --no-libtool --namespace=GstBase --nsversion=1.0 --warn-all --output subprojects/gstreamer/libs/gst/base/GstBase-1.0.gir '--add-init-section=extern void gst_init(gint*,gchar**);g_setenv("GST_REGISTRY_DISABLE", "yes", TRUE);g_setenv("GST_REGISTRY_1.0", "/no/way/this/exists.reg", TRUE);g_setenv("GST_PLUGIN_PATH_1_0", "", TRUE);g_setenv("GST_PLUGIN_SYSTEM_PATH_1_0", "", TRUE);gst_init(NULL,NULL);' --quiet --c-include=gst/base/base.h -I/home/xclaesse/programmation/gstreamer/subprojects/gstreamer/libs/gst/base -I/home/xclaesse/programmation/gstreamer/builddir/subprojects/gstreamer/libs/gst/base -I/home/xclaesse/programmation/gstreamer/subprojects/gstreamer/. -I/home/xclaesse/programmation/gstreamer/builddir/subprojects/gstreamer/. -I/home/xclaesse/programmation/gstreamer/subprojects/gstreamer/libs -I/home/xclaesse/programmation/gstreamer/builddir/subprojects/gstreamer/libs --filelist=/home/xclaesse/programmation/gstreamer/builddir/subprojects/gstreamer/libs/gst/base/libgstbase-1.0.so.0.2500.0.p/GstBase_1.0_gir_filelist --include=GLib-2.0 --include=GObject-2.0 --include=GModule-2.0 --include=Gst-1.0 --symbol-prefix=gst --identifier-prefix=Gst --pkg-export=gstreamer-base-1.0 --cflags-begin -DG_DISABLE_DEPRECATED -DG_DISABLE_CAST_CHECKS -I/home/xclaesse/programmation/gstreamer/subprojects/gstreamer/. -I/home/xclaesse/programmation/gstreamer/builddir/subprojects/gstreamer/. -I/home/xclaesse/programmation/gstreamer/subprojects/gstreamer/libs -I/home/xclaesse/programmation/gstreamer/builddir/subprojects/gstreamer/libs -I/home/xclaesse/programmation/gstreamer/subprojects/gstreamer/gst/parse -I/home/xclaesse/programmation/gstreamer/builddir/subprojects/gstreamer/gst/parse -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/gobject-introspection-1.0 --cflags-end -I/home/xclaesse/programmation/gstreamer/subprojects/gstreamer/. -I/home/xclaesse/programmation/gstreamer/builddir/subprojects/gstreamer/. -I/home/xclaesse/programmation/gstreamer/subprojects/gstreamer/libs -I/home/xclaesse/programmation/gstreamer/builddir/subprojects/gstreamer/libs -I/home/xclaesse/programmation/gstreamer/subprojects/gstreamer/gst -I/home/xclaesse/programmation/gstreamer/builddir/subprojects/gstreamer/gst --add-include-path=/home/xclaesse/programmation/gstreamer/builddir/subprojects/gstreamer/gst --add-include-path=/usr/share/gir-1.0 --add-include-path=/home/xclaesse/programmation/gstreamer/subprojects/gstreamer/. --add-include-path=/home/xclaesse/programmation/gstreamer/builddir/subprojects/gstreamer/. --add-include-path=/home/xclaesse/programmation/gstreamer/subprojects/gstreamer/libs --add-include-path=/home/xclaesse/programmation/gstreamer/builddir/subprojects/gstreamer/libs --add-include-path=/home/xclaesse/programmation/gstreamer/subprojects/gstreamer/gst --add-include-path=/home/xclaesse/programmation/gstreamer/builddir/subprojects/gstreamer/gst -L/home/xclaesse/programmation/gstreamer/builddir/subprojects/gstreamer/libs/gst/base --library gstbase-1.0 -L/home/xclaesse/programmation/gstreamer/builddir/subprojects/gstreamer/gst --extra-library=glib-2.0 --extra-library=gobject-2.0 --extra-library=gmodule-2.0 --extra-library=girepository-1.0 --sources-top-dirs /home/xclaesse/programmation/gstreamer/subprojects/gstreamer --sources-top-dirs /home/xclaesse/programmation/gstreamer/builddir/subprojects/gstreamer
(process:927104): GLib-GIO-DEBUG: 17:36:04.441: Using cross-namespace EXTERNAL authentication (this will deadlock if server is GDBus < 2.73.3)
(process:927104): GLib-GIO-DEBUG: 17:36:04.447: _g_io_module_get_default: Found default implementation gvfs (GDaemonVfs) for ‘gio-vfs’
/usr/bin/ld: /home/xclaesse/programmation/gstreamer/builddir/tmp-introspectfhl5josf/GstBase-1.0.o: undefined reference to symbol 'gst_init'

@xclaesse
Copy link
Member

The gnome module has a bunch of isinstance(lib, build.SharedLibrary): that would break if you get BothLibraries there I think.

@xclaesse
Copy link
Member

@dcbaker @bruchar1 This is causing GStreamer CI regression: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/7753.

@bruchar1
Copy link
Member Author

I'm not really setup for working with the gnome module. Would running the test (e.g. framework/7 gnome) with both_libraries trigger the bug? My guess is that calling isinstance(lib.get('auto'), build.SharedLibrary): would solve the problem.

Maybe should you open a separate issue to track this?

@xclaesse
Copy link
Member

I managed to write a unit test: #13835.

xclaesse added a commit to xclaesse/meson that referenced this pull request Oct 31, 2024
This partly reverts mesonbuild#12632 and provide an alternative implementation.
The issue is we cannot propagate BothLibraries objects beyond the
interpreter because it breaks many isinstance(obj, SharedLibrary) cases.

Instead make SharedLibrary and StaticLibrary cross reference each other
so we can take their brother library in as_static() and as_shared()
methods.
xclaesse added a commit to xclaesse/meson that referenced this pull request Oct 31, 2024
This partly reverts mesonbuild#12632 and provide an alternative implementation.
The issue is we cannot propagate BothLibraries objects beyond the
interpreter because it breaks many isinstance(obj, SharedLibrary) cases.

Instead make SharedLibrary and StaticLibrary cross reference each other
so we can take their brother library in as_static() and as_shared()
methods.
xclaesse added a commit to xclaesse/meson that referenced this pull request Nov 1, 2024
This partly reverts mesonbuild#12632 and provide an alternative implementation.
The issue is we cannot propagate BothLibraries objects beyond the
interpreter because it breaks many isinstance(obj, SharedLibrary) cases.

Instead make SharedLibrary and StaticLibrary cross reference each other
so we can take their brother library in as_static() and as_shared()
methods.
xclaesse added a commit to xclaesse/meson that referenced this pull request Nov 1, 2024
This partly reverts mesonbuild#12632 and provide an alternative implementation.
The issue is we cannot propagate BothLibraries objects beyond the
interpreter because it breaks many isinstance(obj, SharedLibrary) cases.

Instead make SharedLibrary and StaticLibrary cross reference each other
so we can take their brother library in as_static() and as_shared()
methods.
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.

both_libraries() does not deal with dependencies correctly
3 participants