Bug#1035820: 9base: leaves entries in /etc/shells after upgrade from bullseye
On 14/06/2023 19.04, Andreas Beckmann wrote: I've prepared a NMU with three changes: Uploaded to DELAYED/10: debianutils (5.7-0.5) unstable; urgency=medium . * Non-maintainer upload. * update-shells: Do not create duplicate entries in /etc/shells. * update-shells: Manage (/usr)/bin/sh in the state file. * update-shells: Fix canonicalization of shells in aliased locations. (Closes: #1035820) Andreas
Bug#1035820: 9base: leaves entries in /etc/shells after upgrade from bullseye
Control: unmerge -1 Control: reassign -1 debianutils 5.7-0.4 Control: affects -1 + src:9base Control: tag -1 patch pending On 13/06/2023 07.26, Helmut Grohne wrote: The entry is added by /usr/lib/usrmerge/convert-etc-shells (which is the policy violation this bug has been merged into). The merging strategy there is based on a knowledge of the symlinks that should exist rather than looking them up in the filesystem. So this is two distinct bugs actually. We also need to fix update-shells to properly deal with update-alternatives using the patch above. Let's use this bug for the debianutils side. I've prepared a NMU with three changes: https://salsa.debian.org/anbe/debianutils/-/commits/fix-update-shells * avoid creating duplicate entries in /etc/shells * handle /bin/sh as defined in the template file * fix canonicalization of shells that are symlinks Especially with the /bin/sh change in place, update-shells and convert-etc-shells should produce similar results. convert-etc-shells needs to call update-shells (before manipulating /etc/shells on its own, s.t. the statefile gets updated), and maybe it can be replaced entirely by update-shells. Andreas
Bug#1035820: 9base: leaves entries in /etc/shells after upgrade from bullseye
Hi Andreas, On Mon, Jun 12, 2023 at 07:40:32PM +0200, Andreas Beckmann wrote: > > It's a 12 byte difference. That's not 9base's entries. What you see here > > is "/usr/bin/sh\n". So this is a /usr-merge bug. We already know it. > > Thus force-merging. > > No, it's "/usr/bin/rc\n". Huh! I stand corrected. > # cat /usr/share/debianutils/shells.d/plan9 > /bin/rc > /usr/lib/plan9/bin/rc > > If I install 9base in an unmerged-/usr sid chroot, I get > > # /etc/shells: valid login shells > /bin/sh > /bin/bash > /bin/rbash > /bin/dash > /bin/rc > /usr/lib/plan9/bin/rc > /usr/lib/plan9/bin/rc > > who is responsible for the duplicate entry? This is caused by update-shells and the code it carries to support /usr-merge. When it encounters /bin/rc, it canonicalizes it using dpkg-realpath and adds it as well. So the first duplicate is the canonicalization of /bin/rc and the second one is the actual entry. In essence, the /usr-merge implementation in update-shells assumes that any kind of symlinking for shells happens due to /usr-merge. This assumption is false and violated by the use of update-alternatives. In update-shells, we should not resolve the final link, but only directory components as those are the ones that may lead to aliasing effects. Instead of realshell=$(dirname "$(dpkg-realpath --root "$ROOT" "$line")")/$(basename "$line") we probably should have said realshell="$(dpkg-realpath --root "$ROOT" "$(dirname "$line")")/$(basename "$line")" and with that, we'd pass /bin rather than /bin/rc to dpkg-realpath and avoid resolving the alternative. > If I reconfigure usrmerge afterwards, I get > > # /etc/shells: valid login shells > /bin/sh > /bin/bash > /usr/bin/bash > /bin/rbash > /usr/bin/rbash > /bin/dash > /usr/bin/dash > /usr/bin/sh > /bin/rc > /usr/lib/plan9/bin/rc > /usr/lib/plan9/bin/rc > /usr/bin/rc > > Now we got the additional /usr/bin/rc entry The entry is added by /usr/lib/usrmerge/convert-etc-shells (which is the policy violation this bug has been merged into). The merging strategy there is based on a knowledge of the symlinks that should exist rather than looking them up in the filesystem. So this is two distinct bugs actually. We also need to fix update-shells to properly deal with update-alternatives using the patch above. Thank you Andreas for your persistence on this. Helmut
Bug#1035820: 9base: leaves entries in /etc/shells after upgrade from bullseye
Now I'm really confused: I start with an unmerged sid chroot, install 9base there. Then I remove /etc/unsupported-skip-usrmerge-conversion and install usrmerge. Now /etc/shells contains # /etc/shells: valid login shells /bin/sh /bin/bash /bin/rbash /bin/dash /bin/rc /usr/lib/plan9/bin/rc /usr/lib/plan9/bin/rc /usr/bin/sh /usr/bin/bash /usr/bin/rbash /usr/bin/dash /usr/bin/rc and /var/lib/shells.state /bin/bash /bin/rbash /bin/dash /bin/rc /usr/lib/plan9/bin/rc /usr/lib/plan9/bin/rc Then I run update-shells which does not modify /etc/shells, but the statefile now contains /bin/bash /usr/bin/bash /bin/rbash /usr/bin/rbash /bin/dash /usr/bin/dash /bin/rc /usr/lib/plan9/bin/rc /usr/lib/plan9/bin/rc So the /bin entries for bash/rbash/dash get their /usr counterparts added, but /bin/rc does not. OK, back to the minimal unmerged sid chroot, without 9base this time. After installation of usrmerge and successful conversio /etc/shells contains # /etc/shells: valid login shells /bin/sh /bin/bash /bin/rbash /bin/dash /usr/bin/sh /usr/bin/bash /usr/bin/rbash /usr/bin/dash and /var/lib/shells.state contains /bin/bash /bin/rbash /bin/dash Then I delete all /usr/bin entries from /etc/shells (sed -i /usr/d /etc/shells) and run update-shells, resulting in /etc/shells containing # /etc/shells: valid login shells /bin/sh /bin/bash /bin/rbash /bin/dash /usr/bin/bash /usr/bin/rbash /usr/bin/dash (i.e. only /usr/bin/sh is missing compared to the file made by usrmerge) and the statefile contains /bin/bash /usr/bin/bash /bin/rbash /usr/bin/rbash /bin/dash /usr/bin/dash This seems to imply that usrmerge could call update-shells to perform most of the changes it does to /etc/shells. (And to update /var/lib/shells.state at the same time) ... later ... much later ... After reading update-shells several times (and still not fully getting how it works (or at least should work)), I think its behavior is not well defined in the context of alternatives as used by 9base. At least this line seems to be wrong: realshell=$(dirname "$(dpkg-realpath --root "$ROOT" "$line")")/$(basename "$line") It resolves e.g. /usr/bin/clang++-14 to non-existant /usr/lib/llvm-14/bin/clang++-14 Shouldn't that rather be realshell=$(dpkg-realpath --root "$ROOT" "$(dirname "$line")")/$(basename "$line") Andreas
Bug#1035820: 9base: leaves entries in /etc/shells after upgrade from bullseye
On 09/05/2023 16.55, Helmut Grohne wrote: Control: forcemerge 1033167 -1 Control: affects 1033167 + 9base Hi Andreas, On Tue, May 09, 2023 at 04:39:21PM +0200, Andreas Beckmann wrote: during a test with piuparts I noticed your package leaves modifications in /etc/shells after upgrading from bullseye to bookworm and purging the package. 9base/bullseye called add-shell/remove-shell in its postinst/postrm. 9base/bookworm no longer does that, but it also does not clean up the leftover entries from bullseye in its postinst. 9base/bookworm no longer does, because it now uses dpkg-triggers to perform the cleanup. It actually does clean up its entries. >From the attached log (scroll to the bottom...): 0m45.2s ERROR: FAIL: After purging files have been modified: /etc/shells not owned You should look closer: 0m45.2s DEBUG: Modified(user, group, mode, size, target): /etc/shells expected(root, root, - 100644, 128, None) != found(root, root, - 100644, 140, None) It's a 12 byte difference. That's not 9base's entries. What you see here is "/usr/bin/sh\n". So this is a /usr-merge bug. We already know it. Thus force-merging. No, it's "/usr/bin/rc\n". # cat /usr/share/debianutils/shells.d/plan9 /bin/rc /usr/lib/plan9/bin/rc If I install 9base in an unmerged-/usr sid chroot, I get # /etc/shells: valid login shells /bin/sh /bin/bash /bin/rbash /bin/dash /bin/rc /usr/lib/plan9/bin/rc /usr/lib/plan9/bin/rc who is responsible for the duplicate entry? If I install 9base in a merged-/usr sid chroot, I get # /etc/shells: valid login shells /bin/sh /bin/bash /usr/bin/bash /bin/rbash /usr/bin/rbash /bin/dash /usr/bin/dash /usr/bin/sh /bin/rc /usr/lib/plan9/bin/rc /usr/lib/plan9/bin/rc same duplication ... If I reconfigure usrmerge afterwards, I get # /etc/shells: valid login shells /bin/sh /bin/bash /usr/bin/bash /bin/rbash /usr/bin/rbash /bin/dash /usr/bin/dash /usr/bin/sh /bin/rc /usr/lib/plan9/bin/rc /usr/lib/plan9/bin/rc /usr/bin/rc Now we got the additional /usr/bin/rc entry Andreas
Bug#1035820: 9base: leaves entries in /etc/shells after upgrade from bullseye
On 09/05/2023 16.55, Helmut Grohne wrote: 9base/bookworm no longer does, because it now uses dpkg-triggers to Good to know that there is a better way nowadays ... >From the attached log (scroll to the bottom...): 0m45.2s ERROR: FAIL: After purging files have been modified: /etc/shells not owned You should look closer: 0m45.2s DEBUG: Modified(user, group, mode, size, target): /etc/shells expected(root, root, - 100644, 128, None) != found(root, root, - 100644, 140, None) It's a 12 byte difference. That's not 9base's entries. What you see here is "/usr/bin/sh\n". So this is a /usr-merge bug. We already know it. Thus force-merging. Given that this seems to be the only package showing this behavior in my bullseye2bookworm tests so far, I didn' expect some "systematic" error outside of this package. Andreas
Bug#1035820: 9base: leaves entries in /etc/shells after upgrade from bullseye
Control: forcemerge 1033167 -1 Control: affects 1033167 + 9base Hi Andreas, On Tue, May 09, 2023 at 04:39:21PM +0200, Andreas Beckmann wrote: > during a test with piuparts I noticed your package leaves modifications > in /etc/shells after upgrading from bullseye to bookworm and purging the > package. > > 9base/bullseye called add-shell/remove-shell in its postinst/postrm. > 9base/bookworm no longer does that, but it also does not clean up the > leftover entries from bullseye in its postinst. 9base/bookworm no longer does, because it now uses dpkg-triggers to perform the cleanup. It actually does clean up its entries. > >From the attached log (scroll to the bottom...): > > 0m45.2s ERROR: FAIL: After purging files have been modified: > /etc/shells not owned You should look closer: 0m45.2s DEBUG: Modified(user, group, mode, size, target): /etc/shells expected(root, root, - 100644, 128, None) != found(root, root, - 100644, 140, None) It's a 12 byte difference. That's not 9base's entries. What you see here is "/usr/bin/sh\n". So this is a /usr-merge bug. We already know it. Thus force-merging. > The following (untested) snippet for the postinst should perform the > neccessary cleanup: > > if [ "$1" = "install" ] || [ "$1" = "upgrade" ]; then > if dpkg --compare-versions "$2" lt-nl "1:6-14~" ; then > remove-shell /bin/rc > remove-shell /usr/lib/plan9/bin/rc > fi > fi No. Please continue to use the declarative approach. Helmut
Bug#1035820: 9base: leaves entries in /etc/shells after upgrade from bullseye
Package: 9base Version: 1:6-13 Severity: serious User: debian...@lists.debian.org Usertags: piuparts Hi, during a test with piuparts I noticed your package leaves modifications in /etc/shells after upgrading from bullseye to bookworm and purging the package. 9base/bullseye called add-shell/remove-shell in its postinst/postrm. 9base/bookworm no longer does that, but it also does not clean up the leftover entries from bullseye in its postinst. >From the attached log (scroll to the bottom...): 0m45.2s ERROR: FAIL: After purging files have been modified: /etc/shellsnot owned The following (untested) snippet for the postinst should perform the neccessary cleanup: if [ "$1" = "install" ] || [ "$1" = "upgrade" ]; then if dpkg --compare-versions "$2" lt-nl "1:6-14~" ; then remove-shell /bin/rc remove-shell /usr/lib/plan9/bin/rc fi fi cheers, Andreas 9base_1:6-13.log.gz Description: application/gzip
