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

Failure to build binary using proper source directory #2360

Closed
bubbleguuum opened this issue Jun 11, 2022 · 22 comments
Closed

Failure to build binary using proper source directory #2360

bubbleguuum opened this issue Jun 11, 2022 · 22 comments
Labels
3-series Related to releases whose major version is 3. build high

Comments

@bubbleguuum
Copy link
Contributor

bubbleguuum commented Jun 11, 2022

Observed using latest master as of the date of this report.

I'm using a custom build of nyxt installed on a system.

Any functionality that requires loading the installed source lisp files fails (for example using describe-function), because the built nyxt binary is looking for them in the source folder used for the build, rather than the installed source dir:

<WARN> [23:25:28] Warning: Error while processing the "nyxt:" URL: The file #P&quot;/home/abuild/rpmbuild/BUILD/nyxt-2.2.4+git20220610.fb7645ae/source/mode/document.lisp&quot; does not exist: No such file or directory
<INFO> [23:25:28] Failed to load URL nyxt:describe-function?universal=%1Bnil&function=%1Bnyxt%2Fdocument-mode%3Aprevious-heading in buffer 686.

/home/abuild/rpmbuild/BUILD/nyxt-2.2.4+git20220610.fb7645ae refers to the directory used to build nyxt (for reference it is built locally as a rpm package, in a sandbox, by Open Build Service).

The resulting RPM installs the source in /usr/share/nyxt/source but the nyxt binary think they are in /home/abuild/rpmbuild/BUILD/nyxt-2.2.4+git20220610.fb7645ae.

I have a similar problem with gtk extensions with this message on startup (weirdly, it appears twice):

<INFO> [23:25:16] GTK extensions directory: #P"/home/abuild/rpmbuild/BUILD/nyxt-2.2.4+git20220610.fb7645ae/libraries/web-extensions/"
<INFO> [23:25:16] GTK extensions directory: #P"/home/abuild/rpmbuild/BUILD/nyxt-2.2.4+git20220610.fb7645ae/libraries/web-extensions/"

So, how to have these directories point the correct install location ?

Build commands are nothing special:

make all NYXT_SUBMODULES=false
make PREFIX=/usr install
@Ambrevar
Copy link
Member

Ambrevar commented Jun 13, 2022

Ah, you are opening this issue right on time! I was going to ask for someone to test this before we release 3.0 :p

I just tried here again and it works for me with

make install DESTDIR=/tmp/nyxtdest/ NYXT_SUBMODULES=false

(I'm disabling submodules because I'm on Guix.)

Could you try setting DESTDIR see if it makes a difference?

@aartaka Any idea why the GTK extensions dir message appears twice?

@Ambrevar
Copy link
Member

Actually I may have misunderstood your issue. @bubbleguuum Can you post an exact recipe for your issue? How do you start nyxt, what command do you run with what input, what do you expect, what do you see?

@bubbleguuum
Copy link
Contributor Author

I'm building latest git for openSUSE Tumbleweed (TW).
I'm going to publish the RPM recipe (.spec file) soon on openbuildservice.org (in my user projects) so you can have a look at it. I will also investigate a bit more to check if the issue is on my side.
The ultimate goal is to have Nyxt 3.0 integrated into official TW repo, so users can just install it with zypper install nyxt.

Currently, gcc 12.1 is giving me a compilation error on tabs.c, but I'm going to open a separate issue for that, with the compiler output. It compiled fine 2 days ago, but Tumbleweed being a rolling distro, it is a moving target.

@bubbleguuum
Copy link
Contributor Author

There is no compilation issue with gcc 12.1, just warnings that I thought made the compilation fail but it was something else.

@Ambrevar
Copy link
Member

To clarify, I was asking for a usage recipe, not the recipe you've to build Nyxt ;)

But anyways, I can reproduce locally now, I'll handle it.

@bubbleguuum
Copy link
Contributor Author

bubbleguuum commented Jun 13, 2022

Ah sorry for the misunderstanding.
Glad you reproduced it.

Other packaging related issues for which rpmlint complain:

  • make install installs .c and .h files in /usr/share/nyxt/libraries/web-extensions/. I suppose this is not needed ?
  • installed libnyxt.so is not stripped:
[   52s] nyxt.x86_64: W: unstripped-binary-or-object /usr/share/nyxt/libraries/web-extensions/libnyxt.so
[   52s] This executable should be stripped from debugging symbols, in order to take
[   52s] less space and be loaded faster. This is usually done automatically at
[   52s] buildtime by rpm.
  • this warning about the nyxt binary:
nyxt.x86_64: W: position-independent-executable-suggested /usr/bin/nyxt
[   52s] This executable should be position independent (all binaries should).  Check
[   52s] that it is built with -fPIE/-fpie in compiler flags and -pie in linker flags.
  • this warning (which can be ignored but mentioning for completeness):
[   52s] nyxt.x86_64: E: arch-dependent-file-in-usr-share (Badness: 590) /usr/share/nyxt/libraries/web-extensions/libnyxt.so
[   52s] This package installs an ELF binary in the /usr/share hierarchy, which is
[   52s] reserved for architecture-independent files only.

@Ambrevar Ambrevar added build 3-series Related to releases whose major version is 3. labels Jun 13, 2022
@Ambrevar
Copy link
Member

Thanks for the report!

make install installs .c and .h files in /usr/share/nyxt/libraries/web-extensions/. I suppose this is not needed ?

No, but for the sake of completeness, all sources are included.

installed libnyxt.so is not stripped:

I believe this is up to the packagers to do it. Are we missing a compilation flags in the makefile?

nyxt.x86_64: W: position-independent-executable-suggested /usr/bin/nyxt

Does not apply to SBCL executables I believe.

[ 52s] nyxt.x86_64: E: arch-dependent-file-in-usr-share (Badness: 590) /usr/share/nyxt/libraries/web-extensions/libnyxt.so
[ 52s] This package installs an ELF binary in the /usr/share hierarchy, which is

This one is tricky, because I don't know how to find libraries reliably from Common Lisp. Packagers are free to install it wherever they want, so there should be a dynamic way to resolve the library location at run-time.

An environment variable maybe?

For now I'm storing it together with the source because the nyxt.asd install location is saved within the binary.

@Ambrevar
Copy link
Member

I know what's going on: we are correctly (?) setting the ASDF location for nyxt.asd, but this has nothing to do with source code location, which is a compiler-dependent feature.

SBCL hard-codes the location of its compiled files. I'm not sure what to do about this.

  • John's suggestion: wrap around swank/backend:find-source-location and substitute the build path for the installation path. This means that some Swank utils won't be usable, like swank:find-definition-for-thing in function-lambda-string. Doable, but not very pretty.
  • My suggestion: install the source first, then build them. This way the compiler will hard code the final installation paths. Downside: it's awkward to package.

Any ideea, anyone?

@bubbleguuum
Copy link
Contributor Author

bubbleguuum commented Jun 13, 2022

Thanks for the report!

make install installs .c and .h files in /usr/share/nyxt/libraries/web-extensions/. I suppose this is not needed ?

No, but for the sake of completeness, all sources are included.

Understood.

For info, rpmlint complains with a bunch of these (one per .c/.h file):

nyxt.x86_64: E: devel-file-in-non-devel-package (Badness: 50) /usr/share/nyxt/libraries/web-extensions/alarms.c

This adds up to a lot of Badness score which makes the RPM build fail.

This can be fixed (on the packaging side) by either:

  • removing installed .c/.h files
  • making them ignored by a rpmlintrc file containing:
addFilter(".*devel-file-in-non-devel-package.*c")
addFilter(".*devel-file-in-non-devel-package.*h")
  • making a nyxt-devel package maybe containing all the source (Lisp+C) with the main package excluding these.

installed libnyxt.so is not stripped:

I believe this is up to the packagers to do it. Are we missing a compilation flags in the makefile?

Yes it can be stripped by packagers. But I think it would be better to do it in the makefile (or in lisp code called by the makefile)
just invoking strip -s on libnyxt.so just after it is compiled.

nyxt.x86_64: W: position-independent-executable-suggested /usr/bin/nyxt

Does not apply to SBCL executables I believe.

It might be only for static binaries (which the nyxt executable is not) but not an expert on that topic.

[ 52s] nyxt.x86_64: E: arch-dependent-file-in-usr-share (Badness: 590) /usr/share/nyxt/libraries/web-extensions/libnyxt.so
[ 52s] This package installs an ELF binary in the /usr/share hierarchy, which is

This one is tricky, because I don't know how to find libraries reliably from Common Lisp. Packagers are free to install it wherever they want, so there should be a dynamic way to resolve the library location at run-time.

An environment variable maybe?

For now I'm storing it together with the source because the nyxt.asd install location is saved within the binary.

Wouldn't it work to put it in $PREFIX/lib64, with $PREFIX being derived from the nyxt.asd install location ?
For example /usr/share/nyxt/nyxt.asd => /usr/lib64/libnyxt.so.
I suppose that this lib is dlopen'ed at runtime ? What specific actions in Nyxt cause it to load it ?

@Ambrevar Ambrevar added the high label Jun 14, 2022
@Ambrevar
Copy link
Member

removing installed .c/.h files

I'll update the .asd to not include the .c/.h files.

But I think it would be better to do it in the makefile (or in lisp code called by the makefile)

I beg to differ: it's an OS-level configuration detail. For instance Arch Linux or Gentoo let the user decide whether they want to strip the library or not. Applications should not force it on their side.

Wouldn't it work to put it in $PREFIX/lib64, with $PREFIX being derived from the nyxt.asd install location ?
For example /usr/share/nyxt/nyxt.asd => /usr/lib64/libnyxt.so.

/usr/lib64/ is only looked up if your dynamic linker is configured to check it. It's not looked up on OSes like Guix or Nix.

I suppose that this lib is dlopen'ed at runtime ? What specific actions in Nyxt cause it to load it ?

When we enable WebKit extensions, in renderer/gtk.lisp:

(gobject:g-signal-connect
       context "initialize-web-extensions"
       (lambda (context)
         (with-protect ("Error in \"initialize-web-extensions\" signal thread: ~a" :condition)
           (webkit:webkit-web-context-set-web-extensions-directory
            context
            (uiop:native-namestring gtk-extensions-path)))))

@Ambrevar
Copy link
Member

@bubbleguuum
Copy link
Contributor Author

bubbleguuum commented Jun 14, 2022

/usr/lib64/ is only looked up if your dynamic linker is configured to check it. It's not looked up on OSes like Guix or Nix.

Doesn't nyxt dlopen libnyxt.so using its full absolute path, making the dynamic linker configured loading path not relevant ? If that's the case, it could load it from /usr/lib64/ (or /usr/lib64/nyxt, relatively to the install prefix) rather than a subfolder of /usr/share.
This is just food for thought, as I do not have a whole understanding on how it is loaded and other possible complications.

@Ambrevar
Copy link
Member

libnyxt.so is loaded by webkitgtk, we have no control on how its done (unless there is some environment variable to control this, but then it's probably up to the packager to handle this.

@bubbleguuum
Copy link
Contributor Author

But how webkitgtk (assuming the .so library installed on the system and supposedly dynamically loaded by the nyxt executable) can find libnyxt.so, if nyxt does not tell it (thus control) where to find it ?

@Ambrevar
Copy link
Member

Oops! You got me there, I wasn't thinking clearly!

You are right, it's us who are in full control of the library location. Here is the relevant code (from renderer/gtk.lisp):

(define-class gtk-extensions-directory (nyxt-file)
  ((files:name "gtk-extensions"))
  (:export-class-name-p t)
  (:accessor-name-transformer (class*:make-name-transformer name))
  (:documentation "Directory where to load the 'libnyxt' library.
By default it is found in the source directory."))

(defmethod files:resolve ((profile nyxt-profile) (file gtk-extensions-directory))
  (asdf:system-relative-pathname :nyxt "libraries/web-extensions/"))

So what we could do is check multiple folders instead of just there above:

  • /lib
  • /lib64
  • etc.

But maybe that's overly complicated. Alternatively, we could add a knob in the makefile / nyxt.asd, like NYXT_LIB_PATH, defaulting to (asdf:system-relative-pathname :nyxt "libraries/web-extensions/") and which the packager can customize

@Ambrevar
Copy link
Member

@bubbleguuum Can you test #2371 see if it fixes the source location issue on your side?

@Ambrevar
Copy link
Member

Ambrevar commented Jun 15, 2022

To do after #2371 is merged:

  • Discard .c and .h files.
  • Store libnyxt.so to a more standard location (what's a good default?).
  • Make libnyxt.so executable.
  • Fix function-lambda-string or describe-* to not completely fail when the source location is missing.

@Ambrevar
Copy link
Member

@aartaka Does webkit:webkit-web-context-set-web-extensions-directory try to load all libraries and then call webkit_web_extension_initialize?

If so, then I guess we should not set the extension dir to /usr/lib or the like, and thus keep the library in its own private dir?

Is it fine to store it in /usr/lib/nyxt for instance/

@bubbleguuum
Copy link
Contributor Author

bubbleguuum commented Jun 20, 2022

I also think the lib should go into its own directory (ie /usr/lib/nyxt). The /usr/lib dir is nornally for 32-bit libraries while /usr/lib64 is for 64-bit.
However, /usr/libexec/nyxt might be a better candidate for the location. On my system, I have 2 packages that put their private libs that they dlopen in /usr/libexec/<packagename>: sudo and valgrind.

@aartaka
Copy link
Contributor

aartaka commented Jun 20, 2022

@aartaka Does webkit:webkit-web-context-set-web-extensions-directory try to load all libraries and then call webkit_web_extension_initialize?

The direct effect of this function is adding the directory to WebKit sandbox, no more. But I am not certain about what happens next—it most probably calls dlopen on the files in this directory, so putting anything by extension libraries there is a dangerous business.

If so, then I guess we should not set the extension dir to /usr/lib or the like, and thus keep the library in its own private dir?

Is it fine to store it in /usr/lib/nyxt for instance/

Yes, that should be the best option—it should also resolve the #1781 gratiously.

@Ambrevar
Copy link
Member

OK, but that would still not answer the question for Guix and Nix that do not have /usr/lib.

@Ambrevar
Copy link
Member

Done with 4615482.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3-series Related to releases whose major version is 3. build high
Development

No branches or pull requests

3 participants