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

spec: Iterate over installed kernels #9

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Sid127
Copy link

@Sid127 Sid127 commented Jul 3, 2024

As mentioned on openrazer/openrazer#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)

@Sid127 Sid127 changed the title spec: Iterate over installed kernels on Fedora spec: Iterate over installed kernels Jul 3, 2024
@z3ntu
Copy link
Member

z3ntu commented Jul 3, 2024

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?

openrazer.spec Outdated Show resolved Hide resolved
@z3ntu
Copy link
Member

z3ntu commented Jul 3, 2024

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
Copy link
Author

Sid127 commented Jul 3, 2024

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
Copy link
Author

Sid127 commented Jul 3, 2024

Updated to use the /lib/modules method regardless.

@z3ntu
Copy link
Member

z3ntu commented Jul 3, 2024

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
Copy link
Author

Sid127 commented Jul 3, 2024

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
Copy link
Member

z3ntu commented Jul 3, 2024

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

@Sid127
Copy link
Author

Sid127 commented Jul 3, 2024

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
Copy link
Member

z3ntu commented Jul 3, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants