Crash after theme selection

[Emuzex]

After install i started gittyup, then i picked the dark theme. The program crashed instantly and now doesnt start.
Installed via yay

Console output after start gittup

Theme dir: QDir ("/home/emuzex/.cache/yay/gittyup/src/gittyup/conf", nameFilters = {""}, QDir :: SortFlags (Name | IgnoreCase), QDir :: Filters (Dirs | Files | Drives | AllEntries))
Theme dir: QDir ("/home/emuzex/.cache/yay/gittyup/src/gittyup/conf", nameFilters = {""}, QDir :: SortFlags (Name | IgnoreCase), QDir :: Filters (Dirs | Files | Drives | AllEntries))
PANIC: unprotected error in call to Lua API (cannot open /home/emuzex/.cache/yay/gittyup/src/gittyup/conf/System.lua: No such file or directory)
zsh: IOT instruction (core dumped) gittyup

System:
OS: Manjaro Linux x86_64
Host: ASUS TUF Gaming A15 FA506IU_FA506IU 1.0
Kernel: 5.15.38-1-MANJARO
CPU: AMD Ryzen 9 4900H with Radeon Graphics (16) @ 3.300GHz
GPU: NVIDIA GeForce GTX 1660 Ti Mobile
GPU: AMD ATI 06:00.0 Renoir
Memory: 31582MiB

[Murmele]

Hi,

Currently the aur package is provided by someone outside of the project.

The error was reported also here: https://aur.archlinux.org/packages/gittyup

Officially we support only the flatpak package: https://flathub.org/apps/details/com.github.Murmele.Gittyup

[Emuzex]

Thank u for fast response.

[alerque]

I am the AUR packager. I'm happy to fix the packaging, but I believe this is actually a legitimate upstream issue and needs fixing here in the project.

Somewhere in the build system (cmake + ninja build) an assumption is being made that a full file path as detected during build will be the same after install (ninja install). This is a bogus assumption that will not be true for many/most packaging systems.

[Murmele]

Hi @alerque, good to know. I'm working currently on using libgit2 upstream instead of a custom one. So I will notify you as soon as this will be merged (#153).

I will check out where the lua path is set wrongly

[alerque]

Thanks. I'll keep an eye out and test, and if I get a chance try to root out the issue myself. Also if I can't find the source I might temporarily monkey patch the installed package files by search/replacing the build path to the permanent one—at least until this is fixed for all builds by default. Note that similar issues have come up with some Lua Rocks packages that try to detect their own location and then hard code it into themselves. Just to say you may be looking for something even upstream in a Lua package.

(Also great to hear that you are moving towards the official libgit2 upstream!)

[alerque]

Just a follow up some initial searching suggests this isn't something that can be monkey patched because the faulty path isn't anything in plain text in Lua files it is in a binary somewhere.

[ReubenM]

@alerque I believe you could monkey patch the conf path set in src/conf/CMakeLists.txt line 13

Gittyup/src/conf/CMakeLists.txt

Line 13 in f38eb16

target_compile_definitions(conf PRIVATE CONF_DIR="${CMAKE_SOURCE_DIR}/conf"

[alerque]

@ReubenM Interesting find. Definitely a bug in the build if CMAKE_SOURCE_DIR is ending up baked into the final installation binaries, but yes we might be able to patch that before build. Did you try it by the way? It's possible that will break other things during the build process.

[ReubenM]

@alerque Haven't tried building it patched yet, but that appears to be the source of the issue

[alerque]

I tried patching that line and still don't get a working build. I do get a build, but if the build time source paths are not available at run time (as is the case for an installed program) it still crashes

prepare() {
  sed -i -e '/CONF_DIR/{s!SOURCE!BINARY!;s!/conf!!}' src/conf/CMakeLists.txt
}

[Murmele]

@alerque maybe you can have a look through the FLATPAK if's, maybe you find something there

Destination is set to: Resources

[Murmele]

@Emuzex Can you have a look into /usr/lib/gittyup
Is there any Resources folder? And maybe the lua plugins

[Murmele]

sed -i -e '/CONF_DIR/{s!SOURCE!BINARY!;s!/conf!!}' src/conf/CMakeLists.txt

Can you try to set it to {CMAKE_BINARY_DIR}/Resources
and maybe also the L10N_DIR as it is done for the flatpak package?

It uses those definitions only if Resources is not found.

[alerque]

Edit: Scratch the first part of my earlier comment here. Having recompiled this a few times the error is still showing up with the /build/... path, not whatever I change the CONF_DIR value to be so we're not ever hitting that variable's value.

---

Having a look at where the System.lua file actually is, it looks like it is in .../Resources/themes/.

$ pacman -Ql gittyup | grep System.lua
gittyup /usr/lib/gittyup/Resources/themes/System.lua

I could keep trying to patch it, but I think something is seriously off about this path handling. A variable named "CONF_DIR" shouldn't have a hard coded path to a theme directory. Lua includes shouldn't request just require('System') but require('themes.system'), while also having added a pattern for the app's resource dir to the lua package.path.

[Murmele]

$ pacman -Ql gittyup | grep System.lua
gittyup /usr/lib/gittyup/Resources/themes/System.lua

The binary is in /usr/lib/gittyup or?

[alerque]

The binary is installed as /usr/bin/gittyup, per FHS and Arch packaging guidelines.

[Murmele]

Ah ok you are moving the binary:
install -Dm0755 "$_bin" "$pkgdir/usr/bin/$pkgname"

That means cd("Resources") will not work anymore. Then it will be searched in CONF_DIR. Can you try to copy the Resources folder manually to the correct place?

[Murmele]

What is the correct location those configuration files? /usr/share?

[alerque]

Yes, I think ideally the resources should end up in /usr/share/gittyup/, perhaps even without the 'Resources' segment of the path. At the moment the installer puts them in /usr/lib/gittyup/Resources along with the other stuff (the executables, plugins, lib directory, etc should probably stay in /usr/lib;; the include directory also in there should almost certainly either not be packaged or moved to /usr/include).

[Murmele]

I can introduce a DATA_INSTALL_DIR variable and use it instead of the CMAKE_SOURCE_DIR?

@exactly-one-kas maybe you have also an idea

[alerque]

DATA_INSTALL_DIR sounds like a much better plan than CMAKE_SOURCE_DIR!

[alerque]

Looking at other vars, maybe CMAKE_INSTALL_DATADIR would be a better naming scheme (see existing CMAKE_INSTALL_PREFIX and CMAKE_INSTALL_MANDIR).

[alerque]

...on second thought, the CMAKE_INSTALL_PREFIX is almost certainly a heterodox usage, that should be just /usr instead of /usr/lib/gittyup like it needs to be right now. And then things like the MANDIR (and proposed DATADIR) should probably use the prefix plus their own bit of path. At least that's how these vars would be expected to behave in autotools builds, I'm not sure about the cmake idiomatic usage.

[Murmele]

I created a pull request. Let's discuss everything there

[Murmele]

I think we can close this, because this is related to #217