[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#844458: bootstrap-base: if debootstrap_script is unset, DEBOOTSTRAP_SCRIPT is set to a directory, breaking the install

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
  if [ ! -e /usr/share/debootstrap/scripts/"$DEBOOTSTRAP_SCRIPT" ]; then
 	error "debootstrap script '$DEBOOTSTRAP_SCRIPT' doesn't exist"

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:


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:


(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

Reply to: