Hi Guillem,
you asked me to review your next/d-m-h-root branch. Thanks to all who've
worked on this! I've looked at the two commits as one diff
(12961967a563..6aa3bf8f98b8) without attributing individual hunks to the
respective authors. This is what I found:
* Diagnostic messages tend to include DPKG_ROOT. Is that a useful thing
to do? My intuition was that I'd not include DPKG_ROOT there.
For example:
echo "Obsolete conffile ${DPKG_ROOT}$CONFFILE has been modified by you."
Counter examples:
echo "Restoring backup of $SYMLINK ..."
echo "Restoring backup of $PATHNAME ..."
At the very least, consistency seems in order here.
* In dir_to_symlink, in the case for postrm, in the second (long)
condition, I think there is a line that misses a trailing backslash.
[ \( ! -h "${DPKG_ROOT}$PATHNAME" -a
* In readlink_canon, in the final realpath the $dst argument likely
should be quoted.
* In readlink_canon I was wondering whether passing DPKG_ROOT inside a
sed expression is a good idea. How about reworking the final line:
-realpath $dst | sed -e "s:^$DPKG_ROOT::"
+dst=$(realpath $dst)
+echo ${dst#"$DPKG_ROOT"}
The quoting of the variable removes any pattern meaning from its
contents.
* I observe that readlink -f and readlink_canon significantly differ in
their behaviour. While readlink -f canonicalizes symbolic links in
every component of the path, readlink_canon only canonicalizes
symbolic links in the final component. It then uses realpath saying
that no symlinks are involved, which may be untrue.
I do wonder whether implementing this feature here is a useful thing
to do. It seems that we need a chroot-aware readlink and/or realpath
in more occasions. Possibly extending coreutils is a better solution?
In any case, I'm attaching a version of readlink_canon that deals
with this.
* I observe that the coding style is inconsistent about using test
expressions using -a/-o and &&/||. There is a slight reason to prefer
the latter, see:
https://github.com/koalaman/shellcheck/wiki/SC2166
That's all. Looks like we can go ahead with this with minor changes.
Helmut
Attachment:
readlink_f.sh
Description: Bourne shell script