[pacman-dev] Duplicated lines of "downloading foo.db..." with --noprogressbar

[Andrew Gregory]

On 05/04/15 at 02:47pm, Allan McRae wrote:
> On 23/04/15 06:27, David Macek wrote:
> > Hello everyone. I see a problem with --noprogressbar output, but it seems to be dependent on server/network. Can anyone confirm with the following settings?
> > 
> > # append to pacman.conf
> > [mingw64]
> > Server = ftp://ftp.heanet.ie/mirrors/download.sourceforge.net/pub/sourceforge/m/ms/msys2/REPOS/MINGW/x86_64
> 
> Don't let Dave see you using ftp!
> 
> > 
> > $ pacman -Syy --noprogressbar
> > 
> > My output contains multiple "downloading" lines for this DB:
> > 
> > downloading mingw64.db...
> > downloading mingw64.db...
> > downloading mingw64.db...
> > downloading mingw64.db...
> > downloading mingw64.db...
> > 
> 
> allan at arya ~
> $ pacman -Syy --noprogressbar
> :: Synchronizing package databases...
> downloading staging.db...
> downloading testing.db...
> downloading core.db...
> downloading extra.db...
> downloading extra.db...
> downloading extra.db...
> downloading extra.db...
> downloading extra.db...
> downloading community-testing.db...
> downloading community.db...
> downloading multilib-staging.db...
> downloading multilib-testing.db...
> downloading multilib.db...
> 
> I can replicate using the Arch dev server (which is really slow from here).
> 
> > It seems that the cause is libcurl calling the progress callback
> > multiple times before the first bytes get downloaded, which then
> > in turns `cb_dl_progress` multiple times with `file_xfered` = 0,
> > which then prints the message each time.
> > 
> > I don't want to just complain, so I whipped up a quick patch.
> > I can submit it properly if it gets an upvote.
> > 
> > diff --git a/src/pacman/callback.c b/src/pacman/callback.c
> > index 695e38d..3bc43b7 100644
> > --- a/src/pacman/callback.c
> > +++ b/src/pacman/callback.c
> > @@ -628,6 +628,7 @@ void cb_dl_progress(const char *filename, off_t file_xfered, off_t file_total)
> >  	static double rate_last;
> >  	static off_t xfered_last;
> >  	static int64_t initial_time = 0;
> > +	static char *filename_last = NULL;
> >  	int infolen;
> >  	int filenamelen;
> >  	char *fname, *p;
> > @@ -646,7 +647,11 @@ void cb_dl_progress(const char *filename, off_t file_xfered, off_t file_total)
> >  	const unsigned short cols = getcols();
> >  
> >  	if(config->noprogressbar || cols == 0 || file_total == -1) {
> > -		if(file_xfered == 0) {
> > +		if(filename_last != NULL || strcmp(filename_last, filename) != 0) {
> > +			if(filename_last != NULL) {
> > +				free(filename_last);
> > +			}
> > +			filename_last = strdup(filename);
> >  			printf(_("downloading %s...\n"), filename);
> >  			fflush(stdout);
> >  		}
> > 
> 
> This seems a reasonable solution.

I don't think this is the right solution.  The comments in
dload_progress_cb suggest that the callback is not intended to be
called until we have actually downloaded something. So, file_xfered
should always equal 0 exactly once per downloaded file.  I think it
makes more sense to fix dload_progress_cb.  We might also consider
using the event callback rather than the download callback when
noprogressbar is set.

apg