On Tuesday, August 2, 2016 11:27:28 AM CEST Matthew White wrote: > On Mon, 01 Aug 2016 16:32:30 +0200 > > Tim Ruehsen <[email protected]> wrote: > > On Saturday, July 30, 2016 9:41:48 PM CEST Matthew White wrote: > > > Hello! > > > > > > I think that sometimes it could help to keep downloaded Metalink's files > > > which have a bad hash. > > > > > > The default wget behaviour is to delete such files. > > > > > > This patch provides a way to keep files which have a bad hash through > > > the > > > option --keep-badhash. It appends the suffix .badhash to the file name, > > > except without overwriting existing files. In the latter case, an unique > > > suffix is appended after .badhash. > > > > > > I made this patch working on the following branch: > > > master (latest 20cac2c5ab3d63aacfba35fb10878a2d490e2377) > > > git://git.savannah.gnu.org/wget.git > > > > > > What do you think? > > > > Hi Matthew, > > > > good work ! > > > > While your FSF assignment is underway (PM), we can continue polishing. > > > > Didn't test your code yet, but from what I see, there are just these > > peanuts: > > > > 1. > > + bhash = malloc (strlen (name) + strlen (".badhash") + 1); > > + strcat (strcpy (bhash, name), ".badhash"); > > > > Please consider using concat_strings() from util.h. > > And please leave an empty line between var declaration and code - just for > > readability. > > > > 2. > > Please add 'link' and 'unlink' to bootstrap.conf. Gnulib will emulate > > these on platforms where they are not available (or have a slightly > > different behavior). I guess we simply forgot 'unlink' when we switched > > to gnulib. > > > > 3. > > The (GNU) commit messages should ideally look like: > > > > one line of short description > > <empty line> > > file changes > > <empty line> > > long description > > > > For example: > > Add new option --keep-badhash > > > > * src/init.c: Add keepbadhash > > * src/main.c: Add keep-badhash > > * src/options.h: Add keep_badhash > > * doc/wget.texi: Add docs for --keep-badhash > > * src/metalink.h: Add prototypes badhash_suffix(), badhash_or_remove() > > * src/metalink.c: New functions badhash_suffix(), badhash_or_remove(). > > > > (retrieve_from_metalink): Call append .badhash() > > > > With --keep-badhash, append .badhash to Metalink's files with checksum > > mismatch, except without overwriting existing files. > > > > Without --keep-badhash, remove downloaded files with checksum mismatch > > (this conforms to the old behaviour). > > > > [This also applies to to your other patches] > > > > 4. > > Please not only write 'keepbadhash', but also what you did (e.g. remove, > > rename, add, ...), see my example above. > > > > Those ChangeLog entries should allow finding changes / faults / > > regressions > > etc. even when a versioning system is not at hand, e.g. when unpacking the > > sources from a tarball. (Not debatable that consulting commit messages > > directly is much more powerful.) > > > > 5. > > You write "except without overwriting existing files", maybe you should > > mention appending a counter instead of overwriting existent files !? > > > > > > Regards, Tim > > Thanks Tim! > > I really needed your guidance. I sent the modified patches to > [email protected]. > > I believe there are more things to fix. > > Consider the following after applying the attached patch: > * src/metalink.c (retrieve_from_metalink): line 124: If the download is > interrupted (output_stream isn't NULL), and there are more urls for the > same file (we are still in the download loop), switching to the next url > should resume the download (instead than start it over). > > In this patch I added a fix to rename/remove the fully downloaded file > (output_stream is NULL), but there was an error (probably checksum) and we > are still in the download loop (more urls for the same file). But as said > before, switching to the next url should continue the download: * > src/metalink.c (retrieve_from_metalink): line 131: Rename/remove fully > downloaded file on error > > I still have to investigate the problem...
So, I wait with 0001-... !? 0002-... has been pushed (extended with 'symlink'). Thanks for your contribution. Tim
signature.asc
Description: This is a digitally signed message part.
