Re: [Bug-wget] [PATCH 04/25] Bugfix: Keep the download progress when alternating metalink:url

2016-09-13 Thread Matthew White
On Sun, 11 Sep 2016 22:04:41 +0200
Giuseppe Scrivano  wrote:

> Matthew White  writes:
> 
> > From f9fc03c0788675275041d0876d3e7fffd3f50eee Mon Sep 17 00:00:00 2001
> > From: Matthew White 
> > Date: Thu, 4 Aug 2016 11:35:42 +0200
> > Subject: [PATCH 04/25] Bugfix: Keep the download progress when alternating
> >  metalink:url
> >
> > * src/metalink.c (retrieve_from_metalink): On download error, resume
> >   output_stream with the next mres->url. Keep fully downloaded files
> >   started with --continue, otherwise rename/remove the file
> >
> > Before this patch, with --continue, existing and/or fully retrieved
> > files which fail the sanity tests were renamed (--keep-badhash), or
> > removed.
> >
> > This patch ensures that --continue doesn't rename/remove existing
> > and/or fully retrieved files which fail the sanity tests.
> > ---
> 
> ACK

Patch 12/25 is now squased in:
http://lists.gnu.org/archive/html/bug-wget/2016-09/msg00086.html

The series of patches is now all amended/rebased. There are still changes to do.

Posting after final decisions are taken about open topics in this series of 
patches.

> 
> Giuseppe

Regards,
Matthew

-- 
Matthew White 


pgp1UbwBOQybg.pgp
Description: PGP signature


Re: [Bug-wget] [PATCH 04/25] Bugfix: Keep the download progress when alternating metalink:url

2016-09-11 Thread Giuseppe Scrivano
Matthew White  writes:

> From f9fc03c0788675275041d0876d3e7fffd3f50eee Mon Sep 17 00:00:00 2001
> From: Matthew White 
> Date: Thu, 4 Aug 2016 11:35:42 +0200
> Subject: [PATCH 04/25] Bugfix: Keep the download progress when alternating
>  metalink:url
>
> * src/metalink.c (retrieve_from_metalink): On download error, resume
>   output_stream with the next mres->url. Keep fully downloaded files
>   started with --continue, otherwise rename/remove the file
>
> Before this patch, with --continue, existing and/or fully retrieved
> files which fail the sanity tests were renamed (--keep-badhash), or
> removed.
>
> This patch ensures that --continue doesn't rename/remove existing
> and/or fully retrieved files which fail the sanity tests.
> ---

ACK

Giuseppe



[Bug-wget] [PATCH 04/25] Bugfix: Keep the download progress when alternating metalink:url

2016-09-10 Thread Matthew White
[Coverity Scan is ok, make syntax-check is ok, make check-valgrind is ok, 
contrib/check-hard is ok]

This serves a double purpose:
- when a Metalink resource fails, keep the download progress and continue with 
the next resource;
- when --continue is used, retain the existing file or the just fully 
downloaded file if the file verification fails.

The following description is verbatim from the patch:
-
Before this patch, with --continue, existing and/or fully retrieved
files which fail the sanity tests were renamed (--keep-badhash), or
removed.

This patch ensures that --continue doesn't rename/remove existing
and/or fully retrieved files which fail the sanity tests.
-

Regards,
Matthew

-- 
Matthew White 
>From f9fc03c0788675275041d0876d3e7fffd3f50eee Mon Sep 17 00:00:00 2001
From: Matthew White 
Date: Thu, 4 Aug 2016 11:35:42 +0200
Subject: [PATCH 04/25] Bugfix: Keep the download progress when alternating
 metalink:url

* src/metalink.c (retrieve_from_metalink): On download error, resume
  output_stream with the next mres->url. Keep fully downloaded files
  started with --continue, otherwise rename/remove the file

Before this patch, with --continue, existing and/or fully retrieved
files which fail the sanity tests were renamed (--keep-badhash), or
removed.

This patch ensures that --continue doesn't rename/remove existing
and/or fully retrieved files which fail the sanity tests.
---
 src/metalink.c | 62 +-
 1 file changed, 40 insertions(+), 22 deletions(-)

diff --git a/src/metalink.c b/src/metalink.c
index 4a0a56e..956070d 100644
--- a/src/metalink.c
+++ b/src/metalink.c
@@ -96,12 +96,14 @@ retrieve_from_metalink (const metalink_t* metalink)
  1 -> verified successfully  */
   char sig_status = 0;
 
+  bool skip_mfile = false;
+
   output_stream = NULL;
 
   DEBUGP (("Processing metalink file %s...\n", quote (mfile->name)));
 
   /* Resources are sorted by priority.  */
-  for (mres_ptr = mfile->resources; *mres_ptr; mres_ptr++)
+  for (mres_ptr = mfile->resources; *mres_ptr && !skip_mfile; mres_ptr++)
 {
   metalink_resource_t *mres = *mres_ptr;
   metalink_checksum_t **mchksum_ptr, *mchksum;
@@ -117,26 +119,30 @@ retrieve_from_metalink (const metalink_t* metalink)
   continue;
 }
 
-  retr_err = METALINK_RETR_ERROR;
-
-  /* If output_stream is not NULL, then we have failed on
- previous resource and are retrying. Thus, rename/remove
- the file.  */
-  if (output_stream)
+  /* The file is fully downloaded, but some problems were
+ encountered (checksum failure?).  The loop had been
+ continued to switch to the next url.  */
+  if (output_stream && retr_err == RETROK)
 {
+  /* Do not rename/remove a continued file. Skip it.  */
+  if (opt.always_rest)
+{
+  skip_mfile = true;
+  continue;
+}
+
   fclose (output_stream);
   output_stream = NULL;
   badhash_or_remove (filename);
   xfree (filename);
 }
-  else if (filename)
+  else if (!output_stream && filename)
 {
-  /* Rename/remove the file downloaded previously before
- downloading it again.  */
-  badhash_or_remove (filename);
   xfree (filename);
 }
 
+  retr_err = METALINK_RETR_ERROR;
+
   /* Parse our resource URL.  */
   iri = iri_new ();
   set_uri_encoding (iri, opt.locale, true);
@@ -156,17 +162,29 @@ retrieve_from_metalink (const metalink_t* metalink)
   /* Avoid recursive Metalink from HTTP headers.  */
   bool _metalink_http = opt.metalink_over_http;
 
-  /* Assure proper local file name regardless of the URL
- of particular Metalink resource.
- To do that we create the local file here and put
- it as output_stream. We restore the original configuration
- after we are finished with the file.  */
-  if (opt.always_rest)
-/* continue previous download */
-output_stream = fopen (mfile->name, "ab");
+  /* If output_stream is not NULL, then we have failed on
+ previous resource and are retrying. Thus, continue
+ with the next resource.  Do not close output_stream
+ while iterating over the resources, or the download
+ progress will be lost.  */
+  if (output_stream)
+{
+  DEBUGP (("Previous resource failed, continue with next resource.\n"));
+}
   else
-/* create a file with an unique name */
-output_stream = uniqu