openrazer / OBS-packaging

Open Build Service packaging for OpenRazer
https://build.opensuse.org/package/show/hardware:razer/openrazer
5 stars 8 forks source link

spec: Iterate over installed kernels #9

Open Sid127 opened 3 months ago

Sid127 commented 3 months ago

As mentioned on https://github.com/openrazer/openrazer/issues/2177

Makes the post transaction hook use rpm to get kernel versions if rpm is available (like on fedora based distros) and falls back to reading dir names in /lib/modules to get the appropriate kernel version string if rpm is not available (like on openSUSE based distros)

z3ntu commented 3 months ago

Thanks for working on this!

falls back to reading dir names in /lib/modules to get the appropriate kernel version string if rpm is not available (like on openSUSE based distros)

Isn't openSUSE also an rpm-based distro? I mean we're building rpms here. How is rpm not available then?

z3ntu commented 3 months ago

Also do you think it'd be a disadvantage to always use the /lib/modules method? Would be one less code path to maintain

Sid127 commented 3 months ago

Isn't openSUSE also an rpm-based distro? I mean we're building rpms here. How is rpm not available then?

AFAIK rpm isn't available on openSUSE, they use zypper as their package manager. I could be wrong, however. I'll check and update accordingly

do you think it'd be a disadvantage to always use the /lib/modules method

from what I've noticed, the purge-kernels thing/equivalent on fedora doesn't clean up the dir if you were using a dkms/akmods out of tree kernel module, like the nvidia kernel module for example. Using the package manager to query installed kernels ensures the respective dirs are actually populated, though I imagine it should be fine to just read /lib/modules anyway since dkms will quietly fail on invalid versions without breaking the for loop

Sid127 commented 3 months ago

Updated to use the /lib/modules method regardless.

z3ntu commented 3 months ago

AFAIK rpm isn't available on openSUSE, they use zypper as their package manager. I could be wrong, however. I'll check and update accordingly

Without knowing more about it (or checking), Fedora is also using dnf as user-facing tool, similar to openSUSE's zypper. But again, I'm not very familiar with these distros :)

since dkms will quietly fail on invalid versions without breaking the for loop

$ sudo dkms install openrazer-driver/3.8.0 -k foobar
Error! Your kernel headers for kernel foobar cannot be found at /usr/lib/modules/foobar/build or /usr/lib/modules/foobar/source.
Please install the linux-headers-foobar package or use the --kernelsourcedir option to tell DKMS where it's located.
$ echo $?
1

Doesn't seem to fail quietly for me? Of course we could add a || true at the end to continue anyways and it'd also tell the user that something's a bit wrong on their setup that some old kernel directories are floating around.

Sid127 commented 3 months ago

I don't think adding || true would be the best thing to do as it'd remove the error message that the OP of #2177 had, prompting them to install the kernel header packages

EDIT: turns out rpm is available on all rpm based distros, but they have different package name formats. Using the /lib/modules path would be the easiest.

EDIT EDIT: alternatively we could switch to using akmods instead of dkms, which apparently handles this case out of the box

z3ntu commented 3 months ago

For akmod see https://github.com/openrazer/openrazer/issues/1747, I don't know much about that either but I don't think we can use just that in the short-term.

Sid127 commented 3 months ago

Valid, I don't know much here either ^^'

I myself use Arch, and I just am reporting issues for friends who switched from Windows as they're not yet used to the whole issue reporting cycle. Always glad to help, but this is for a distro and software I myself don't use :P

I do think the current state of this PR should be fine on most systems, however. It will complain about missing headers for dirs that don't contain sources, yes, but it will also install correctly for every other kernel, making the error mostly ignorable for most users.

z3ntu commented 3 months ago

I don't think adding || true would be the best thing to do as it'd remove the error message that the OP of #2177 had, prompting them to install the kernel header packages

I don't think this would change anything there. Just for the post install script it would not fail completely with that, not sure what rpm does if a post-install script exits with exit code 1... And || true doesn't silence any errors, they still get printed, just sort of ignored :P