Philip Hands <phil@hands.com> writes: > Tianon Gravi <tianon@debian.org> writes: > >> On 15 November 2016 at 15:36, Philip Hands <phil@hands.com> wrote: >>> This seems to have resulted from the recent change to bootstrap-base to >>> allow the script to be specified only as the codename, but which is not >>> checking whether the debconf variable is actually set. >>> >>> This commit should therefore fix the problem: >>> >>> http://deb.li/iUtYi >>> >>> I've applied that change to a broken image by hand, and it does solve the problem. >> >> Doh, sorry I missed this edge case! >> >> For what it's worth, the proposed change looks legit to me. :) > > Glad yo like it, but on re-examination, I'm not so sure -- should the > concatenated path not also be checked to see if it exists? So, the previous code was: db_get base-installer/debootstrap_script DEBOOTSTRAP_SCRIPT="$RET" if [ ! -e /usr/share/debootstrap/scripts/"$DEBOOTSTRAP_SCRIPT" ]; then error "debootstrap script '$DEBOOTSTRAP_SCRIPT' doesn't exist" fi which throws an error if base-installer/debootstrap_script is set to a path that does not exist, but does not throw an error if it is unset (as long as the /usr/share/debootstrap/scripts/ directory exists). It seems to me that the commit message for a8b68f8e should have been "Allow absolute paths ..." because the code already dealt with codenames, since it was already checking for the existence of: /usr/share/debootstrap/scripts/"$DEBOOTSTRAP_SCRIPT" Looking at the debootstrap code, if this is unset, then there is no $4 so we're going to get the script set to "$DEBOOTSTRAP_DIR/scripts/$1" (where $1 is the suite), possibly with .$VARIANT added if set. This seems to be the default behaviour. If the debconf variable was set to an absolute path, then I guess the old code would break, since it unconditionally prefixed the path before checking for existence. If it were set to the suite name, then the test in bootstrap-base.postinst would pass, but debootstrap would then have a suite name as the script, so the: if [ ! -e "$SCRIPT" ]; then test would fail, so I'm guessing this code never worked. Is that actually right? In light of all that, I think that this commit actually does the right thing, and is rather more obvious than my previous attempt: http://deb.li/3IOXc (but I still need to test it...) Cheers, Phil. P.S. I used the if ... && [ "$RET" ] style test because it's also in use for the variant setting, and in the unlikely event that base-installer/debootstrap_script were unset, it would still do the right thing, rather than throwing an error becuase of the set -e. -- |)| Philip Hands [+44 (0)20 8530 9560] HANDS.COM Ltd. |-| http://www.hands.com/ http://ftp.uk.debian.org/ |(| Hugo-Klemm-Strasse 34, 21075 Hamburg, GERMANY
Attachment:
signature.asc
Description: PGP signature