Bug#1035654: non-essential adduser poses problems to purging packages

[Andreas Beckmann]

[resending to the bug]

On 07/05/2023 10.49, Sebastian Ramacher wrote:
> #1034758 x2goserver-common x2goserver-common: fails to purge - command (deluser|delgroup) in postrm not found
> #1035291 desktop-autoloader desktop-autoloader: fails to purge - command (deluser|adduser) in postrm not found 2023-04-30
> #1035292 debian-edu-fai debian-edu-fai: fails to purge - command (deluser|adduser) in postrm not found 2023-04-30
> #1035435 webdis webdis: fails to purge - command (deluser|adduser) in postrm not found

#1035284, znc, postinst, fixed in sid, needs unblock
#1035436, rkdeveloptool, postinst
#1034530, mrtg, postrm, fixed in sid, maybe needs unblock
#1035494, moonshot-trust-router, postrm, fixed in sid, maybe needs unblock

There are a few more only affecting sid.

Andreas

PS: once apt drops the dependency on adduser again in bookworm+1, we
should consider adding versioned Breaks against the packages that didn't
get their adduser usage fixed for bookworm

[Johannes Schauer Marin Rodrigues]

Hi,

Quoting Helmut Grohne (2023-05-07 13:52:50)
>
> I contend that:
>
> 1. This change is in unstable since 2022-10-31, i.e. more than half a
> year.
> 2. While having adduser drop from the essential+apt set is caused by
> apt dropping it, this was an implementation detail and any package
> using adduser without a dependency was (invisibly) buggy before.
>
> So I don't quite buy the reasoning of "too late" or it being apt's
> fault.
>
> On the flip side, I think it would technically have been the
> responsibility of the proponents of dropping adduser to do the testing.
> The proponents have been Josch and my self and we ultimately failed to do so.
> Thanks to Andreas for doing it for us.

this is true. I must admit that I haven't thought of maintainer scripts using
adduser during purge and failing because they expect adduser to always be
installed. Even though it is too late, I ran this script on all 11864 binary
packages in unstable amd64 that have either a postrm or prerm script:

#!/bin/sh
set -eu
ret=0
PKG_TO_TEST="$1" mmdebstrap --logfile="$1.log" --variant=essential \
--customize-hook='APT_CONFIG=$MMDEBSTRAP_APT_CONFIG apt-get -oDPkg::Chroot-Directory="$1" --yes install --no-install-recommends "$PKG_TO_TEST"' \
--customize-hook='APT_CONFIG=$MMDEBSTRAP_APT_CONFIG apt-get -oDPkg::Chroot-Directory="$1" --yes remove adduser' \
--customize-hook='printf "#!/bin/sh\nset -eu\necho \"I: adduser-dummy: attempting to run \$0\" >&2\nexit 1\n" > "$1/usr/sbin/adduser-dummy"' \
--customize-hook='chmod +x "$1/usr/sbin/adduser-dummy"' \
--customize-hook='ln -s adduser-dummy "$1/usr/sbin/adduser"' \
--customize-hook='ln -s adduser-dummy "$1/usr/sbin/deluser"' \
--customize-hook='ln -s adduser-dummy "$1/usr/sbin/addgroup"' \
--customize-hook='ln -s adduser-dummy "$1/usr/sbin/delgroup"' \
--customize-hook='APT_CONFIG=$MMDEBSTRAP_APT_CONFIG apt-get -oDPkg::Chroot-Directory="$1" --yes remove --purge "$PKG_TO_TEST"' \
unstable /dev/null http://127.0.0.1:3142/debian || ret=$?
if [ "$ret" -eq 0 ] && ! grep --silent "^I: adduser-dummy: attempting to run " "$1.log"; then
rm "$1.log"
fi

I did not limit the investigation to the packages with strings like deluser and
delgroup in their maintainer scripts to make sure to also catch packages that
indirectly call adduser tools through another script.

Out of the tested 11864 packages, 133 called adduser tools at one point or the
other but purging still succeeds (for example because of a || true). 210 of the
tested 11864 packages fail this script. Out of the 210 failures, 17 are
Essential:yes packages and thus failed. Out of the 193 remaining failures, 113
fail to install in the first place. Out of the 80 remaining failures, 32 fail
because they try calling adduser tools which are not installed and thus fail. I
skimmed through the 48 other purging failures and they seem to be nearly all
related to gtk and gsettings. Of the remaining 32 adduser related purging
problems, 18 packages check if deluser/delgroup exist before calling it and
then fail because I installed the dummy. The remaining 14 failures belong to
the following 9 source packages:

amavisd-new #1035841
debian-edu-fai #1035292
desktop-autoloader #1035291
kismet #1035290
matrix-sydent #1035844
tcpcryptd #1035845
webdis #1035435
x2gobroker #1035847
x2gothinclient #1034755

Out of these 9 remaining source packages, 5 had bugs already filed by Andreas
Beckmann. I filed bugs with patches for the remaining 4 packages and offered
to NMU if necessary.

> > With such a change I would have expected upgrade/piuparts tests from
> > bullseye to bookworm that tried to remove adduser a various stages and
> > check for the fallout. Given that Andreas is only doing them now, that's
> > too late for changes to the pseudo-essential set.

Given that only 9 source packages in unstable fail to purge because they expect
adduser to be present, I have another proposal. I think making adduser
Essential:yes (even if temporary) or making it a dependency of an Essential:yes
has the downside, that now package maintainers have official reason to rely on
its functionality and include that into their maintainer scripts. Finding these
new bugs becomes a bit more tricky because to test for this, an Essential:yes
package has to be removed. Same goes for making it pseudo-essential via apt
because removing adduser to test regressions would remove apt.

According to [popcon], adduser is installed on 100% of the systems providing
popcon data. This makes sense because probably near 100% of the systems
submitting popcon data either has apt installed (which used to pull in adduser)
or was installed via d-i (which pulls in adduser because it is
Priority:important).

I would thus argue that what we have to guard against is only the *removal* of
adduser. How about, instead of re-introducing adduser to the Essential or
pseudo-Essential set (and thus communicating the wrong thing to maintainers),
we make adduser Protected:yes for a release cycle and thus make its removal
much more scary. I'd argue that since everybody has adduser installed, we do
not need to make sure it gets installed again but just make sure that it
doesn't get removed. Doing it like that would also have the additional benefit
of not changing the Essential set this late in the release cycle which will
introduce RC bugs in other packages.

What do you think?

Thanks!

cheers, josch

[popcon] https://qa.debian.org/popcon.php?package=adduser

[Johannes Schauer Marin Rodrigues]

Hi,

Quoting Nicolas Dandrimont (2023-05-18 20:51:04)
> On Thu, May 18, 2023, at 10:03, Marc Haber wrote:
> > adduser probably needs an additional hint because the new upload makes
> > piuparts fail now, as discussed yesterday.
> To work around this issue on the piuparts side, it sounds like we should make
> piuparts treat adduser as fake-essential for tests ending at bookworm or sid,
> so that we don't try to uninstall it; Andreas, what do you think?

a more general solution would be to skip uninstallation on all packages marked
with Protected:yes or to only uninstall with --allow-remove-essential. Such a
solution would not only benefit adduser but also any future packages marked
with Protected:yes.

Thanks!

cheers, josch

[Mike Gabriel]

Hi Josch,

On Do 18 Mai 2023 00:24:39 CEST, Johannes Schauer Marin Rodrigues wrote:

> Mike, you said on IRC that you want to file the unblock bugs for
> debian-edu-fai, desktop-autoloader, x2gobroker and x2gothinclient
> (which didn't
> happen yet). If you like, I can file these for you. Just say the word. :)

because I am running short of time, I'd like to accept your offer of
filing the unblock bugs. Please go ahead.

Thanks,
Mike
--

mike gabriel aka sunweaver (Debian Developer)
mobile: +49 (1520) 1976 148
landline: +49 (4351) 486 14 27

GnuPG Fingerprint: 9BFB AEE8 6C0A A5FF BF22 0782 9AF4 6B30 2577 1B31
mail: sunw...@debian.org, http://sunweavers.net