Bug#618920: debootstrap: needs more robust download error handling

2012-03-13 Thread Colin Watson 
On Sat, Mar 19, 2011 at 11:18:14AM -0400, Michael Gilbert wrote:
 debootstrap's current download error handling isn't very robust.  It
 declares success just for the presence of a downloaded file, which may
 be a partial download, or one for which the checksum doesn't match.
 Eventually those conditions will lead to unhandled failures elsewhere
 within debootstrap.

Thanks for the patch!  I've attached a test proxy which (with various
modifications depending on exactly what you're trying to test) can be
useful to forcibly recreate this kind of bug by making certain downloads
be truncated or fail in whatever other way you like.

I'm particularly disturbed that I've found it possible for debootstrap
to succeed in some cases despite some corrupted downloads.  This should
definitely not be possible, and I agree that we need something along the
lines of your patch.

 It may also be useful to expand a bit on these d-i debootstrap error
 messages: when an error happens, the right answer that the user wants is
 to hit 'go back' twice in a row to start the debootstrap all over again,
 but the dialogs are confusing, and 'continue' seems to be the obvious
 choice, but that will lead to the broken debootstrap continuing to
 completion with various brokenness. Anyway, that maybe should be
 submitted as another bug.

This is http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=283600, I
think.

 So, back to the original issue, I've created a patch that will retry
 downloads whenever anything in the get routine fails, which I
 believe is much more robust than the current situation.  Please see
 attached patch.

 --- newhd/source/debootstrap-1.0.28/functions 2011-02-21 19:25:08.0 
 -0500
 +++ /usr/share/debootstrap/functions  2011-03-19 10:58:57.0 -0400
 @@ -1,3 +1,5 @@
 +MAXATTEMPTS=10
 +
  ### smallutils
  
  smallyes() {
 @@ -241,6 +243,13 @@
  }
  
  get () {
 + for iters in $(seq 1 $MAXATTEMPTS); do
 + if single_get $@; then break; fi
 + warning RETRYING Retrying failed download.
 + done
 +}
 +
 +single_get () {
   # args: from dest 'nocache'
   # args: from dest [checksum size] [alt {checksum size type}]
   local displayname
 @@ -331,13 +340,6 @@
   # http/ftp mirror
   if wgetprogress -O $dest $from; then
   return 0
 - elif [ -s $dest ]; then
 - local iters=0
 - while [ $iters -lt 3 ]; do
 - warning RETRYING Retrying failed download of 
 %s $from
 - if wgetprogress -c -O $dest $from; then 
 break; fi
 - iters=$(($iters + 1))
 - done
[...]

This isn't quite right, though.  You can tell by the change in the
signature of the RETRYING warning; compare that with
base-installer/debian/bootstrap-base.templates and you'll see that
that's going to confuse d-i because there's one parameter too few for
the warning.  In any case, I think the warning should indicate which
download it's retrying.

This can be fixed by moving the retry loop into the original get
function, inside the 'for a in $order' loop.  That way we know $from
when issuing the RETRYING warning.  A further refinement is to remove
$dest2 on failure, so that it doesn't inadvertently confuse something
else later.

I've committed my improved version, which you're welcome to review:

  
http://anonscm.debian.org/gitweb/?p=d-i/debootstrap.git;a=commitdiff;h=733069bb97bdfe3f9c16ca4c9ef58685205eabf3;hp=412608c1fbe28074f56d930242e4269d5588d101

Thanks,

-- 
Colin Watson   [cjwat...@debian.org]
#! /usr/bin/perl
use warnings;
use strict;
use HTTP::Proxy;
use HTTP::Proxy::HeaderFilter::simple;
use HTTP::Proxy::BodyFilter::simple;

sub truncate {
my ($self, $dataref, $message, $protocol, $buffer) = @_;
$$dataref = '';
#$$dataref = substr $$dataref, 0, 1;
}

my $proxy = HTTP::Proxy-new(port = 7890, logmask = HTTP::Proxy::STATUS);
$proxy-push_filter(
path = qr/klibc/,
mime = undef,
response = HTTP::Proxy::BodyFilter::simple-new(\truncate));
$proxy-start;

Bug#618920: marked as done (debootstrap: needs more robust download error handling)

2012-03-13 Thread Debian Bug Tracking System 
Your message dated Tue, 13 Mar 2012 17:32:12 +
with message-id e1s7va0-0003jh...@franck.debian.org
and subject line Bug#618920: fixed in debootstrap 1.0.39
has caused the Debian Bug report #618920,
regarding debootstrap: needs more robust download error handling
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact ow...@bugs.debian.org
immediately.)


-- 
618920: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=618920
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems
---BeginMessage---
package: debootstrap
version: 1.0.28
severity: important
tags: patch

debootstrap's current download error handling isn't very robust.  It
declares success just for the presence of a downloaded file, which may
be a partial download, or one for which the checksum doesn't match.
Eventually those conditions will lead to unhandled failures elsewhere
within debootstrap.

This can make d-i very rocky on certain mirrors since one bad package
download ultimately lead to a complete failure of the debootstrap
process, and thus a failed install.  An expert can recover from this,
but an average user will get rather frustrated (especially since the
dialogs for debootstrap errors are rather confusing).  

It may also be useful to expand a bit on these d-i debootstrap error
messages: when an error happens, the right answer that the user wants is
to hit 'go back' twice in a row to start the debootstrap all over again,
but the dialogs are confusing, and 'continue' seems to be the obvious
choice, but that will lead to the broken debootstrap continuing to
completion with various brokenness. Anyway, that maybe should be
submitted as another bug.

So, back to the original issue, I've created a patch that will retry
downloads whenever anything in the get routine fails, which I
believe is much more robust than the current situation.  Please see
attached patch.

Best wishes,
Mike
--- newhd/source/debootstrap-1.0.28/functions	2011-02-21 19:25:08.0 -0500
+++ /usr/share/debootstrap/functions	2011-03-19 10:58:57.0 -0400
@@ -1,3 +1,5 @@
+MAXATTEMPTS=10
+
 ### smallutils
 
 smallyes() {
@@ -241,6 +243,13 @@
 }
 
 get () {
+	for iters in $(seq 1 $MAXATTEMPTS); do
+		if single_get $@; then break; fi
+		warning RETRYING Retrying failed download.
+	done
+}
+
+single_get () {
 	# args: from dest 'nocache'
 	# args: from dest [checksum size] [alt {checksum size type}]
 	local displayname
@@ -331,13 +340,6 @@
 		# http/ftp mirror
 		if wgetprogress -O $dest $from; then
 			return 0
-		elif [ -s $dest ]; then
-			local iters=0
-			while [ $iters -lt 3 ]; do
-warning RETRYING Retrying failed download of %s $from
-if wgetprogress -c -O $dest $from; then break; fi
-iters=$(($iters + 1))
-			done
 		else
 			rm -f $dest
 			return 1
@@ -346,13 +348,6 @@
 		# http/ftp mirror
 		if wgetprogress $CHECKCERTIF $CERTIFICATE $PRIVATEKEY -O $dest $from; then
 			return 0
-		elif [ -s $dest ]; then
-			local iters=0
-			while [ $iters -lt 3 ]; do
-warning RETRYING Retrying failed download of %s $from
-if wgetprogress $CHECKCERTIF $CERTIFICATE $PRIVATEKEY -c -O $dest $from; then break; fi
-iters=$(($iters + 1))
-			done
 		else
 			rm -f $dest
 			return 1
---End Message---
---BeginMessage---
Source: debootstrap
Source-Version: 1.0.39

We believe that the bug you reported is fixed in the latest version of
debootstrap, which is due to be installed in the Debian FTP archive:

debootstrap-udeb_1.0.39_all.udeb
  to main/d/debootstrap/debootstrap-udeb_1.0.39_all.udeb
debootstrap_1.0.39.dsc
  to main/d/debootstrap/debootstrap_1.0.39.dsc
debootstrap_1.0.39.tar.gz
  to main/d/debootstrap/debootstrap_1.0.39.tar.gz
debootstrap_1.0.39_all.deb
  to main/d/debootstrap/debootstrap_1.0.39_all.deb



A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 618...@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Colin Watson cjwat...@debian.org (supplier of updated debootstrap package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmas...@debian.org)


-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

Format: 1.8
Date: Tue, 13 Mar 2012 17:21:13 +
Source: debootstrap
Binary: debootstrap debootstrap-udeb
Architecture: source all
Version: 1.0.39
Distribution: unstable
Urgency: low
Maintainer: Debian Install System

Bug#618920:

2012-03-13 Thread unitedcabletv 




--
To UNSUBSCRIBE, email to debian-boot-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/vmgtpwgqglnv.rcywc...@smtp.gmail.com

Bug#269656: Bug#618920: debootstrap: needs more robust download error handling

2012-03-13 Thread Michael Gilbert 
On Tue, Mar 13, 2012 at 1:06 PM, Colin Watson wrote:
 I've committed my improved version, which you're welcome to review:

Cool!  Thanks for looking into this.  This problem has reared its head
on me too many times to count over the past few years ;)

Thanks again!
Mike



-- 
To UNSUBSCRIBE, email to debian-boot-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: 
http://lists.debian.org/CANTw=MMbHx+zma5Vxz5wZq_Vwif=ml9u4_zm-8v9vogfqnr...@mail.gmail.com

Bug#618920: debootstrap: needs more robust download error handling

2011-03-19 Thread Michael Gilbert 
package: debootstrap
version: 1.0.28
severity: important
tags: patch

debootstrap's current download error handling isn't very robust.  It
declares success just for the presence of a downloaded file, which may
be a partial download, or one for which the checksum doesn't match.
Eventually those conditions will lead to unhandled failures elsewhere
within debootstrap.

This can make d-i very rocky on certain mirrors since one bad package
download ultimately lead to a complete failure of the debootstrap
process, and thus a failed install.  An expert can recover from this,
but an average user will get rather frustrated (especially since the
dialogs for debootstrap errors are rather confusing).  

It may also be useful to expand a bit on these d-i debootstrap error
messages: when an error happens, the right answer that the user wants is
to hit 'go back' twice in a row to start the debootstrap all over again,
but the dialogs are confusing, and 'continue' seems to be the obvious
choice, but that will lead to the broken debootstrap continuing to
completion with various brokenness. Anyway, that maybe should be
submitted as another bug.

So, back to the original issue, I've created a patch that will retry
downloads whenever anything in the get routine fails, which I
believe is much more robust than the current situation.  Please see
attached patch.

Best wishes,
Mike
--- newhd/source/debootstrap-1.0.28/functions	2011-02-21 19:25:08.0 -0500
+++ /usr/share/debootstrap/functions	2011-03-19 10:58:57.0 -0400
@@ -1,3 +1,5 @@
+MAXATTEMPTS=10
+
 ### smallutils
 
 smallyes() {
@@ -241,6 +243,13 @@
 }
 
 get () {
+	for iters in $(seq 1 $MAXATTEMPTS); do
+		if single_get $@; then break; fi
+		warning RETRYING Retrying failed download.
+	done
+}
+
+single_get () {
 	# args: from dest 'nocache'
 	# args: from dest [checksum size] [alt {checksum size type}]
 	local displayname
@@ -331,13 +340,6 @@
 		# http/ftp mirror
 		if wgetprogress -O $dest $from; then
 			return 0
-		elif [ -s $dest ]; then
-			local iters=0
-			while [ $iters -lt 3 ]; do
-warning RETRYING Retrying failed download of %s $from
-if wgetprogress -c -O $dest $from; then break; fi
-iters=$(($iters + 1))
-			done
 		else
 			rm -f $dest
 			return 1
@@ -346,13 +348,6 @@
 		# http/ftp mirror
 		if wgetprogress $CHECKCERTIF $CERTIFICATE $PRIVATEKEY -O $dest $from; then
 			return 0
-		elif [ -s $dest ]; then
-			local iters=0
-			while [ $iters -lt 3 ]; do
-warning RETRYING Retrying failed download of %s $from
-if wgetprogress $CHECKCERTIF $CERTIFICATE $PRIVATEKEY -c -O $dest $from; then break; fi
-iters=$(($iters + 1))
-			done
 		else
 			rm -f $dest
 			return 1

Bug#618920: debootstrap: needs more robust download error handling

2011-03-19 Thread Michael Gilbert 
I probably should have mentioned that these errors are much more
likely within d-i for some reason.  The best way to reproduce this is
to do an installation up through partitioning, then do:

$ debootstrap wheezy /target/wheezy-chroot
http://snapshot.debian.org/archive/debian/20110318T093210Z

At least a few package downloads should fail from that mirror, and
they'll be different every time.

Executing the same command in a full system seems to work fine for
that exact same mirror, so maybe its some other variability within d-i
that is the cause of this issue.

Best wishes,
Mike



-- 
To UNSUBSCRIBE, email to debian-boot-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: 
http://lists.debian.org/AANLkTinic9TK-G7pu+=a5aafaboifjhzhwhirebns...@mail.gmail.com