[pacman-dev] [PATCH] Not trigger twice progressbar on some redirections

[Dave Reisner]

On Fri, Jan 11, 2013 at 11:00:29AM +0100, Alexandre Filgueira wrote:
> I found that pacman was triggerin the progressbar callback twice when
> pointing to my repo, which uses Mirrorbrain to geolocate the closest mirror.
> After asking here in this list and in the curl library list, the thing was
> that my header redirection response had a body, so curl was triggering
> twice, downloading first just bytes (of the redirection) and then the
> proper file.
> I changed the dload.c code to avoid this, I don't know if it is correct to
> do it the way I did. Now is triggering just once.
> 
> This is my modifications:
> 
> --- dload_original.c 2013-01-11 10:48:21.751443000 +0100
> +++ dload_new.c 2013-01-11 10:44:13.049630000 +0100
> @@ -95,6 +95,8 @@
>  {
>   struct dload_payload *payload = (struct dload_payload *)file;
>   off_t current_size, total_size;
> + alpm_handle_t *handle = payload->handle;
> + CURL *curl = get_libcurl_handle(handle);
> 
>   /* SIGINT sent, abort by alerting curl */
>   if(dload_interrupted) {
> @@ -103,6 +105,13 @@
> 
>   current_size = payload->initial_size + (off_t)dlnow;
> 
> + /* not show progressbar when redirection has body*/
> + long respcode = 0;
> + curl_easy_getinfo (curl, CURLINFO_RESPONSE_CODE, &respcode);

Please add this to the payload struct itself. We read the response code
in order to validate that the transfer was a success, so it's worth
holding on to. If possible, I'd also opt to read this off the curl
handle in the header callback, not in the progress function. I say if
possible because I'm not sure in what order the callbacks are called.

> + if (respcode == 302 && current_size > 0) {

This is insufficient since 301s are valid redirects as well. I would go
even further and say that for any response >= 300, it isn't worth trying
to show a progress bar.

> + return 0;
> + }
> +
>   /* is our filesize still under any set limit? */
>   if(payload->max_size && current_size > payload->max_size) {
>   dload_interrupted = ABORT_OVER_MAXFILESIZE;
>