| Message ID | 20241203065441.2341579-2-josch@mister-muffin.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | scripts/package/builddeb: allow hooks also in /usr/share/kernel | expand |
On Tue, Dec 3, 2024 at 3:55 PM Johannes Schauer Marin Rodrigues <josch@mister-muffin.de> wrote: > > By passing an additional directory to run-parts, allow Debian and its > derivatives to ship maintainer scripts in /usr while at the same time > allowing the local admin to override or disable them by placing hooks of > the same name in /etc. This adds support for the mechanism described in > the UAPI Configuration Files Specification for kernel hooks. The same > idea is also used by udev, systemd or modprobe for their config files. > https://uapi-group.org/specifications/specs/configuration_files_specification/ > > This functionality relies on run-parts 5.21 or later. It is the > responsibility of packages installing hooks into /usr/share/kernel to > also declare a Depends: debianutils (>= 5.21). > > KDEB_HOOKDIR can be used to change the list of directories that is > searched. By default, /etc/kernel and /usr/share/kernel are hook > directories. > > Signed-off-by: Johannes Schauer Marin Rodrigues <josch@mister-muffin.de> > --- > scripts/package/builddeb | 33 +++++++++++++++++++++++++-------- > 1 file changed, 25 insertions(+), 8 deletions(-) > > diff --git a/scripts/package/builddeb b/scripts/package/builddeb > index 441b0bb66e0d..6e83f6f3ec6d 100755 > --- a/scripts/package/builddeb > +++ b/scripts/package/builddeb > @@ -5,10 +5,12 @@ > # > # Simple script to generate a deb package for a Linux kernel. All the > # complexity of what to do with a kernel after it is installed or removed > -# is left to other scripts and packages: they can install scripts in the > -# /etc/kernel/{pre,post}{inst,rm}.d/ directories (or an alternative location > -# specified in KDEB_HOOKDIR) that will be called on package install and > -# removal. > +# is left to other scripts and packages. Scripts can be placed into the > +# preinst, postinst, prerm and postrm directories in /etc/kernel or > +# /usr/share/kernel. A different list of search directories can be given > +# via KDEB_HOOKDIR. Scripts in directories earlier in the list will > +# override scripts of the same name in later directories. The script will > +# be called on package installation and removal. > > set -eu > > @@ -68,11 +70,18 @@ install_linux_image () { > # kernel packages, as well as kernel packages built using make-kpkg. > # make-kpkg sets $INITRD to indicate whether an initramfs is wanted, and > # so do we; recent versions of dracut and initramfs-tools will obey this. > - debhookdir=${KDEB_HOOKDIR:-/etc/kernel} > + debhookdir=${KDEB_HOOKDIR:-/etc/kernel /usr/share/kernel} > + > + # Only pre-create the first hook directory. Support for more than one hook > + # directory requires run-parts 5.21 and it is the responsibility of packages > + # creating additional hook directories to declare that dependency. > + firsthookdir=${debhookdir%% *} > for script in postinst postrm preinst prerm; do > - mkdir -p "${pdir}${debhookdir}/${script}.d" > + mkdir -p "${pdir}${firsthookdir}/${script}.d" I still do not understand why this 'mkdir' is needed. linux-image package does not install any script into the hook directory. In general, there exist some scripts (e.g. initramfs-tools) already under /etc/kernel/*.d/ (and under /usr/share/kernel/*.d/ once the new location is used broader). If nothing exists under the hook directory, there is no point to execute run-parts. > + done > > - mkdir -p "${pdir}/DEBIAN" > + mkdir -p "${pdir}/DEBIAN" Please drop this noise change. If you want to optimize this, please split it into a separate patch like "kbuild: deb-pkg: create DEBIAN directory just once" etc. > + for script in postinst postrm preinst prerm; do > cat <<-EOF > "${pdir}/DEBIAN/${script}" > #!/bin/sh > > @@ -84,7 +93,15 @@ install_linux_image () { > # Tell initramfs builder whether it's wanted > export INITRD=$(if_enabled_echo CONFIG_BLK_DEV_INITRD Yes No) > > - test -d ${debhookdir}/${script}.d && run-parts --arg="${KERNELRELEASE}" --arg="/${installed_image_path}" ${debhookdir}/${script}.d > + # run-parts will error out if one of its directory arguments does not > + # exist, so filter the list of hook directories accordingly. > + hookdirs= > + for dir in ${debhookdir}; do > + test -d "\$dir/${script}.d" || continue > + hookdirs="\$hookdirs \$dir/${script}.d" > + done > + hookdirs="\${hookdirs# }" > + test -n "\$hookdirs" && run-parts --arg="${KERNELRELEASE}" --arg="/${installed_image_path}" \$hookdirs > exit 0 > EOF > chmod 755 "${pdir}/DEBIAN/${script}" > -- > 2.39.2 > > -- Best Regards Masahiro Yamada
Quoting Masahiro Yamada (2024-12-03 10:27:11) > > @@ -68,11 +70,18 @@ install_linux_image () { > > # kernel packages, as well as kernel packages built using make-kpkg. > > # make-kpkg sets $INITRD to indicate whether an initramfs is wanted, and > > # so do we; recent versions of dracut and initramfs-tools will obey this. > > - debhookdir=${KDEB_HOOKDIR:-/etc/kernel} > > + debhookdir=${KDEB_HOOKDIR:-/etc/kernel /usr/share/kernel} > > + > > + # Only pre-create the first hook directory. Support for more than one hook > > + # directory requires run-parts 5.21 and it is the responsibility of packages > > + # creating additional hook directories to declare that dependency. > > + firsthookdir=${debhookdir%% *} > > for script in postinst postrm preinst prerm; do > > - mkdir -p "${pdir}${debhookdir}/${script}.d" > > + mkdir -p "${pdir}${firsthookdir}/${script}.d" > > I still do not understand why this 'mkdir' is needed. > > linux-image package does not install any script into the hook directory. > In general, there exist some scripts (e.g. initramfs-tools) already > under /etc/kernel/*.d/ (and under /usr/share/kernel/*.d/ once the > new location is used broader). > If nothing exists under the hook directory, there is no point to > execute run-parts. Unless I'm misunderstanding the old code, the existing script *does* create /etc/kernel/*.d/ (That's the "- mkdir -p" line above) so I wanted to preserve this behaviour even with more than one directory in KDEB_HOOKDIR. Do I misunderstand something? Are you saying that with this change, no /etc/kernel/*.d/ should be created anymore? Why? > > + done > > > > - mkdir -p "${pdir}/DEBIAN" > > + mkdir -p "${pdir}/DEBIAN" > > Please drop this noise change. > > If you want to optimize this, please split it into a separate patch like > "kbuild: deb-pkg: create DEBIAN directory just once" etc. Okay, no need to optimize this now. mkdir -p is cheap. Thanks! cheers, josch
On Tue, Dec 3, 2024 at 6:41 PM Johannes Schauer Marin Rodrigues <josch@mister-muffin.de> wrote: > > Quoting Masahiro Yamada (2024-12-03 10:27:11) > > > @@ -68,11 +70,18 @@ install_linux_image () { > > > # kernel packages, as well as kernel packages built using make-kpkg. > > > # make-kpkg sets $INITRD to indicate whether an initramfs is wanted, and > > > # so do we; recent versions of dracut and initramfs-tools will obey this. > > > - debhookdir=${KDEB_HOOKDIR:-/etc/kernel} > > > + debhookdir=${KDEB_HOOKDIR:-/etc/kernel /usr/share/kernel} > > > + > > > + # Only pre-create the first hook directory. Support for more than one hook > > > + # directory requires run-parts 5.21 and it is the responsibility of packages > > > + # creating additional hook directories to declare that dependency. > > > + firsthookdir=${debhookdir%% *} > > > for script in postinst postrm preinst prerm; do > > > - mkdir -p "${pdir}${debhookdir}/${script}.d" > > > + mkdir -p "${pdir}${firsthookdir}/${script}.d" > > > > I still do not understand why this 'mkdir' is needed. > > > > linux-image package does not install any script into the hook directory. > > In general, there exist some scripts (e.g. initramfs-tools) already > > under /etc/kernel/*.d/ (and under /usr/share/kernel/*.d/ once the > > new location is used broader). > > If nothing exists under the hook directory, there is no point to > > execute run-parts. > > Unless I'm misunderstanding the old code, the existing script *does* create > /etc/kernel/*.d/ (That's the "- mkdir -p" line above) so I wanted to preserve > this behaviour even with more than one directory in KDEB_HOOKDIR. Do I > misunderstand something? Right. The existing code does create empty directories, which are unnecessary. > Are you saying that with this change, no > /etc/kernel/*.d/ should be created anymore? Why? The 'mkdir' is unnecessary regardless of your patch, unless I am misunderstanding the code. At present, it is a single line of code. You are extending it into several lines of verbose code simply in order to precisely retain these unnecessary directories. This is a problem for me because I will need to maintain code whose necessity is unclear. Judging from your cautious approach and verbose changes, I assume you are trying to avoid any risks (a common trait among many contributors). That said, I understand you are not motivated to strive for clean code at all costs. Once you commit the run-parts changes, you may feel your work is done. However, as the maintainer, I must manage this code for many years, so I aim to proactively remove unneeded code. I have decided to take responsibility for cleaning up this single line myself: https://lore.kernel.org/linux-kbuild/CAK7LNARU=M282fAOOgzPOBPtDNFPjH8To9eK2vYstWxkEDEEPA@mail.gmail.com/T/#t If something breaks due to missing directories, it will be my fault, not yours. Now that the dummy directories are gone from the linux-image package, please prepare the next version without the "pre-create the first hook directory" stuff. A few more requests. Please add the version number (the next patch will be v4?) like others do. And "kbuild: deb-pkg:" as the patch subject. ('git log script/package/buildeb' to see examples) > > > > + done > > > > > > - mkdir -p "${pdir}/DEBIAN" > > > + mkdir -p "${pdir}/DEBIAN" > > > > Please drop this noise change. > > > > If you want to optimize this, please split it into a separate patch like > > "kbuild: deb-pkg: create DEBIAN directory just once" etc. > > Okay, no need to optimize this now. mkdir -p is cheap. > > Thanks! > > cheers, josch -- Best Regards Masahiro Yamada
diff --git a/scripts/package/builddeb b/scripts/package/builddeb index 441b0bb66e0d..6e83f6f3ec6d 100755 --- a/scripts/package/builddeb +++ b/scripts/package/builddeb @@ -5,10 +5,12 @@ # # Simple script to generate a deb package for a Linux kernel. All the # complexity of what to do with a kernel after it is installed or removed -# is left to other scripts and packages: they can install scripts in the -# /etc/kernel/{pre,post}{inst,rm}.d/ directories (or an alternative location -# specified in KDEB_HOOKDIR) that will be called on package install and -# removal. +# is left to other scripts and packages. Scripts can be placed into the +# preinst, postinst, prerm and postrm directories in /etc/kernel or +# /usr/share/kernel. A different list of search directories can be given +# via KDEB_HOOKDIR. Scripts in directories earlier in the list will +# override scripts of the same name in later directories. The script will +# be called on package installation and removal. set -eu @@ -68,11 +70,18 @@ install_linux_image () { # kernel packages, as well as kernel packages built using make-kpkg. # make-kpkg sets $INITRD to indicate whether an initramfs is wanted, and # so do we; recent versions of dracut and initramfs-tools will obey this. - debhookdir=${KDEB_HOOKDIR:-/etc/kernel} + debhookdir=${KDEB_HOOKDIR:-/etc/kernel /usr/share/kernel} + + # Only pre-create the first hook directory. Support for more than one hook + # directory requires run-parts 5.21 and it is the responsibility of packages + # creating additional hook directories to declare that dependency. + firsthookdir=${debhookdir%% *} for script in postinst postrm preinst prerm; do - mkdir -p "${pdir}${debhookdir}/${script}.d" + mkdir -p "${pdir}${firsthookdir}/${script}.d" + done - mkdir -p "${pdir}/DEBIAN" + mkdir -p "${pdir}/DEBIAN" + for script in postinst postrm preinst prerm; do cat <<-EOF > "${pdir}/DEBIAN/${script}" #!/bin/sh @@ -84,7 +93,15 @@ install_linux_image () { # Tell initramfs builder whether it's wanted export INITRD=$(if_enabled_echo CONFIG_BLK_DEV_INITRD Yes No) - test -d ${debhookdir}/${script}.d && run-parts --arg="${KERNELRELEASE}" --arg="/${installed_image_path}" ${debhookdir}/${script}.d + # run-parts will error out if one of its directory arguments does not + # exist, so filter the list of hook directories accordingly. + hookdirs= + for dir in ${debhookdir}; do + test -d "\$dir/${script}.d" || continue + hookdirs="\$hookdirs \$dir/${script}.d" + done + hookdirs="\${hookdirs# }" + test -n "\$hookdirs" && run-parts --arg="${KERNELRELEASE}" --arg="/${installed_image_path}" \$hookdirs exit 0 EOF chmod 755 "${pdir}/DEBIAN/${script}"
By passing an additional directory to run-parts, allow Debian and its derivatives to ship maintainer scripts in /usr while at the same time allowing the local admin to override or disable them by placing hooks of the same name in /etc. This adds support for the mechanism described in the UAPI Configuration Files Specification for kernel hooks. The same idea is also used by udev, systemd or modprobe for their config files. https://uapi-group.org/specifications/specs/configuration_files_specification/ This functionality relies on run-parts 5.21 or later. It is the responsibility of packages installing hooks into /usr/share/kernel to also declare a Depends: debianutils (>= 5.21). KDEB_HOOKDIR can be used to change the list of directories that is searched. By default, /etc/kernel and /usr/share/kernel are hook directories. Signed-off-by: Johannes Schauer Marin Rodrigues <josch@mister-muffin.de> --- scripts/package/builddeb | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-)