atlas-engineer / nyxt

Nyxt - the hacker's browser.
https://nyxt-browser.com/
9.65k stars 406 forks source link

Should nyxt dlopen versioned libfixposix and libwebkit2gtk rather than .so ? #3043

Closed bubbleguuum closed 11 months ago

bubbleguuum commented 1 year ago

Currently, the nyxt binary dynamically load (dlopen) .so files for libfixposix and libwebkit2gtk on startup.

For example, with my openSUSE RPM package it will dlopen libfixposix.so and libwebkit2gtk-4.1.so.

The problem is that at least on openSUSE (and I suppose Debian), these .so files are in the devel packages of libfixposix and libwebkit2gtk-4.1 while the actual libs they point to are in their respective non-devel packages.

So for the nyxt RPM package to have these 2 required .so files, there are 2 solutions:

For the openSUSE package, I opted for the second solution (see spec file) resulting in the script creating the 2 .so files in /usr/libexec/nyxt:

/usr/libexec/nyxt> ls -l
total 8
lrwxrwxrwx 1 root root 28 Jun 19 18:13 libfixposix.so -> ../../lib64/libfixposix.so.4
lrwxrwxrwx 1 root root 34 Jun 19 18:13 libwebkit2gtk-4.1.so -> ../../lib64/libwebkit2gtk-4.1.so.0

And a wrapper shell script to launch nyxt with LD_LIBRARY_PATH pointing to directory above:

#!/bin/sh
LD_LIBRARY_PATH=/usr/libexec/nyxt exec -a nyxt nyxt.bin "$@"

I tried to get this package included in the official openSUSE Tumbleweed repo but it was refused due to how nyxt handled these .so which was deemed "wrong". The argument of the openSUSE devs is that no program should dynamically load system installed .so as there could be versioning problems and that the only valid case for loading .so is for a program's private plugins. Long story short, it should not load system installed .so directly but instead the versioned files. So in that case it should dlopen libfixposix.so.4 and libwebkit2gtk-4.1.so.0. The appended number suffix for each lib is its major lib version that the binary was built upon.

It would be great if the nyxt build could be modified to accommodate that, which should also make the wrapper hack above not needed. Not sure how it is easy to do, but it would have at build time to figure out the major version of the libs and make sure that whatever is calling dlopen make use of it.

Ambrevar commented 1 year ago

Does not look like something we have control over.

The DLOPEN calls are made by CFFI and SBCL, not by Nyxt.

Maybe we can instruct CFFI to load libraries differently, but I don't know how to test if this would satisfy OpenSUSE. Sounds very much like a packaging design issue on Suse's side (I don't really get their rationale).

Maybe someone could try tweking cl-webkit and build the Nyxt Suse package and see if it loads the right .so then. For the record, the libwebkit libs to load are specified here: https://github.com/joachifm/cl-webkit/blob/e4b1952ff05b8c0d01eee681f8d77df4818b9b43/webkit2/webkit2.init.lisp#L13

bubbleguuum commented 1 year ago

Here's the stack trace for the dlopen:

4: (SB-SYS:DLOPEN-OR-LOSE #S(SB-ALIEN::SHARED-OBJECT :PATHNAME #P"libfixposix.so" :NAMESTRING "libfixposix.so" :HANDLE NIL :DONT-SAVE NIL))
5: (SB-ALIEN::TRY-REOPEN-SHARED-OBJECT #S(SB-ALIEN::SHARED-OBJECT :PATHNAME #P"libfixposix.so" :NAMESTRING "libfixposix.so" :HANDLE NIL :DONT-SAVE NIL))
6: (SB-SYS:REOPEN-SHARED-OBJECTS)
7: (SB-IMPL::FOREIGN-REINIT)
8: (SB-IMPL::REINIT T)
9: ((FLET SB-UNIX::BODY :IN SB-IMPL::START-LISP))
10: ((FLET "WITHOUT-INTERRUPTS-BODY-3" :IN SB-IMPL::START-LISP))
11: (SB-IMPL::%START-LISP)

I do not know how easy it would be to pass the the versioned .so.

See here for the failed package submission discussion (that did not go very far): https://build.opensuse.org/request/show/1087966

Independently of that, the fact that in many distros the .so files are in the devel packages is a real problem. Making the nyxt package depend at runtime on devel packages is not a good solution (webkitgtk devel depencies are huge!) which prompted me to implement the wrapper solution I used (which is rejected by openSUSE as would adding a runtime devel dependency).

bubbleguuum commented 1 year ago

OK, webkit was trivial to fix with a patch to add libwebkit2gtk-4.1.so.0 on top:

 (:unix (:or
            "libwebkit2gtk-4.1.so.0"
              ...

Now libfixposix remains.

bubbleguuum commented 1 year ago

Yay, libfixposix trivially fixed in iolib/src/syscalls/ffi-functions-unix.lisp replacing existing code with:

(eval-when (:compile-toplevel :load-toplevel :execute)
  (load-foreign-library "libfixposix.so.4"))
Ambrevar commented 1 year ago

I don't know what cl-webkit and libfixposix can do about this, because they can't possibly list all versions in define-foreign-library. Thus it makes sense to have the .so at the top instead of the .so.VERSION.

To me it looks like something that OpenSuse must fix on its end.

bubbleguuum commented 1 year ago

It is something I fixed at the packaging level, applying this patch before compilation:

https://build.opensuse.org/package/view_file/network/nyxt/so_ver_fix.patch?expand=1

I can now resubmit the package for inclusion in Tumbleweed and we'll see how it goes, but I cannot see why it would not be accepted now.

This patch would also be useful for packing nyxt for any distro that puts it's .so files (the symbolic link to the real versioned lib file) in a devel package, for not having a runtime dependency on devel packages (which for openSUSE would not be accepted anyway).

aartaka commented 1 year ago

I don't know what cl-webkit and libfixposix can do about this, because they can't possibly list all versions in define-foreign-library. Thus it makes sense to have the .so at the top instead of the .so.VERSION.

Well, there's already Fedora-specific library name in cl-webkit, so why not include the Suse one?

bubbleguuum commented 1 year ago

Glad to report nyxt has been accepted as an official package in openSUSE Tumbleweed:

https://build.opensuse.org/request/show/1094811

jmercouris commented 1 year ago

Woohoo! Does that mean we should upgrade the directions on the README?

bubbleguuum commented 1 year ago

Yes for Tumbleweed. for which user will be to install it with just zypper in nyxt.

Instructions for openSUSE LEAP 15.4 and 15.5 remain the same, or for installing the nyxt-git package I maintain. The LEAP instructions should be updated to mention that if using LEAP 15.5 (the latest version) the repo URL is: https://download.opensuse.org/repositories/home:/bobbie424242:/nyxt/15.5/

I'll see if I can get nyxt in official LEAP 15.5 repo but I'm not sure it is possible. In any case, it would automatically be in 15.6 that will be released next year since LEAP is built upon Tumbleweed.

aadcg commented 11 months ago

Currently, the instructions read:

- [[https://www.opensuse.org/][OpenSuse]] (user-maintained package of stable and latest `git master` build):
  - Tumbleweed:

    #+begin_src sh
    zypper ar https://download.opensuse.org/repositories/home:/bobbie424242:/nyxt/openSUSE_Tumbleweed/ nyxt
    #+end_src

  - LEAP:

    #+begin_src sh
    zypper ar https://download.opensuse.org/repositories/home:/bobbie424242:/nyxt/15.4/ nyxt
    #+end_src

  Then install with =zypper in nyxt= for the latest stable release or =zypper in
  nyxt-git= for the latest `git master` build. Both versions cannot be installed
  at the same time.

Are they out-of-date? Could someone send a patch or instruct on what is incorrect?

If everything is correct, let's close this issue. Thanks.

Ambrevar commented 11 months ago

Well, there's already Fedora-specific library name in cl-webkit, so why not include the Suse one?

@aartaka You're right, we could do that, but then it's a pain to follow up with Fedora / Suse updates. We're better off if they fix the issue on their end.

bubbleguuum commented 11 months ago

It's not really a distro problem.

The issue is that nyxt is opening unversioned .so libs (libwebkit2gtk-4.1.so in webkit2.init.lisp, libfixposix.so in ffi-functions-unix.lisp), or if the .so is not available it fallbacks to a few arbitrary hardcoded versioned variations (for webkit). The thing is that these unversioned .so loaded by nyxt at runtime (which are symbolic links) could point to a library whose version is not compatible with what nyxt was compiled with (if the library's major version has changed). Add to this, that some distros put libwebkit2gtk-4.1.so and libfixposix.so (these exact files) in devel packages and you should not have to install devel packages as a dependency of a user facing program. That's why I believe webkit2.init.lisp has hardcoded several filenames for libwebkit2gtk.

I think the proper fix would be that at compile time the the relevant functions in webkit2.init.lisp and webkit2.init.lisp where replaced to actually load the versioned .so (aka the soname, made of the lib name and its major version) matching the version used for the compilation (for example libwebkit2gtk-4.1.so.0 and libfixposix.so.4). Dynamically loading unversioned .so should be limited to a program's private plugins.

Ambrevar commented 11 months ago

It kind of is a distro problem, more specifically, a problem that plagues all non-functional distros like Suse and Fedora. Nix and Guix, being functional, don't suffer from this issue.

Anyways, what can we do about it? We could add the latest soname to the define-foreign-library call, but that would mean keeping up with the latest release of WebKit. Whenver we fail to stay up-to-date, Suse would fail to find the file.

For libfixposix, we would need to ask upstream.

bubbleguuum commented 11 months ago

Ideally, the build system would determine dynamically the soname of both webkit2gtk and libfixposix libraries nyxt is being built against. For example it, at build time it would dynamically determine that the current major version of libwebkit2gtk-4.1 is 0, thus its soname is libwebkit2gtk-4.1.so.0 and would dynamically edit the relevant function in webkit2.init.lisp before build to reflect that. Same with libfixposix (whose current major version is 4). With such system, whenever upstream increments the major version of libfixposix or webkit2gtk, any new build using these would just work, with any distro, functional or not. Although I understand that might be quite a bit of work for something (major lib versions) that rarely changes and you prefer to leave it to non functional distro packagers to handle it by patching webkit2.init.lisp and ffi-functions-unix.lisp. Or not patching it and expecting major lib versions to never change (and even: not patching it requires the .so files to be installed, which are usually in development packages adding a dependency on them, so not an option on at least openSUSE).

aartaka commented 11 months ago

that rarely changes

Indeed, shared libraries don't change names often, whatever the distro! So it's more or less safe adding sonames to cl-webkit directly. If something breaks, then it will be clear why and the fix will be a predictable one-liner.