[PATCH] make builddeb invoke calls to linux-update-symlink

[PATCH] make builddeb invoke calls to linux-update-symlink

[Jason Self]

The official Debian kernel packages, in addition to running the
scripts in /etc/kernel, also runs linux-update-symlinks via the
postinst and postrm scripts. This maintains the /vmlinuz and
/initrd.img (if it uses an initramfs) symlinks. This change makes the
builddeb script generate those same calls.

Signed-off-by: Jason Self <j@jxself.org>
---
 scripts/package/builddeb | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/scripts/package/builddeb b/scripts/package/builddeb
index 6df3c9f8b2da..f34c11c63ede 100755
--- a/scripts/package/builddeb
+++ b/scripts/package/builddeb
@@ -195,6 +195,14 @@ export DEB_MAINT_PARAMS="\$*"
 # Tell initramfs builder whether it's wanted
 export INITRD=$(if_enabled_echo CONFIG_BLK_DEV_INITRD Yes No)
 
+$(case $script in
+postinst)
+        echo linux-update-symlinks install $version $installed_image_path
+        ;;
+postrm)
+        echo linux-update-symlinks remove $version $installed_image_path
+esac)
+
 test -d $debhookdir/$script.d && run-parts --arg="$version" --arg="/$installed_image_path" $debhookdir/$script.d
 exit 0
 EOF
-- 
2.26.0

Re: [PATCH] make builddeb invoke calls to linux-update-symlink

[Masahiro Yamada]

Hi Ben,

On Sun, May 10, 2020 at 1:06 PM Jason Self <j@jxself.org> wrote:
>
> The official Debian kernel packages,

I'd like to consult you.
Does this patch look OK?


Thanks.



> in addition to running the
> scripts in /etc/kernel, also runs linux-update-symlinks via the
> postinst and postrm scripts. This maintains the /vmlinuz and
> /initrd.img (if it uses an initramfs) symlinks. This change makes the
> builddeb script generate those same calls.
>
> Signed-off-by: Jason Self <j@jxself.org>
> ---
>  scripts/package/builddeb | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/scripts/package/builddeb b/scripts/package/builddeb
> index 6df3c9f8b2da..f34c11c63ede 100755
> --- a/scripts/package/builddeb
> +++ b/scripts/package/builddeb
> @@ -195,6 +195,14 @@ export DEB_MAINT_PARAMS="\$*"
>  # Tell initramfs builder whether it's wanted
>  export INITRD=$(if_enabled_echo CONFIG_BLK_DEV_INITRD Yes No)
>
> +$(case $script in
> +postinst)
> +        echo linux-update-symlinks install $version $installed_image_path
> +        ;;
> +postrm)
> +        echo linux-update-symlinks remove $version $installed_image_path
> +esac)
> +
>  test -d $debhookdir/$script.d && run-parts --arg="$version" --arg="/$installed_image_path" $debhookdir/$script.d
>  exit 0
>  EOF
> --
> 2.26.0
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH] make builddeb invoke calls to linux-update-symlink

[Jason Self]

[-- Attachment #1: Type: text/plain, Size: 619 bytes --]

On Sat, 23 May 2020 02:01:17 +0900
Masahiro Yamada <masahiroy@kernel.org> wrote:

> Hi Ben,
> 
> On Sun, May 10, 2020 at 1:06 PM Jason Self <j@jxself.org> wrote:
> >
> > The official Debian kernel packages,  
> 
> I'd like to consult you.
> Does this patch look OK?

No. After submitting the patch I saw that I made a stupid mistake and
forgot to terminate one of the case statements and to use 
/$installed_image_path instead of $installed_image_path so that it
provides the absolute path. I can resubmit it with those corrections
but have been waiting before doing so in case there is more feedback.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH] make builddeb invoke calls to linux-update-symlink

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 2020 bytes --]

On Sat, 2020-05-23 at 02:01 +0900, Masahiro Yamada wrote:
> Hi Ben,
> 
> On Sun, May 10, 2020 at 1:06 PM Jason Self <j@jxself.org> wrote:
> > The official Debian kernel packages,
> 
> I'd like to consult you.
> Does this patch look OK?

No, it doesn't.

> > in addition to running the
> > scripts in /etc/kernel, also runs linux-update-symlinks via the
> > postinst and postrm scripts. This maintains the /vmlinuz and
> > /initrd.img (if it uses an initramfs) symlinks. This change makes the
> > builddeb script generate those same calls.
> > 
> > Signed-off-by: Jason Self <j@jxself.org>
> > ---
> >  scripts/package/builddeb | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/scripts/package/builddeb b/scripts/package/builddeb
> > index 6df3c9f8b2da..f34c11c63ede 100755
> > --- a/scripts/package/builddeb
> > +++ b/scripts/package/builddeb
> > @@ -195,6 +195,14 @@ export DEB_MAINT_PARAMS="\$*"
> >  # Tell initramfs builder whether it's wanted
> >  export INITRD=$(if_enabled_echo CONFIG_BLK_DEV_INITRD Yes No)
> > 
> > +$(case $script in
> > +postinst)
> > +        echo linux-update-symlinks install $version $installed_image_path
> > +        ;;
> > +postrm)
> > +        echo linux-update-symlinks remove $version $installed_image_path
> > +esac)

This doesn't handle upgrades properly.  The extra conditions in the
maintainer scripts for official Debian packages really are needed.

It's also not correct to assume that linux-update-symlinks is always
available.  If the generated linux-image package depends on "linux-base 
(>= 4.1~)" then the postinst script can use it unconditionally. 
However, the postrm script will always have to check whether it is
present.

Ben.

> >  test -d $debhookdir/$script.d && run-parts --arg="$version" --arg="/$installed_image_path" $debhookdir/$script.d
> >  exit 0
> >  EOF
> > --
> > 2.26.0
> > 
> 
> 
-- 
Ben Hutchings
Every program is either trivial or else contains at least one bug



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH] make builddeb invoke calls to linux-update-symlinks

[Jason Self]

The official Debian kernel packages, in addition to running the
scripts in /etc/kernel, also runs linux-update-symlinks via the
postinst script. This updates the /vmlinuz and /initrd.img (if it uses
an initramfs) symlinks to point to the newly installed kernel. This
update causes the builddeb script to do the same.

This version of the patch incorporates the feedback from Ben
Hutchings.

Signed-off-by: Jason Self <j@jxself.org>
---
 scripts/package/builddeb | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/scripts/package/builddeb b/scripts/package/builddeb
index 6df3c9f8b2da..707693867c29 100755
--- a/scripts/package/builddeb
+++ b/scripts/package/builddeb
@@ -195,6 +195,37 @@ export DEB_MAINT_PARAMS="\$*"
 # Tell initramfs builder whether it's wanted
 export INITRD=$(if_enabled_echo CONFIG_BLK_DEV_INITRD Yes No)
 
+$(case $script in
+    postinst)
+        echo "if [ -f /lib/modules/$version/.fresh-install ]; then
+    change=install
+else
+    change=upgrade
+fi
+if command -v linux-update-symlinks > /dev/null; then
+    linux-update-symlinks $change $version /$installed_image_path
+fi
+rm -f /lib/modules/$version/.fresh-install"
+        ;;
+    postrm)
+        echo "if command -v linux-update-symlinks > /dev/null; then
+    linux-update-symlinks remove $version /$installed_image_path
+fi"
+        ;;
+    preinst)
+        echo 'if [ "$1" = install ]; then
+    # Create a flag file for postinst
+    mkdir -p /lib/modules/$version
+    touch /lib/modules/$version/.fresh-install
+fi'
+        ;;
+    prerm)
+        echo "if command -v linux-check-removal > /dev/null; then
+    linux-check-removal $version
+fi"
+        ;;
+esac)
+
 test -d $debhookdir/$script.d && run-parts --arg="$version" --arg="/$installed_image_path" $debhookdir/$script.d
 exit 0
 EOF
-- 
2.26.0

Re: [PATCH] make builddeb invoke calls to linux-update-symlinks

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 2697 bytes --]

On Sat, 2020-05-23 at 13:38 -0700, Jason Self wrote:
> The official Debian kernel packages, in addition to running the
> scripts in /etc/kernel, also runs linux-update-symlinks via the
> postinst script. This updates the /vmlinuz and /initrd.img (if it uses
> an initramfs) symlinks to point to the newly installed kernel. This
> update causes the builddeb script to do the same.
> 
> This version of the patch incorporates the feedback from Ben
> Hutchings.

Well, partly.  You implemented detection of upgrades on the install
side, but not on the remove side.

In case of an upgrade, both the old version's prerm and postrm, and the
new version's preinst and postinst, will be run:
https://www.debian.org/doc/debian-policy/ch-maintainerscripts.html#details-of-unpack-phase-of-installation-or-upgrade

> Signed-off-by: Jason Self <j@jxself.org>
> ---
>  scripts/package/builddeb | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/scripts/package/builddeb b/scripts/package/builddeb
> index 6df3c9f8b2da..707693867c29 100755
> --- a/scripts/package/builddeb
> +++ b/scripts/package/builddeb
> @@ -195,6 +195,37 @@ export DEB_MAINT_PARAMS="\$*"
>  # Tell initramfs builder whether it's wanted
>  export INITRD=$(if_enabled_echo CONFIG_BLK_DEV_INITRD Yes No)
>  
> +$(case $script in
> +    postinst)
> +        echo "if [ -f /lib/modules/$version/.fresh-install ]; then
> +    change=install
> +else
> +    change=upgrade
> +fi
> +if command -v linux-update-symlinks > /dev/null; then
> +    linux-update-symlinks $change $version /$installed_image_path
> +fi
> +rm -f /lib/modules/$version/.fresh-install"
> +        ;;
> +    postrm)
> +        echo "if command -v linux-update-symlinks > /dev/null; then
> +    linux-update-symlinks remove $version /$installed_image_path
> +fi"

postrm should not run linux-update-symlinks if "$1" is "upgrade".

> +        ;;
> +    preinst)
> +        echo 'if [ "$1" = install ]; then
> +    # Create a flag file for postinst
> +    mkdir -p /lib/modules/$version
> +    touch /lib/modules/$version/.fresh-install
> +fi'
> +        ;;
> +    prerm)
> +        echo "if command -v linux-check-removal > /dev/null; then
> +    linux-check-removal $version
> +fi"

prerm should not run linux-check-removal if "$1" is "upgrade".

Also, you should mention use of linux-check-removal in the commit
message.

Ben.

> +        ;;
> +esac)
> +
>  test -d $debhookdir/$script.d && run-parts --arg="$version" --arg="/$installed_image_path" $debhookdir/$script.d
>  exit 0
>  EOF
-- 
Ben Hutchings
You can't have everything.  Where would you put it?


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] make builddeb invoke calls to linux-update-symlink

[Jason Self]

[-- Attachment #1: Type: text/plain, Size: 185 bytes --]

My apologies. I realized this patch is incomplete. It should also be
invoking linux-update-symlinks as part of postrm as well. I will
re-send this. My apologies for the added noise.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[PATCH] make builddeb invoke calls to linux-update-symlink

[Jason Self]

The official Debian kernel packages, in addition to running the
scripts in /etc/kernel, also runs linux-update-symlinks via the
postinst script. This updates the /vmlinuz and /initrd.img (if it uses
an initramfs) symlinks to point to the newly installed kernel. This
update causes the builddeb script to do the same.

Signed-off-by: Jason Self <j@jxself.org>
---
 scripts/package/builddeb | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/scripts/package/builddeb b/scripts/package/builddeb
index 6df3c9f8b2da..1932c54cc338 100755
--- a/scripts/package/builddeb
+++ b/scripts/package/builddeb
@@ -195,6 +195,12 @@ export DEB_MAINT_PARAMS="\$*"
 # Tell initramfs builder whether it's wanted
 export INITRD=$(if_enabled_echo CONFIG_BLK_DEV_INITRD Yes No)
 
+$(case $script in
+postinst)
+        echo linux-update-symlinks install $version $installed_image_path
+        ;;
+esac)
+
 test -d $debhookdir/$script.d && run-parts --arg="$version" --arg="/$installed_image_path" $debhookdir/$script.d
 exit 0
 EOF
-- 
2.26.0