Bug#853224: desktop-base: Minor problems with maintainer scripts

[Aurélien COUDERC <zecoucou@free.fr>]

control: tags -1 pending
control: severity -1 minor

Hi Raphaël,

thanks for your in depth review.
It was obviously useful. :-)


Le 30/01/2017 à 18:49, Raphaël Hertzog a écrit :
> Package: desktop-base
> Version: 9.0.2
> Severity: normal
> User: devel@kali.org
> Usertags: origin-kali
>
> I'm updating the Kali package based on 9.0.2 and I am thus reviewing the
> changes. I found some problems:
>
> * In debian/postrm:
That’s prerm.

> - there are many "$priority" variables used that should be dropped as the
>   update-alternatives --remove call does not need a priority
Yes, fixed.

> - there's a "read theme priority" that should be changed into "read theme"
Yes, fixed.

> - you are removing the "desktop-grub.sh" alternative but it's a slave
>   alternative... aren't those automatically removed?
>   And in fact, you are removing them twice... in two consecutive loops.
Indeed, fixed.

> * In debian/preinst:
>   You have code like this:
>   if $2 <= 9.0.0~exp1; then
>     ...
>     if $2 >= 9.0.0~exp1; then
>       echo "ge 9.0.0~exp1"
>       ...
>     fi
>   fi
>   if $2 < 9.0.0~; then
>     ...
>   fi
>
>   First there's debugging code (the "echo" line)
Yes, removed.
>  and what's the point of
>   the embedded check >= 9.0.0~exp1 ?  The only way to trigger it is when $2 = 9.0.0~exp1
>   since it must meet >= 9.0.0~exp1 and <= 9.0.0~exp1 at the same time.
>
>   The ordering does not seem very logic. You usally deal with upgrade
>   problems of older versions first. So it seems to me that you should
>   do something like this:
>   if $2 < 9.0.0~; then
>     ...
>   fi
>   if $2 = 9.0.0~exp1; then
>     ...
>   fi
>
>   And AFAIK you don't need any other case since 9.0.0~exp1 followed
>   immediately 8.0.2.
Right, agreed, fixed as you proposed.

So all this is committed into SVN.
I’ve quickly tested install and upgrades from 8.0.2 and from 9.0.2, and
they worked without errors.
Feel free to re-check though :
https://anonscm.debian.org/viewvc/debian-desktop?view=revision&revision=445

It won’t make it into Stretch as the freeze it at the door, but it
should be fine. These are all more style / cleanups fixes that don’t
change the result of maintscripts execution AFAICT.
I’ve requalified severity as minor to match the bug’s title but if you
think it’s inappropriate, please revert it.


Cheers,
--Aurélien