| Message ID | 20241128062940.1708257-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 Thu, Nov 28, 2024 at 3:29 PM Johannes Schauer Marin Rodrigues <josch@mister-muffin.de> wrote: > > By passing an additional directory to run-parts, allow Debian and its > derivatives an easy way to ship maintainer scripts in /usr while at the > same time allowing the local admin to easily 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: > 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 | 44 ++++++++++++++++++++++++++++++++-------- > 1 file changed, 36 insertions(+), 8 deletions(-) > > diff --git a/scripts/package/builddeb b/scripts/package/builddeb > index 441b0bb66e0d..2772146a76ce 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,26 @@ 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" || exit 0 > + > + # If more than one hook directory remained, check version of run-parts. If > + # run-parts is too old, fall back to only processing the first. > + case \$hookdirs in *" "*) if ! run-parts --help 2>&1 \ > + | grep -Fxq "Usage: run-parts [OPTION]... DIRECTORY [DIRECTORY ...]"; then > + echo "E: run-parts >=5.21 is required for multiple hook directories, falling back to $firsthookdir" >&2 Same comment as in the previous version. If both /etc/kernel/postinst.d/ and /usr/share/kernel/postinst.d/ exist, can we assume the run-parts>=5.12 on that system? Do we need to check the help message and offer the fallback mechanism? > + test -d "${firsthookdir}/${script}.d" || exit 0 > + hookdirs="${firsthookdir}/${script}.d" > + fi > + esac > + run-parts --arg="${KERNELRELEASE}" --arg="/${installed_image_path}" \$hookdirs > exit 0 > EOF > chmod 755 "${pdir}/DEBIAN/${script}" > -- > 2.39.2 > >
Hi, Quoting Masahiro Yamada (2024-12-02 16:42:02) > > @@ -84,7 +93,26 @@ 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" || exit 0 > > + > > + # If more than one hook directory remained, check version of run-parts. If > > + # run-parts is too old, fall back to only processing the first. > > + case \$hookdirs in *" "*) if ! run-parts --help 2>&1 \ > > + | grep -Fxq "Usage: run-parts [OPTION]... DIRECTORY [DIRECTORY ...]"; then > > + echo "E: run-parts >=5.21 is required for multiple hook directories, falling back to $firsthookdir" >&2 > > Same comment as in the previous version. > If both /etc/kernel/postinst.d/ and /usr/share/kernel/postinst.d/ exist, > can we assume the run-parts>=5.12 on that system? since KDEB_HOOKDIR can now be any directories and any number of directories, the question should rather be: if more than one directory from KDEB_HOOKDIR exists, can we assume that run-parts>=5.12 exists on that system? Personally, I'd prefer a best-effort fallback mechanism. The alternative would be that kernel installation would just error out in case a (buggy) package on a distro ships something in /usr/share/kernel/postinst.d/ but failed to also declare a versioned dependency against debianutils. The error message cannot (or rather only with considerable effort) tell the user *why* their kernel installation errored out. By only considering the first hook directory (probably /etc) in those situation, the kernel would succeed to install and the hooks from the (buggy) package would be skipped. I understand that such a behaviour comes with its own set of disadvantages. One could also argue, that it is better to error out loudly in case of an error instead of hiding a message prefixed with a "E:" in a bunch of console output when a kernel package gets installed. What is your position on this question? What behaviour would you prefer? If you strongly prefer the kernel installation to error out loudly if run-parts is too old, then my next patch will implement just that. I think whether "we can assume run-parts>=5.12" depends on what we declare to be the right way to hold this feature. If we say "packages must declare this versioned dependency and if they fail to do this then it is their bug and not ours" then yes, then we can assume run-parts>=5.12 in case of multiple directories. > Do we need to check the help message and offer the fallback mechanism? The answer two that question depends on the answer to the last question. If we want to error out loudly with unsupported run-parts, then no help message has to be checked. Otherwise, when we want to check what version of run-parts we have, then there are two options. Either parsing the --version output (which is not trivial itself because run-parts prints six lines of copyright information) or parsing the --help output. The debianutils maintainer encouraged using the latter option which is why I chose that one. Thanks! cheers, josch
On Tue, Dec 3, 2024 at 2:59 AM Johannes Schauer Marin Rodrigues <josch@mister-muffin.de> wrote: > > Hi, > > Quoting Masahiro Yamada (2024-12-02 16:42:02) > > > @@ -84,7 +93,26 @@ 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" || exit 0 > > > + > > > + # If more than one hook directory remained, check version of run-parts. If > > > + # run-parts is too old, fall back to only processing the first. > > > + case \$hookdirs in *" "*) if ! run-parts --help 2>&1 \ > > > + | grep -Fxq "Usage: run-parts [OPTION]... DIRECTORY [DIRECTORY ...]"; then > > > + echo "E: run-parts >=5.21 is required for multiple hook directories, falling back to $firsthookdir" >&2 > > > > Same comment as in the previous version. > > If both /etc/kernel/postinst.d/ and /usr/share/kernel/postinst.d/ exist, > > can we assume the run-parts>=5.12 on that system? > > since KDEB_HOOKDIR can now be any directories and any number of directories, > the question should rather be: if more than one directory from KDEB_HOOKDIR > exists, can we assume that run-parts>=5.12 exists on that system? > > Personally, I'd prefer a best-effort fallback mechanism. The alternative would > be that kernel installation would just error out in case a (buggy) package on a > distro ships something in /usr/share/kernel/postinst.d/ but failed to also > declare a versioned dependency against debianutils. The error message cannot > (or rather only with considerable effort) tell the user *why* their kernel > installation errored out. By only considering the first hook directory > (probably /etc) in those situation, the kernel would succeed to install and the > hooks from the (buggy) package would be skipped. I understand that such a > behaviour comes with its own set of disadvantages. One could also argue, that > it is better to error out loudly in case of an error instead of hiding a > message prefixed with a "E:" in a bunch of console output when a kernel package > gets installed. > > What is your position on this question? What behaviour would you prefer? If you > strongly prefer the kernel installation to error out loudly if run-parts is too > old, then my next patch will implement just that. I think whether "we can > assume run-parts>=5.12" depends on what we declare to be the right way to hold > this feature. If we say "packages must declare this versioned dependency and if > they fail to do this then it is their bug and not ours" then yes, then we can > assume run-parts>=5.12 in case of multiple directories. My preference is to pass the existing hook directories to run-parts. If KDEB_HOOKDIR specifies two directories and both exist, pass them to run-parts. > > > Do we need to check the help message and offer the fallback mechanism? > > The answer two that question depends on the answer to the last question. If we > want to error out loudly with unsupported run-parts, then no help message has > to be checked. Otherwise, when we want to check what version of run-parts we > have, then there are two options. Either parsing the --version output (which is > not trivial itself because run-parts prints six lines of copyright information) > or parsing the --help output. The debianutils maintainer encouraged using the > latter option which is why I chose that one. > > Thanks! > > cheers, josch
diff --git a/scripts/package/builddeb b/scripts/package/builddeb index 441b0bb66e0d..2772146a76ce 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,26 @@ 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" || exit 0 + + # If more than one hook directory remained, check version of run-parts. If + # run-parts is too old, fall back to only processing the first. + case \$hookdirs in *" "*) if ! run-parts --help 2>&1 \ + | grep -Fxq "Usage: run-parts [OPTION]... DIRECTORY [DIRECTORY ...]"; then + echo "E: run-parts >=5.21 is required for multiple hook directories, falling back to $firsthookdir" >&2 + test -d "${firsthookdir}/${script}.d" || exit 0 + hookdirs="${firsthookdir}/${script}.d" + fi + esac + 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 an easy way to ship maintainer scripts in /usr while at the same time allowing the local admin to easily 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: 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 | 44 ++++++++++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 8 deletions(-)