Hi, I've spend the better part of today on this thanks to Freexian.
On Mon, Nov 20, 2023 at 12:35:58AM +0100, Helmut Grohne wrote: > Different reproducer: > > mmdebstrap trixie /dev/null http://deb.debian.org/debian > --include=systemd-sysv,molly-guard --customize-hook='sed -i -e s/trixie/sid/ > $1/etc/apt/sources.list' --chrooted-customize-hook='apt-get update' > --customize-hook='test -e $1/lib/molly-guard/reboot' > --chrooted-customize-hook='apt-get -y install systemd-sysv' > --customize-hook='ls -l $1/lib/molly-guard/' > I've dug into dpkg and usually when it moves a file from / to /usr, > it'll first unpack the new file (unknowingly replacing the existing old > one) and then removes the old one (via pkg_remove_old_files). During > that removal, it has a check to see whether the file to be removed > happens to match one of the files it just installed and skips the > removal in that case. For some reason, this safety check does not work > when the file is diverted. I retried this a few times and still think it is correct. As a consequence, the original approach of duplicating diversions cannot work at all. As soon as we have two diversions whose targets equal up to aliasing, we run into this problem and to make matters worse, we are loosing a file that we just unpacked. We have no way of keeping (and later restoring) it via any kind of maintainer scripts. Therefore we really cannot do the duplicate diversion approach. It was a nice idea, but doesn't work. Back to the drawing board. We still must have two diversions to as unpacking either of the locations from another package would clobber the location we want diverted. What changes is that the diversion targets must differ in more than aliasing. I think an example would help. Say we divert /sbin/halt. We must divert both /sbin/halt and /usr/sbin/halt or we risk clobbering our copy. If we divert the latter to /usr/lib/molly-guard/halt, we must not divert the former to /lib/molly-guard/halt or the file will be lost in unpack. So the later might become /lib/molly-guard/halt.usr-is-merged or something. This is fairly inconvenient as molly-guard doesn't know about halt.usr-is-merged. I think there is three possible ways to deal with this: a) molly-guard tries both locations. b) We add a dpkg-trigger on /sbin/halt such that when /usr/lib/molly-guard/halt is missing but /lib/molly-guard/halt.usr-is-merged is not, we can add a symlink. c) We give up on supporting /sbin/halt (as opposed to /usr/sbin/halt) and simply declare Breaks against any provider of /sbin/halt. I argue that we should be selecting that last option, because /sbin/halt is something that we want to go away entirely. When we release trixie, we want all of them moved to /usr/sbin/halt. So we'd have versioned Breaks for systemd-sysv, sysvinit-core, runit-init and finit-sysv. Note that since Breaks does not prevent concurrent unpack, we must still install the extra diversion. Note that since the other package must declare Conflicts with molly-guard, molly-guard cannot use Conflicts without impacting upgrades. The mutual conflict would require apt to temporarily remove molly-guard (or /sbin/init!) and that is known to impact upgrades. Also note that since sysvinit-core and others have not yet moved, molly-guard must now declare unversioned Breaks for all of them for as long as they have not moved and once they moved, it can add a version to the Breaks. So I went ahead an implemented this. Since I didn't want to fall into my trap of too little testing again, I wrote some test cases (attached) and to my surprise found another problem! /sbin/halt actually is a symbolic link to /bin/systemctl. In 255, /usr/sbin/halt becomes a symlink to ../bin/systemctl. Observe how we turned an absolute symlink into a relative once since we no longer cross a toplevel directory in line with policy. As we move this relative symlink to /usr/lib/molly-guard, it becomes a broken symbolic link. So if you now install molly-guard and systemd-sysv in unstable, these links are gone. You may then reinstall systemd-sysv and see them reinstated. And now they're broken. There is no easy way to fix this beyond moving these programs to a different name in the same directory. I suggest /usr/sbin/halt.no-molly-guard. Appending a suffix is what most diversions do and this avoids breaking symlinks. I've attached the prospective change to this mail. It passes piuparts and it passes my own test cases. I definitely think it should be reviewed before uploaded. One thing I already see for possible improvement is that since we declare Breaks against all providers of /sbin/halt, this diversion does not actually have to persist beyond postinst. Once molly-guard is configured, we know that any broken package is no longer installed nor unpacked. A molly-guard.postinst could remove the diversion of /sbin/halt to /sbin/halt.no-molly-guard.usr-is-merged. Then we are only left with one diversion per command which also makes this a lot less confusing. So rather than rushing this now, I hope to elicit some feedback and review. I've made some non-trivial choices here and want you to understand the trade-offs involved. Changing plans after upload would complicate maintainer scripts to handle upgrades. I hope that systemd people can provide some patience as we sort out their fallout. And if this leaves more questions than answers, by all means ask. You wouldn't be the first to raise an important matter via an innocuous looking question. If you wonder about the strange synax I introduced in maintainer scripts, see https://salsa.debian.org/jelmer/lintian-brush/-/merge_requests/39 and feel free to remove my markup. More fundamentally, I'll also have to rework the DEP17 section M18 as it doesn't work the way it is currently pictured. Thanks for bearing with me Helmut
testcase.sh
Description: Bourne shell script
TESTS= \
-_molly \
molly_molly \
sidmolly_molly \
newmolly_rmmolly \
systemd_molly \
sysvinit_molly \
systemd-newmolly_rmmolly \
sysvinit-newmolly_rmmolly \
molly-systemd_molly \
molly-sysvinit_molly \
sidmolly-systemd_molly \
sidmolly-sysvinit_molly \
newmolly_systemd \
newmolly-systemd_systemd \
newmolly-systemd_rmsystemd \
newmolly-systemd_rmmolly \
newmolly-systemd_sysvinit \
newmolly_sidsystemd_rmsystemd \
newmolly_sidsystemd_rmmolly \
newmolly_sidsystemd_sysvinit \
newmolly_sysvinit \
newmolly-sysvinit_rmsysvinit \
newmolly-sysvinit_rmmolly \
newmolly-sysvinit_systemd \
all: $(foreach t,$(TESTS),testout/$(t))
testout/%:
./testcase.sh "$(firstword $(subst _, ,$*))" "$(lastword $(subst _,
,$*))" >"$@" 2>&1; echo $$? >> "$@"
diff -Nru molly-guard-0.8.1/debian/changelog
molly-guard-0.8.1+nmu1/debian/changelog
--- molly-guard-0.8.1/debian/changelog 2023-11-11 23:02:55.000000000 +0100
+++ molly-guard-0.8.1+nmu1/debian/changelog 2023-11-20 09:18:25.000000000
+0100
@@ -1,3 +1,10 @@
+molly-guard (0.8.1+nmu1) UNRELEASED; urgency=medium
+
+ * Non-maintainer upload.
+ * Attempt to fix the /usr-merge fallout.
+
+ -- Helmut Grohne <hel...@subdivi.de> Mon, 20 Nov 2023 09:18:25 +0100
+
molly-guard (0.8.1) unstable; urgency=medium
* Upload to unstable
diff -Nru molly-guard-0.8.1/debian/control molly-guard-0.8.1+nmu1/debian/control
--- molly-guard-0.8.1/debian/control 2023-11-11 23:02:55.000000000 +0100
+++ molly-guard-0.8.1+nmu1/debian/control 2023-11-20 09:18:25.000000000
+0100
@@ -23,6 +23,7 @@
systemd,
sysvinit,
upstart
+Breaks: systemd-sysv (<< 255), sysvinit-core, finit-sysv, runit-init
Description: protects machines from accidental shutdowns/reboots
The package installs a shell script that overrides the existing
shutdown/reboot/halt/poweroff/coldreboot/pm-hibernate/pm-suspend* commands
diff -Nru molly-guard-0.8.1/debian/molly-guard.postrm
molly-guard-0.8.1+nmu1/debian/molly-guard.postrm
--- molly-guard-0.8.1/debian/molly-guard.postrm 2023-11-11 23:02:55.000000000
+0100
+++ molly-guard-0.8.1+nmu1/debian/molly-guard.postrm 2023-11-20
09:18:25.000000000 +0100
@@ -20,11 +20,8 @@
case "$1" in
remove)
for cmd in halt poweroff reboot shutdown coldreboot ; do
- dpkg-divert --package molly-guard --no-rename --remove /sbin/$cmd
- dpkg-divert --package molly-guard --no-rename --remove
"/usr/sbin/$cmd"
- if test -e "/usr/lib/molly-guard/$cmd"; then
- mv "/usr/lib/molly-guard/$cmd" "/usr/sbin/$cmd"
- fi
+ dpkg-divert --package molly-guard --rename --remove /sbin/$cmd
+ dpkg-divert --package molly-guard --rename --remove
"/usr/sbin/$cmd"
done
for cmd in pm-hibernate pm-suspend pm-suspend-hybrid ; do
diff -Nru molly-guard-0.8.1/debian/molly-guard.preinst
molly-guard-0.8.1+nmu1/debian/molly-guard.preinst
--- molly-guard-0.8.1/debian/molly-guard.preinst 2023-11-11
23:02:55.000000000 +0100
+++ molly-guard-0.8.1+nmu1/debian/molly-guard.preinst 2023-11-20
09:18:25.000000000 +0100
@@ -14,35 +14,55 @@
case "$1" in
install|upgrade)
- mkdir -p /usr/lib/molly-guard
-
# Cleanup erroneous diversions added in 0.6.0
for cmd in pm-hibernate pm-suspend pm-suspend-hybrid ; do
dpkg-divert --package molly-guard --rename --remove /sbin/$cmd
done
for cmd in halt poweroff reboot shutdown coldreboot ; do
- dpkg-divert --package molly-guard --divert
"/usr/lib/molly-guard/$cmd" --no-rename --add "/usr/sbin/$cmd"
- # DEP17 M18 duplicated diversion. Can be removed after trixie.
- dpkg-divert --package molly-guard --divert "/lib/molly-guard/$cmd"
--no-rename --add "/sbin/$cmd"
- # Avoid --rename as long as we need duplicated diversions.
+ # begin-remove-after: trixie
+ # Remove possible pre-/usr-merge diversion
+ dpkg-divert --package molly-guard --divert "/lib/molly-guard/$cmd"
--no-rename --remove "/sbin/$cmd"
+ if test "$(dpkg-divert --truename "/usr/sbin/$cmd")" =
"/usr/lib/molly-guard/$cmd"; then
+ dpkg-divert --package molly-guard --divert
"/usr/lib/molly-guard/$cmd" --no-rename --remove "/usr/sbin/$cmd"
+ if test -e "/usr/lib/molly-guard/$cmd"; then
+ mv "/usr/lib/molly-guard/$cmd"
"/usr/sbin/$cmd.no-molly-guard"
+ fi
+ if test -e "/lib/molly-guard/$cmd"; then
+ mv "/lib/molly-guard/$cmd"
"/usr/sbin/$cmd.no-molly-guard.usr-is-merged"
+ fi
+ fi
+ # end-remove-after: trixie
+ # DEP17 M18 duplicated diversion. Can be --removed after trixie.
+ dpkg-divert --package molly-guard --divert
"/sbin/$cmd.no-molly-guard.usr-is-merged" --no-rename --add "/sbin/$cmd"
+ # Add post-/usr-merge diversion meant to stay.
+ dpkg-divert --package molly-guard --divert
"/usr/sbin/$cmd.no-molly-guard" --no-rename --add "/usr/sbin/$cmd"
+ # Avoid --rename as long as we need duplicated diversions.
if test "$1" = install; then
if test -e "/usr/sbin/$cmd"; then
- mv "/usr/sbin/$cmd" "/usr/lib/molly-guard/$cmd"
+ mv "/usr/sbin/$cmd" "/usr/sbin/$cmd.no-molly-guard"
fi
if test -e "/sbin/$cmd"; then
- mv "/sbin/$cmd" "/usr/lib/molly-guard/$cmd"
+ mv "/sbin/$cmd" "/sbin/$cmd.no-molly-guard.usr-is-merged"
fi
fi
done
for cmd in pm-hibernate pm-suspend pm-suspend-hybrid ; do
- if test "$(dpkg-divert --truename "/usr/sbin/$cmd")" =
"/lib/molly-guard/$cmd"; then
- dpkg-divert --package molly-guard --divert
"/lib/molly-guard/$cmd" --no-rename --remove "/usr/sbin/$cmd"
- dpkg-divert --package molly-guard --divert
"/usr/lib/molly-guard/$cmd" --no-rename --add "/usr/sbin/$cmd"
+ # begin-remove-after: trixie
+ truename="$(dpkg-divert --truename "/usr/sbin/$cmd")"
+ if test "${truename#/usr}" = "/lib/molly-guard/$cmd"; then
+ dpkg-divert --package molly-guard --divert "$truename"
--no-rename --remove "/usr/sbin/$cmd"
+ dpkg-divert --package molly-guard --divert
"/usr/sbin/$cmd.no-molly-guard" --no-rename --add "/usr/sbin/$cmd"
+ if test -e "/usr/lib/molly-guard/$cmd" -o -h
"/usr/lib/molly-guard/$cmd"; then
+ mv "/usr/lib/molly-guard/$cmd"
"/usr/sbin/$cmd.no-molly-guard"
+ fi
else
- dpkg-divert --package molly-guard --divert
"/usr/lib/molly-guard/$cmd" --rename "/usr/sbin/$cmd"
+ # end-remove-after: trixie
+ dpkg-divert --package molly-guard --divert
"/usr/sbin/$cmd.no-molly-guard" --rename "/usr/sbin/$cmd"
+ # begin-remove-after: trixie
fi
+ # end-remove-after: trixie
done
;;
diff -Nru molly-guard-0.8.1/shutdown.in molly-guard-0.8.1+nmu1/shutdown.in
--- molly-guard-0.8.1/shutdown.in 2023-11-11 23:02:55.000000000 +0100
+++ molly-guard-0.8.1+nmu1/shutdown.in 2023-11-20 09:18:25.000000000 +0100
@@ -13,11 +13,10 @@
SCRIPTSDIR="@cfgdir@/run.d"
CMD="${0##*/}"
-EXEC="@REALPATH@/$CMD"
case "$CMD" in
halt|reboot|shutdown|poweroff|coldreboot|pm-hibernate|pm-suspend|pm-suspend-hybrid)
- if [ ! -f $EXEC ]; then
+ if ! EXEC=$(command -v "$CMD.no-molly-guard"); then
echo "E: not a regular file: $EXEC" >&2
exit 4
fi
