-
Notifications
You must be signed in to change notification settings - Fork 6
Add dkms-install and dkms-remove hooks #4
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
base: master
Are you sure you want to change the base?
Conversation
|
The shebangs should be |
|
Listing packages was suboptimal indeed, but the problem with file triggers in my first attempt was that there were a lot of them and the tests took quite a long time. I managed to drastically cut down the time by rearranging the tests, it's quite fast now. Also changed the shebang, thanks! About the scripts path, pacman comes with /usr/share/libalpm rather than /usr/share/alpm, shouldn't the hooks.bin directory reside inside the former? |
|
We can make it even faster. Trigger targets can be negated, so by using we can let pacman filter out all the extraneous paths instead of having the script do it. That should let you remove the outer two Please quote all variables; I'm not sure which, if any, can actually contain spaces in this case, but I prefer it for consistency. Does dkms not provide a better way to determine if a module is installed? If not, I think I'd prefer it written as Does the remove hook still need to trigger on upgrades? Now that we're using a File trigger I would think it would only need to be run on Remove.
Probably. I'm still not entirely satisfied with my choices for those paths. Stick with the existing paths for now and I'll look at changing them after this gets merged. |
|
Negative targets, thx for the elegant solution! dkms' output doesn't seem to be translated, and there's no other way to check for modules beyond the status command. But you're right it doesn't harm to let it print an error (plus exit code 3) if the module doesn't exist, plus it's possible to do it in one line that way. That being said, would you like for the output to be redirected to /dev/null, there's quite a lot of it actually (plus the annoying broken pipe messages that have occured for a couple years with no fix on the horizon). There is no need for the remove hook to be run on upgrades indeed, and as a bonus it doesn't remove modules if you reinstall the same kernel or headers. All in all, the script is now very much simpler, thx! |
|
I put this on hold for a while to see what happened with Arch's dkms hook. Do you still think this is good to be merged as-is? |
|
The dkms hook covers this and more, I believe this can be dropped and the PR can be closed. |
These are generic dkms hooks to install/remove modules upon kernel install/upgrade/remove. Individual dkms packages should still have a hook or install script to handle this when they get installed/removed.