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
signature.asc
Description: signature
