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

[Allan McRae]

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.

You leak the final filename_last, otherwise fine.

A