enkore / j4-dmenu-desktop

A fast desktop menu
GNU General Public License v3.0
683 stars 69 forks source link

Enhancment request, Crash on arch due to spdlog/fmt #180

Open SFort opened 1 month ago

SFort commented 1 month ago

I recognize this is no fault of yours. Arch currently packages fmt11 and spdlog which is looking for fmt10. this is entierly an arch issue, but anychance you could not crash if the logger doesn't load? it's very annoying having the primary method of opening apps be broken because package maintainers goof.

meator commented 1 month ago

What's the problem? I've tried to run j4-dmenu-desktop in an Archlinux docker container and it seems to execute properly.

I am not an Archlinux user, so my comments should be taken with a grain of salt, but this looks like a result of a partial upgrade. Does pacman -Syu fix the issue for you?

If you do not trust your shared libraries, you can build j4-dmenu-desktop's dependencies statically. The easiest way to do this is by building j4-dmenu-desktop as follows:

meson setup --buildtype=release -Db_lto=true --force-fallback-for=spdlog,fmt build
# This downloads and links deps manually:    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# And then continue with normal Meson build steps documented in README...

If you happen to have static libraries of spdlog or fmt at hand, you can instruct Meson to use them[^1] using Meson's standard flags (-Dprefer_static=true, -Ddefault_library=static...).

Similar results should be achievable with CMake, but I don't know its flags for this of the top of my head.

The solution you are proposing (continue functioning if there is an error while loading shared libraries) would be pretty difficult to implement and fairly nonstandard. It is reasonable to expect the program to fail if there are problems with shared libraries. Deviating from this behavior could surprise users.

[^1]: This works only if Meson can find the static libraries.

SFort commented 1 month ago

i did a full upgrade again and i'm still getting j4-dmenu-desktop: error while loading shared libraries: libfmt.so.10: cannot open shared object file: No such file or directory the distro has a reputation to break and it isn't undeserved. (i wish i could just staticly build everything in arch, without dynamic deps, because the repos in arch are a mess dep wise)

crashing which was my motivation aside, i also belive that deps should be optional whenever possible. but your right, quietly disabling the logger is an issue. what about a --no-logger param, or --ignore-missing-logger?

although if this is mountains of work, feel free to close.

SFort commented 1 month ago

The solution you are proposing (continue functioning if there is an error while loading shared libraries) would be pretty difficult to implement and fairly nonstandard

i'd just like a way to not require the dep, i don't actually need it to catch errors. dependency hell is something i've been in for too long, so i have a tendency to prefer not having them at all, regardless of lost functionallity. (and this is the sole issue i've had with j4-dmenu-desktop and it isn't even it's fault. so i don't think i'd miss the logger)

meator commented 1 month ago

I think that there are two choices made by j4-dmenu-desktop that are related to this: spdlog and fmt are required dependencies and a shared library dependencies are required in runtime. Let's talk about both of these decisions.

fmt and spdlog are required dependencies

One of the primary changes I have made after I was given the commit bit was adding these two dependencies. This is a pretty major change, because j4-dmenu-desktop depended only on Catch2 and standard C++ libraries[^libstdcpp] before I joined. Catch2 is only needed during build if you want to run unit tests and standard C++ library is installed on pretty much every system under the sun, so j4-dmenu-desktop was "dependencyless".

I have made the choice to add two dependencies when I joined. This wasn't a simple decision to make, I understand the advantages of not having any dependencies and I didn't want to add useless dependencies, but I believe that spdlog is worth it. Having proper logging is essential for a program like j4-dmenu-desktop.

Logging

I have rewritten large chunks of the codebase (some of them several times). Because of this, I expected a lot of new bugs to arise (although I've tried to prevent that). I have anticipated that I'll need a reliable way to diagnose issues on systems I don't have access to. The simplest way to do so is to have good logging, which will point me to the part of the code which is failing.

After I implemented this change (which was requested in #87), I have noticed that it can be quite useful for regular users too. The logging I've implemented clearly shows which desktop files were loaded, which directories were searched and under what name can a desktop file be reached. This is a very good resource for people who want to figure out why their desktop file isn't displayed.

fmt

fmt is a dependency of spdlog, so every system which has spdlog installed will have fmt available too. This is why I've chosen to add fmt as a dependency to j4-dmenu-desktop. It technically adds a second dependency, but it doesn't in practise.

The decision

j4-dmenu-desktop currently places a hard requirement on both dependencies. I could make that optional during build. This would be easy to implement and not controversial. The important decision is whether they should be required by default. Let's consider options:

  1. Make dependencies opt out (required by default, but add a flag to disable these dependencies)
    • This wouldn't be that different from the current approach really.
    • Distributions will most likely not opt out of these dependencies (they'll stick to the defaults), so most of j4-dmenu-desktop's user base, who install j4-dmenu-desktop through a package manager, will still need spdlog and fmt (but see the section below).
    • This would only make sense for users like you @SFort which do not like shared libraries. If people care enough to disable these dependencies, it shouldn't be that hard for them to build dependencies statically as I described above.
    • This means that this would make no difference for the regular user base and it would make no difference for people like you because you can build statically.
  2. Make dependencies opt in (disabled by default, but add a flag to enable these dependencies)
    • Most of j4-dmenu-desktop's user base, who install j4-dmenu-desktop through a package manager, will loose logging support. This isn't user friendly. You may not need logging, but that doesn't mean that there isn't any Archlinux user who would find that useful.
    • Distro packagers could choose to explicitly enable support for logging, but I think that most distros will stick to defaults.
    • This behavior differs from what most projects do. Being "unique" isn't an advantage, people will expect j4-dmenu-desktop to work like other packages.
  3. Enable dependencies if they are available, don't enable them if they aren't available and add a flag to override this system either way.
    • This is a "neutral" solution.
    • This is a compromise, but not a good one. It will not be immediately apparent if the dependency is or isn't required. I think that this is worse than disabling the dependencies by default.
    • This behavior differs from what most projects do. Being "unique" isn't an advantage, people will expect j4-dmenu-desktop to work like other packages.

None of these options are convincing.

Requiring shared libraries

This is more related to your problem. Options are

  1. Require shared libraries when built with them during runtime
    • This is the current behavior
    • This is how all other programs behave.
    • This is how users expect j4-dmenu-desktop to behave.
    • An error will show you that something is wrong with your installation. If this error was silent, the user will most likely not notice it, even though the user might want to know that their shared libraries are broken.
  2. Try to load dynamic libraries, but don't require it if it can't be loaded during runtime
    • This is not how normal programs behave.
    • This is not how users expect j4-dmenu-desktop to behave.
    • Users who want logging will be pretty surprised when they find out that it doesn't work. Some users use j4-dmenu-desktop as a daemon and set up a log file, so they don't execute j4-dmenu-desktop directly. They would discover that logging doesn't work when they'll need the logs and won't find them.
    • I think that this is technically possible to implement, but it is very nonstandard. I have never done anything like this, I currently don't know how to do that (but I could probably figure it out if I really wanted to, I think this has something to do with dlopen() and similar functions).

You suggested adding a --no-logger flag. This cannot be done with the current system, because j4-dmenu-desktop will depend on spdlog shared library even if j4-dmenu-desktop never calls a single function defined in spdlog shared library. Shared libraries are resolved before j4-dmenu-desktop is executed, it can't choose to not load it if it doesn't want to.

--no-logger can only work with the second solution I've described (Try to load dynamic libraries, but don't require it if it can't be loaded). It therefore shares all negatives with this solution. Even requiring shared libraries by default and providing a flag to turn that off at runtime would be comparably difficult to implement.

I assume that you don't want logging because you don't want to deal with shared library mess. If you simply dislike logging and don't care about shared libraries, you can use --log-level ERROR to only log the highest priority log messages and not print any other ones. Or you can 2> /dev/null. Both of these solutions are pretty nonsensical though.

[^libstdcpp]: libstdc++ on most Linux systems.

meator commented 1 month ago

Also, you appear to be aware of this, but I should reiterate that I hold no responsibility over Archlinux's spdlog and fmt packages. It is convenient to use your distribution native package manager, but if you do not trust it, you are always free to compile j4-dmenu-desktop manually using custom dynamic libraries or linking statically.

By using your distribution's package, you are putting your trust into its maintainer's hands. They are responsible for that package on their distribution. You should probably report this to Archlinux.

meator commented 1 month ago

I have looked this up and the "Try to load dynamic libraries, but don't require it if it can't be loaded during runtime " solution shouldn't be that hard to implement: https://stackoverflow.com/q/8610083

SFort commented 1 month ago

I have looked this up and the "Try to load dynamic libraries, but don't require it if it can't be loaded during runtime " solution shouldn't be that hard to implement: https://stackoverflow.com/q/8610083

i mean a lot of programs have optional deps that are checked at runtime i don't find this a fringe situation. obs for example.

i'm also happy to reiterate this is entierly a badly managed package repo with loosely defined deps and nothing at fault with the project.

i'd just like the program to be robust against badly managed package repos if possible, and i also understand that silent failiure being a default behaviour is bad.

i'd still like to have the simplicity of using the package provided by the default repo. without the possibility of booting my system and only having terminal access and i trust j4-dmenu-desktop to remain functional enough to not have logging for it.

You should probably report this to Archlinux

i don't like the arch maintainers, they have repeatedly caused issue and been very loose with defining deps by choice. they'd probably fix this at least, don't trust them to keep it fixed though. i just like pacman, if i knew of a good repo maintainer for it i'd switch.

again thought you can just not deal with this, that's fine.

solution shouldn't be that hard to implement

but if you've got an easy way to add a robust mode i'd certanly be apriciated