On 11.01.2013 11:00, 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.

Shorten that a bit and it would be a perfect commit comment. It should explain the reasoning behind the change and a way to reproduce the original problem. If people want to read the whole story there is an ML archive. Maybe something like this:

Redirections with a body (e.g. mirrorbrain) should trigger the progress bar only for the package download. Without this patch you get two bars (redirection and download).

You can also be more precise in the subject (which is probably more important than the body): "Hide download progress bar for redirection with body"

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); + if (respcode == 302 && current_size > 0) { + return 0; + } +

Please use git send-email[1] in the future. Most other clients will mangle indentation and/or line breaks for long lines. Also please read HACKING (in the top level of the git repo). "if" should be followed by the brace without a space in between. [1]: https://wiki.archlinux.org/index.php/Super_Quick_Git_Guide#Sending_patches

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

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;