Hi, Quoting Santiago Vila (2021-10-04 13:02:33) > Looking at the original report, I read this: > > > It will point to the installation root with the trailing slash > > stripped. That means under normal conditions, it is empty. > > So, if I understood correctly: > > - if installation root is / then DPKG_ROOT is "" > - but if it's /mnt then DPKG_ROOT is "/mnt" > > I guess this is unlikely to change at this point, however: Would not > have been cleaner to specify it as the installation root (without the > "trailing slash stripped" part).
the path not having the trailing slash and thus the root path being the empty
string has the advantage, that it is easy to see that under normal operation,
the DPKG_ROOT patch does nothing. Most of our patches to maintainer scripts
look like this:
-sometool "/some/path"
+sometool "$DPKG_ROOT/some/path"
Since DPKG_ROOT is empty in normal circumstances, it is obvious that the patch
cannot change the behaviour of the script. Furthermore, since DPKG_ROOT is
usually used as a prefix of an absolute path, it also avoids a double slash
when concatenating them.
> Another question: Can you explain the usage of ":" command at the beginning?
>
> : "${DPKG_ROOT:=}"
>
> This is to define DPKG_ROOT as the empty string in case it's undefined,
> right? Is this really needed in a POSIX shell? I believed ${DPKG_ROOT}
> would expand to empty string when it's not defined.
>
> Or maybe you meant this? : "${DPKG_ROOT:=/}"
That line is not needed anymore. The original patch was written in May 2016
when dpkg 1.18.5 which introduced DPKG_ROOT was just freshly uploaded to
unstable. Some maintainer scripts include a "set -u" which would cause an error
when variables that are not set are used. So back then, our patches included
that line to avoid this. Today, dpkg sets DPKG_ROOT even in old-old-stable so
it is save to assume that it is set. Additionally, the base-files script does
not call "set -u" so even if it were unset, it would just become the empty
string. So the line can be safely removed. I just updated our patch in our CI
infrastructure and can confirm that nothing breaks without that line.
> Also: just to be sure: Am I right to think that the sed command in
> change_owner function is there to be able to bootstrap a Debian system from a
> non-Debian system?
No, we don't expect the outside system not to be Debian. The sed is there so
that the chown call obtains user and group ids from the chroot instead of the
outside system. There are alternatives to this approach:
a) assume that the outside /etc/passwd and /etc/group will always be in sync
with the copy of the chroot and thus place a few more restrictions on the
package versions of the outside system
b) hardcode the uid of root and the gid of utmp and staff and hope those
never change
c) obtain the uid of root and the gid of utmp and staff at build-time of
base-files and write them into the postinst script
> And finally: Do we really need to consider DPKG_ROOT in
> update_to_current_default function? (It should only used on upgrades).
No, that's not strictly necessary. Right now we only test installation and not
upgrades, so that can be dropped from the initial patch.
Thanks!
cheers, josch
signature.asc
Description: signature
