bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

Fix libgio/libnotify dlopening on OpenBSD

RESOLVED WORKSFORME

Status

()

Core
XPCOM
RESOLVED WORKSFORME
6 years ago
6 years ago

People

(Reporter: gaston, Assigned: gaston)

Tracking

Trunk
x86
OpenBSD
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
Related/opposed to #624530, on OpenBSD we control .so.X.Y versions in the portstree, so the usual way of dealing with 3rd-party code dlopen()'ing other libs is to make them target libfoo.so (ld.so takes care of opening the correct one) - libgio-2.0.so.0 doesnt exist nor libnotify.so.4, we only have libgio-2.0.so.3400.0 / libnotify.so.4.0 but the versions changes over time.

Per irc discussion with karlt, we might aswell directly link with gio for g_app_* calls, but for libnotify the dlopen() way it is now avoids a dependency.

For more context, i've 'fixed' a similar issue the same way in bug #667325, and discussed to death that openbsd specificity in nspr's bug #650772.
(Assignee)

Comment 1

6 years ago
Created attachment 673541 [details] [diff] [review]
fallback to dlopen(libgio-2.0.so), dlopen(libnotify.so) for OpenBSD

This patch fixes the issue for me - of course this might not be the best fix, i'm open to better ideas - i can wrap it within #ifdef __OpenBSD__ if needed, even if it's ugly
Assignee: nobody → landry
Attachment #673541 - Flags: review?(karlt)

Comment 2

6 years ago
webapprt/gtk2/webapprt.cpp has one more dlopen("libnotify.so.{4,1}",...) per bug 780530
(Assignee)

Comment 3

6 years ago
Right, i'll integrate it in my diff once i know the correct way to fix it for OpenBSD :)
I can just give my two cents from the Linux side.
I'm a bit scared about that unconditional fallback. On a Linux system if the respective library is not found it's highly unlikely that the unversioned link (if at all installed) points to the correct (aka compatible) library. There is high risk that the blindly loaded and used library causes unexpected behaviour or more likely crashes.
From that point of view I rather would prefer a platform specific #ifdef in that case. It's also not uglier than the current patch IMHO. With an ifdef it's actually easier to get the reasoning of the code.
Comment on attachment 673541 [details] [diff] [review]
fallback to dlopen(libgio-2.0.so), dlopen(libnotify.so) for OpenBSD

Yes, trying libnotify.so could be detrimental on Linux, and OpenBSD.

The string specified should be the string that identifies a series of backwardly compatible ABIs.  AFAIK "libnotify.so" is not that string on any OS.
If the string we need for OpenBSD is different to other unices, then we'll need a #ifdef.

<karlt>   gaston: for nsGIOService, i think we can link directly to g_app_info_get_commandline now
<karlt>   that is GLib 2.20
<karlt>   gaston: just bump GIO_VERSION in configure.in

That is the best solution for libgio-2.0.so.
Attachment #673541 - Flags: review?(karlt) → review-
<karlt>   gaston: i assumed the .1 or .4 came from the source package?
<gaston>  karlt: atm on openbsd libnotify is .so.4.0 but 4.0 comes from the portstree, it could be any arbitrary version

http://www.openbsd.org/cgi-bin/man.cgi?query=dlopen says
"When a shared library is specified [...] with a partial
 version, the same library search rules apply that are used for
 ``intrinsic'' shared library searches."

What rules are used for "intrinsic" shared library searches for
"libnotify.so.4"?
or "libnotify.so.4.0"?
or to what does "partial version" refer?

Does OpenBSD use ELF?
What is the SONAME or equivalent field of libnotify.so.4.0?
What is in the NEEDED or equivalent field of objects depending on libnotify?

http://www.openbsd.org/cgi-bin/man.cgi?query=ld.so says
"To quickly locate the required shared objects in the filesystem, ld.so
 may use a ``hints'' file, prepared by the ldconfig(8) utility, in which
 the full path specification of the shared objects can be looked up by
 hashing on the 3-tuple <library-name, major-version-number,
 minor-version-number>."
That suggests, that minor versions are included in NEEDED or OpenBSD equivalent, and perhaps more recent minor versions will be searched.
Is that what happens?

Comment 7

6 years ago
karlt, that is exactly right, minor versions are listed in NEEDED, major version must match exactly, minor version of the library must be identical or newer than the version requested.

OpenBSD doesn't use SONAME. Objects depending on libnotify currently have "NEEDED      libnotify.so.4.0" but the library version will get bumped when the API/ABI changes.

More info at http://www.openbsd.org/faq/ports/specialtopics.html#SharedLibs
Thanks, Stu.  That sounds a good system.
Sounds like the "libnotify.so.4.0" string can be used for OpenBSD then.
For clarity, a #ifdef would be best, as it would indicate the background to the code.

The one thing I'm still not clear on is what was meant by "partial version".
Would passing "libnotify.so.4" to dlopen find "libnotify.so.4.0", if present?
(Assignee)

Comment 9

6 years ago
(In reply to Karl Tomlinson (:karlt) from comment #8)
> Thanks, Stu.  That sounds a good system.
> Sounds like the "libnotify.so.4.0" string can be used for OpenBSD then.
> For clarity, a #ifdef would be best, as it would indicate the background to
> the code.

That will work, until notify gets a new version and gets a slightly modified ABI, in case we bump the major in the portstree (since we control the version), and it becomes libnotify.so.5.0, and i have to open a new bug asking for the #ifdef to be changed, and pulled down to aurora/beta, and then there's a new libnotify version -- rince, repeat. endless joy for the maintainer (me).

using libnotify.so will just work on openbsd - regardless of the real version, see #667325. And i've already discussed that behaviour to death in #650772, explaining everything again is just depressing.
 
> The one thing I'm still not clear on is what was meant by "partial version".
> Would passing "libnotify.so.4" to dlopen find "libnotify.so.4.0", if present?

Yes, that would also work in theory (that's the existing code, but for some reason apparently it doesnt work.. more debugging & time needed for that), but please consider using .so only for OpenBSD, which works here.

That's the only simple way to fix it forever, and it's not like if someone is going to build his own incompatible version of libnotify. Users use the packages we provide them, they are upgraded altogether as a whole, and the pkg tools even have the logic to keep the libs needed by packages not updated for whatever reason.

(In reply to Karl Tomlinson (:karlt) from comment #5)
> <karlt>   gaston: for nsGIOService, i think we can link directly to g_app_info_get_commandline now
> <karlt>   that is GLib 2.20
> <karlt>   gaston: just bump GIO_VERSION in configure.in
> 
> That is the best solution for libgio-2.0.so.

I get it that's a requirement change in the build env, so yet another separate bug. But then, will that allow us to remove that dlopen(), since we link with -lgio-2.0 anyway when --enable-gio is in use ?
(In reply to Landry Breuil (:gaston) from comment #9)
> That will work, until notify gets a new version and gets a slightly modified
> ABI, in case we bump the major in the portstree (since we control the
> version), and it becomes libnotify.so.5.0

In that case, it will also become libnotify.so.5 on linux, and will require a modification in mozilla code as well. It's not a problem for you to dlopen libnotify.so.4.0.
(Assignee)

Comment 11

6 years ago
What apparently i failed to explain is that it's a _coincidence_ that libnotify.so is .4.0 on OpenBSD...

Oh well, let's mark it resolved, since i've tried with fx 17.0b2 unpatched, and dlopening libnotify.so.4 correctly works and shows the popup upon downloading a file. will reopen if the version changes...

The gio side has been fixed by bug #805202.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.