Re: [PATCH 1/1] scripts/package/builddeb: allow hooks also in /usr/share/kernel

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Attachment: signature.asc
Description: signature


[Index of Archives]     [Linux&nblp;USB Development]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite Secrets]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux