Overview
Request 886524 superseded
Add new package openSUSE-signkey-cert (bsc#1182641)
- Created by joeyli
- In state superseded
- Package maintainer: joeyli
- Supersedes 883721
- Superseded by 888962
Why should this live in Base:System at all? If it's for kernel-modules, why not have it live there?
Thanks for review!
We want to install signkey package by pattern. So I created a independent package to help user to enroll key to MOK. It's only for openSUSE KMP on OBS.
- The check in
%build
shouldn't be needed. If the file is missing, it's going to break anyway - Calling rpm in scriptlets might deadlock
- Scriptlets are also called on upgrades, but then most of them shouldn't do anything unless the file changes
- Most of the scriptlets can be simplified by moving them inside
if command -v mokutil
Thanks for your review!
. The check in %build shouldn't be needed. If the file is missing, it's going to break anyway
OK! I will remove the check.
. Calling rpm in scriptlets might deadlock
I will use "ls" to grab the certificate file name.
. Scriptlets are also called on upgrades, but then most of them shouldn't do anything unless the file changes
This RPM will not be rebuilt unless the openSUSE signkey be changed on OBS. When this RPM be upgraded, it means that the openSUSE signkey also be updated.
. Most of the scriptlets can be simplified by moving them inside if command -v mokutil
OK, I will move them inside ' if command -v mokutil '.
This RPM will not be rebuilt unless the openSUSE signkey be changed on OBS. When this RPM be upgraded, it means that the openSUSE signkey also be updated.
There will be forced rebuilds without source changes, so the package has to take that into account.
Please name the package openSUSE-...
Check: rpmqpack |grep -i opensuse
The scriptlets don't take package upgrades/downgrades into account. Those would delete all certificates instead, because
%postun
of the old package is run last: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#orderingI have built two version of RPMs for testing upgrades/downgrades by 'rpm -Uvh ' and 'rpm -Uvh --oldpackage'. I didn't see problem.
The %postun will remove xxx-kmp.crt.del file if the corresponding xxx-kmp.crt is still exists after uninstallation. So it only removes remaining *-kmp.crt.del certificate by mokutils.
Ok. My main confusion with the scripts is that they try to work with multiple files inside the directory, even though there's not actually more than one file in this package and not more than one package planned right now.
AFAICT it's possible to just call mokutil from
%preun
on the final uninstallation, if the scriptlet knows about the fingerprint, but that would need extra logic to deal with detection of fingerprint changes. I don't really have a better idea for this particular case. The cleanest way would be a package with trigger scripts which can tell whether a certificate is about to be removed, replaced or added, but that's just overengineering.Is it necessary to postpone the call to
mokutil --delete
to%postun
or can it be done in%preun
instead? Then it wouldn't be needed to do "backups" ($cert.del
)Can not. Please see my the above comment.
That does not actually run
command -v
, it checks whether the constant string is non-empty. Also, this uses a bashism, so try this instead:Thanks for your review! I will fix it in next version.
Is it necessary to check for the existance of
%{_sysconfdir}/uefi/certs/
? It's owned by the package, so except for the final uninstallation it'll always be present.I will try to remove it. As I remember that I add the check because I got warning messages in OBS log. I will remove the check to see if there have any warning.
Thanks!
I have tested after removed the check for the existance of %{_sysconfdir}/uefi/certs/. Looks no problem. I will remove in next version.
@WernerFink, @a_jaeger, @dirkmueller, @dmolkentin, @msmeissn, @oertel, @seife, @trenn: review reminder
I have sent new version request number: 888962