fix up new maint script code lacking DPKG_ROOT support

This fixes a regression in DPKG_ROOT support introduced via !12 (merged).

Fixes: 38c940fc ("libc6:: create merged-usr symlinks via preinst")

@josch, could you review this?

I've reviewed this and tried it out. Thanks for the ping. The problem is, that even without your patch, our CI will happily pass because it will (wrongly) check /lib on the outside, realize that everything is okay and then not do anything, thus not breaking anything.

@bluca if I understand correctly, this code is only there to guard against cases where libc was removed which would result in symlink removal and in that case, the symlink has to be re-created? In that case, this code is not useful at all for initial installations and instead of helmut's patch and my patch below that does the same for cf50a0b4, we could put the whole code block into a if [ -z "$DPKG_ROOT" ] condition because DPKG_ROOT is only interested in the initial installation but not in package upgrades or removals.

diff --git a/debian/debhelper.in/libc-alt.preinst b/debian/debhelper.in/libc-alt.preinst
index fb544f14..53ad4c28 100644
--- a/debian/debhelper.in/libc-alt.preinst
+++ b/debian/debhelper.in/libc-alt.preinst
@@ -9,13 +9,13 @@ set -e
 # Once all packages install only under /usr, this can be removed, as removing
 # this package will no longer result in the symlink being deleted.
 if [ "$1" = "install" ] || [ "$1" = "upgrade" ]; then
-    if [ -L /lib ]; then
+    if [ -L "$DPKG_ROOT/lib" ]; then
         # Has the link already been created?
         # If it has not, is a directory already there? Half-merged systems are
         # the problem of usrmerge, simply ignore them here.
-        if [ ! -L SLIBDIR ] && [ ! -d SLIBDIR ]; then
-            mkdir -p /usrSLIBDIR
-            ln -s usrSLIBDIR SLIBDIR
+        if [ ! -L "${DPKG_ROOT}SLIBDIR" ] && [ ! -d "${DPKG_ROOT}SLIBDIR" ]; then
+            mkdir -p "${DPKG_ROOT}/usrSLIBDIR"
+            ln -s usrSLIBDIR "${DPKG_ROOT}SLIBDIR"
         fi
     fi
 fi

This snippet is needed on installation though? remove -> install, it's not in the upgrade path, right?

But is it needed on an initial installation? The DPKG_ROOT stuff is needed for creating a chroot directory. So the DPKG_ROOT stuff is only needed for the part where you go from an empty directory to a chroot filled with installed and configured packages. If I understand correctly what this code is guarding against, then something has to be removed first for the situation to happen that this code is attempting to fix, right? And removals do not happen during a debootstrap run.

As far as I understand it is not needed on the initial installation, only on following re-installations (but from uninstalled state, so not upgrades). But @anbe found the issue, so I'll let him confirm whether my understanding is correct.

@helmut you should also look at cf50a0b4

@bluca this is your code, you should deal with the bugs, please review.

looks good to me - the DPKG_ROOT implementers told me in other cases to avoid adding DPKG_ROOT where not requested, and that it will be handled as needed, so I have not added it to these snippets myself

Yes, that is correct. If you do not know exactly what you are doing, I think it's best to leave the work of implementing DPKG_ROOT to the bootstrap team. We do not expect maintainers to do that work for us but will submit patches as required. If anything, a small heads-up would be nice so that we can fix problems before they make it into unstable.

Any progress on that?

As far as I can see, we still need exactly what is proposed:

1. @josch confirmed that we don't see differences in CI, because we check the "wrong" location. This code is relevant to bootstrap.
2. Aurelien pointed at cf50a0b4. I think we can ignore that, because multilib isn't part of the supported package set.
3. While this code could maybe be skipped on foreign bootstraps, it is simpler to just fix it and not bother with running it conditionally.

The remark from @bluca that this code is needed for remove -> install doesn't make too much sense to me though, because removing libc6 is not something you usually do to a Debian system. If it is needed for that situation only, it should be deleted entirely instead. Of course removing multilib libcs is something we do, so that doesn't apply to cf50a0b4.

The remark from @bluca that this code is needed for remove -> install doesn't make too much sense to me though, because removing libc6 is not something you usually do to a Debian system.

Multiarch libc6 can be installed and removed? It might not be a common case, but then again so is installing and removing the multilib one

We may not remove the native libc6, but installing and removing foreign architecture libc6 is a valid possibility.

Andreas

added 2 commits

• 13e20c91 - 1 commit from branch sid
• 3e88ce72 - fix up new maint script code lacking DPKG_ROOT support

Compare with previous version

merged