After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 778193 - Automagic libunwind dependency
Automagic libunwind dependency
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
1.10.x
Other Linux
: Normal normal
: 1.12.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-02-05 06:35 UTC by Mart Raudsepp
Modified: 2017-09-07 09:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch for 1.10 branch (1.55 KB, patch)
2017-02-05 07:06 UTC, Mart Raudsepp
none Details | Review
Patch for 1.10 branch (1.56 KB, patch)
2017-02-05 09:17 UTC, Mart Raudsepp
none Details | Review
configure: Allow specifying libunwind usage explicitly (1.63 KB, patch)
2017-02-09 06:22 UTC, Mart Raudsepp
needs-work Details | Review
meson: Add an option to disable usage of libunwind (3.11 KB, patch)
2017-02-13 19:38 UTC, Thibault Saunier
committed Details | Review
configure.ac patch that adds libdw and libunwind switches (1.85 KB, patch)
2017-08-11 18:47 UTC, Carlos Rafael Giani
none Details | Review
configure.ac patch that adds libdw and libunwind switches , v2 (2.21 KB, patch)
2017-08-11 19:18 UTC, Carlos Rafael Giani
none Details | Review
configure.ac patch that adds libdw and libunwind switches , v3 (2.20 KB, patch)
2017-08-11 20:31 UTC, Carlos Rafael Giani
committed Details | Review

Description Mart Raudsepp 2017-02-05 06:35:19 UTC
1.10.3 has an automatic libunwind package check and usage, without any option to configure it. This leads to uncontrolled dependencies from source builds, what we call automagic dependency, which results in us having to always depend on it (and it seems to be of use to developers, not users) in the package, or patch in a configure switch. Otherwise all of gstreamer becomes broken as it'd be possible to uninstall libunwind:

!!! existing preserved libs:
>>> package: sys-libs/libunwind-1.2_rc1
 *  - /usr/lib64/libunwind.so.8
 *  - /usr/lib64/libunwind.so.8.0.1
 *      used by /usr/lib64/gstreamer-1.0/libgstcoretracers.so (media-libs/gstreamer-1.10.3)


We'll need to patch in a configure switch to control this.
Additionally git master has another optional dependency automagic for the leak tracer (DW), which will be a problem similarly for 1.12 as well.
Comment 1 Mart Raudsepp 2017-02-05 07:06:18 UTC
Created attachment 344965 [details] [review]
Patch for 1.10 branch

configure: Allow specifying libunwind usage explicitly


I'm not sure when I'd get around to patching master, which differs by the addition of DW (libdwarf or something presumably). But if this one gets at least reviewed meanwhile, I'd know that the configure.ac style is fine and do the same to DW on master. My autoconf is a bit rusty.
Comment 2 Mart Raudsepp 2017-02-05 08:52:53 UTC
Comment on attachment 344965 [details] [review]
Patch for 1.10 branch

This patch is broken when --disable-libunwind is passed while libunwind is actually present (errors out wrongly for both enable and auto)
Comment 3 Mart Raudsepp 2017-02-05 09:17:43 UTC
Created attachment 344968 [details] [review]
Patch for 1.10 branch

configure: Allow specifying libunwind usage explicitly

Previous version just had an accidentally missing x prefix in the error test
Comment 4 Mart Raudsepp 2017-02-09 06:22:56 UTC
Created attachment 345289 [details] [review]
configure: Allow specifying libunwind usage explicitly

Previous version still linked to libunwind if present on system at the time with --disable-libunwind due to UNWIND_LIBS getting populated and used regardless.
Comment 5 Tim-Philipp Müller 2017-02-09 08:07:50 UTC
Any chance you could make a patch for the Meson build as well while you're at it?
Comment 6 Mart Raudsepp 2017-02-09 08:47:19 UTC
I'm not familiar enough with meson yet. But if the logic for the last 1.10 patch is right, I can patch master as well for configure.ac (there is a new libdw dependency for the leaks tracer too, that needs a similar treatment).

For meson, I plan to look into it deeper once I'm caught up with all the old stuff delays, as I need it to work with source level package splits I have going on (build everything not in ext/ and sys/ for one package + only a given ext/ or sys/ plugin and if that plugin uses a helper library from the same tarball, have it look at the system version instead as it's provided by the bigger package). So just configurability of libunwind is only a tiny little thing compared to what all I need to be able to switch the packages over :)
Comment 7 Thibault Saunier 2017-02-13 19:38:32 UTC
Created attachment 345666 [details] [review]
meson: Add an option to disable usage of libunwind

Fixes
Comment 8 Tim-Philipp Müller 2017-02-21 16:15:43 UTC
Comment on attachment 345289 [details] [review]
configure: Allow specifying libunwind usage explicitly

>+AM_CONDITIONAL(HAVE_UNWIND, [test "x$HAVE_UNWIND" = "xyes"])

This is not needed, is it? I don't see it used anywhere?
Comment 9 Tim-Philipp Müller 2017-02-23 00:16:57 UTC
Comment on attachment 345289 [details] [review]
configure: Allow specifying libunwind usage explicitly

Please make a patch against master, this does not apply.
Comment 10 Thibault Saunier 2017-02-24 19:23:22 UTC
Comment on attachment 345666 [details] [review]
meson: Add an option to disable usage of libunwind

commit d6dba3fd6fa84bd24705ca72da824edc4af989e7
Author: Thibault Saunier <thibault.saunier@osg.samsung.com>
Date:   Mon Feb 13 15:18:59 2017 -0300

    meson: Add an option to disable usage of libunwind
    
    Fixes https://bugzilla.gnome.org/show_bug.cgi?id=778193
Comment 11 Mart Raudsepp 2017-04-07 00:49:41 UTC
(In reply to Tim-Philipp Müller from comment #9)
> Comment on attachment 345289 [details] [review] [review]
> configure: Allow specifying libunwind usage explicitly
> 
> Please make a patch against master, this does not apply.

Comment #6 already..:
> I'm not familiar enough with meson yet. But if the logic for the last 1.10
> patch is right, I can patch master as well for configure.ac (there is a new
> libdw dependency for the leaks tracer too, that needs a similar treatment).

So is the autoconf logic correct to head towards, to do it for libdw too on master for it to apply...?

Meson is not an option for me until it can take arguments to disable build/install on any of the sys/ or ext/ plugins like autotools gst build system does.
Comment 12 Tim-Philipp Müller 2017-04-07 05:38:44 UTC
Mart, please just rebase your autotools patch on top of master so we can apply it without having to resolve conflicts ourselves :)

The meson part has already been done, there's nothing else to do there.
Comment 13 Tim-Philipp Müller 2017-04-15 14:48:56 UTC
Mart, now is your chance to still get this into 1.12.
Comment 14 Mart Raudsepp 2017-05-05 06:04:05 UTC
I don't see how me not having had time to update the patch myself yet removes the bug.
Comment 15 Carlos Rafael Giani 2017-08-11 18:47:12 UTC
Created attachment 357437 [details] [review]
configure.ac patch that adds libdw and libunwind switches

Here is my take on this. In Yocto, it is necessary to make sure that builds are deterministic, so there was a pressing need for such a patch. It is applicable against current git master.

This introduces --with/--without switches for dw and unwind.
Comment 16 Tim-Philipp Müller 2017-08-11 18:59:32 UTC
Comment on attachment 357437 [details] [review]
configure.ac patch that adds libdw and libunwind switches

>+AC_ARG_WITH([unwind],[AS_HELP_STRING([--with-unwind],[use libunwind])],
>+            [], [with_unwind=auto])

>+AS_IF([test "$with_unwind" = yes],
>+      [PKG_CHECK_MODULES(UNWIND, libunwind)
>...
>+ )

Does this handle the default/auto case correctly?
Comment 17 Carlos Rafael Giani 2017-08-11 19:18:28 UTC
Created attachment 357441 [details] [review]
configure.ac patch that adds libdw and libunwind switches , v2

Yeah, right after I posted it, it hit me that auto was not handled properly...

Here is the updated patch. "auto" worked for me with current git master.
Comment 18 Carlos Rafael Giani 2017-08-11 20:31:41 UTC
Created attachment 357445 [details] [review]
configure.ac patch that adds libdw and libunwind switches , v3

Remove incorrect "LIB" prefix in PKG_CHECK_MODULE calls.
Comment 19 Tim-Philipp Müller 2017-08-13 10:08:07 UTC
Thanks:

commit 4d25706ff1e170a4da3e0d302e25eac8944c939a
Author: Carlos Rafael Giani <dv@pseudoterminal.org>
Date:   Fri Aug 11 21:17:06 2017 +0200

    configure: Add switches for enabling/disabling libdw and libunwind
    
    https://bugzilla.gnome.org/show_bug.cgi?id=778193
Comment 20 Mart Raudsepp 2017-09-06 20:49:17 UTC
Please include this in upcoming 1.12.3 release too (not included in 1.12 branch yet)