[pacman-dev] [PATCH 1/3] dload_progress_cb: cast values to off_t up front
We only ever use them as off_t, so do the conversion early and avoid the need for multiple casts and DOUBLE_EQ. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- lib/libalpm/dload.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index c5186be..cae4b7c 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -94,7 +94,7 @@ static int dload_progress_cb(void *file, double dltotal, double dlnow, double UNUSED ultotal, double UNUSED ulnow) { struct dload_payload *payload = (struct dload_payload *)file; - off_t current_size, total_size; + off_t current_size, total_size, xfer = dlnow, total = dltotal; /* avoid displaying progress bar for redirects with a body */ if(payload->respcode >= 300) { @@ -106,7 +106,7 @@ static int dload_progress_cb(void *file, double dltotal, double dlnow, return 1; } - current_size = payload->initial_size + (off_t)dlnow; + current_size = payload->initial_size + xfer; /* is our filesize still under any set limit? */ if(payload->max_size && current_size > payload->max_size) { @@ -119,21 +119,21 @@ static int dload_progress_cb(void *file, double dltotal, double dlnow, return 0; } - total_size = payload->initial_size + (off_t)dltotal; + total_size = payload->initial_size + total; - if(DOUBLE_EQ(dltotal, 0.0) || payload->prevprogress == total_size) { + if(total == 0 || payload->prevprogress == total_size) { return 0; } /* initialize the progress bar here to avoid displaying it when * a repo is up to date and nothing gets downloaded */ if(payload->prevprogress == 0) { - payload->handle->dlcb(payload->remote_name, 0, (off_t)dltotal); + payload->handle->dlcb(payload->remote_name, 0, total); } /* do NOT include initial_size since it wasn't part of the package's * download_size (nor included in the total download size callback) */ - payload->handle->dlcb(payload->remote_name, (off_t)dlnow, (off_t)dltotal); + payload->handle->dlcb(payload->remote_name, xfer, total); payload->prevprogress = current_size; -- 2.2.1
Move the final call outside of curl's progress callback so that we can notify our callback of completion even if the download size was unknown or we encounter an error. Forces download total >= 0 within the callback so that it can be used to indicate a download error. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- lib/libalpm/dload.c | 34 +++++++++++++++++++++------------- src/pacman/callback.c | 8 +++++++- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index cae4b7c..677cb64 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -94,7 +94,7 @@ static int dload_progress_cb(void *file, double dltotal, double dlnow, double UNUSED ultotal, double UNUSED ulnow) { struct dload_payload *payload = (struct dload_payload *)file; - off_t current_size, total_size, xfer = dlnow, total = dltotal; + off_t current_size, xfer = dlnow, total = dltotal > 0.0 ? dltotal : 0; /* avoid displaying progress bar for redirects with a body */ if(payload->respcode >= 300) { @@ -119,14 +119,13 @@ static int dload_progress_cb(void *file, double dltotal, double dlnow, return 0; } - total_size = payload->initial_size + total; - - if(total == 0 || payload->prevprogress == total_size) { + /* the initial and final calls are explicitly made elsewhere */ + if(total == 0 || xfer == 0 || xfer == total) { return 0; } - /* initialize the progress bar here to avoid displaying it when - * a repo is up to date and nothing gets downloaded */ + /* initialize the progress callback here to avoid calling it + * when nothing actually gets downloaded */ if(payload->prevprogress == 0) { payload->handle->dlcb(payload->remote_name, 0, total); } @@ -463,6 +462,22 @@ static int curl_download_internal(struct dload_payload *payload, /* perform transfer */ payload->curlerr = curl_easy_perform(curl); + curl_easy_getinfo(curl, CURLINFO_FILETIME, &remote_time); + curl_easy_getinfo(curl, CURLINFO_CONTENT_LENGTH_DOWNLOAD, &remote_size); + curl_easy_getinfo(curl, CURLINFO_SIZE_DOWNLOAD, &bytes_dl); + curl_easy_getinfo(curl, CURLINFO_CONDITION_UNMET, &timecond); + curl_easy_getinfo(curl, CURLINFO_EFFECTIVE_URL, &effective_url); + + if(payload->prevprogress > 0) { + /* if we called the callback, notify it that we're done */ + off_t xfer = bytes_dl, size = remote_size > 0 ? remote_size : bytes_dl; + if(payload->curlerr == CURLE_OK && payload->respcode < 400 && xfer == size) { + handle->dlcb(payload->remote_name, xfer, size); + } else { + handle->dlcb(payload->remote_name, -1, -1); + } + } + _alpm_log(handle, ALPM_LOG_DEBUG, "curl returned error %d from transfer\n", payload->curlerr); @@ -519,13 +534,6 @@ static int curl_download_internal(struct dload_payload *payload, goto cleanup; } - /* retrieve info about the state of the transfer */ - curl_easy_getinfo(curl, CURLINFO_FILETIME, &remote_time); - curl_easy_getinfo(curl, CURLINFO_CONTENT_LENGTH_DOWNLOAD, &remote_size); - curl_easy_getinfo(curl, CURLINFO_SIZE_DOWNLOAD, &bytes_dl); - curl_easy_getinfo(curl, CURLINFO_CONDITION_UNMET, &timecond); - curl_easy_getinfo(curl, CURLINFO_EFFECTIVE_URL, &effective_url); - if(final_url != NULL) { *final_url = effective_url; } diff --git a/src/pacman/callback.c b/src/pacman/callback.c index 0d87fd3..3b3a74f 100644 --- a/src/pacman/callback.c +++ b/src/pacman/callback.c @@ -645,7 +645,13 @@ 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_total == -1) { + /* an error occurred, move to the next line for the error message */ + putchar('\n'); + return; + } + + if(config->noprogressbar || cols == 0) { if(file_xfered == 0) { printf(_("downloading %s...\n"), filename); fflush(stdout); -- 2.2.1
Previously the download progress callback would never be called if servers failed to set Content-Length, preventing any output from being shown to the user even though the download was still successfully performed. Instead, show an indeterminate progress bar until the download is complete, at which point it will be filled in. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- Issue discussed here: https://bbs.archlinux.org/viewtopic.php?id=192604 lib/libalpm/dload.c | 2 +- src/pacman/callback.c | 61 +++++++++++++++++++++++++++++++++++++-------------- 2 files changed, 45 insertions(+), 18 deletions(-) diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index 677cb64..d42ca1f 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -120,7 +120,7 @@ static int dload_progress_cb(void *file, double dltotal, double dlnow, } /* the initial and final calls are explicitly made elsewhere */ - if(total == 0 || xfer == 0 || xfer == total) { + if(xfer == 0 || xfer == total) { return 0; } diff --git a/src/pacman/callback.c b/src/pacman/callback.c index 3b3a74f..01e8f39 100644 --- a/src/pacman/callback.c +++ b/src/pacman/callback.c @@ -94,6 +94,33 @@ static int64_t get_update_timediff(int first_call) return retval; } +static void fill_progress_step(const int step, const int proglen) +{ + /* 8 = 1 space + 1 [ + 1 ] + 5 for percent */ + const int hashlen = proglen - 8; + + if(hashlen > 0) { + int i, pos = step % hashlen, len = hashlen / 10 + 1; + fputs(" [", stdout); + for(i = 0; i < hashlen; i++) { + if(i >= pos && i < pos + len) { + putchar('#'); + } else { + putchar('-'); + } + } + putchar(']'); + } + /* print display percent after progress bar */ + /* 5 = 1 space + 3 digits + 1 % */ + if(proglen >= 5) { + fputs(" ???%", stdout); + } + putchar('\r'); + + fflush(stdout); +} + /* refactored from cb_trans_progress */ static void fill_progress(const int bar_percent, const int disp_percent, const int proglen) @@ -628,6 +655,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 int step = 0; int infolen; int filenamelen; char *fname, *p; @@ -641,7 +669,7 @@ void cb_dl_progress(const char *filename, off_t file_xfered, off_t file_total) unsigned int eta_h = 0, eta_m = 0, eta_s = 0; double rate_human, xfered_human; const char *rate_label, *xfered_label; - int file_percent = 0, total_percent = 0; + int total_percent = 0; const unsigned short cols = getcols(); @@ -684,7 +712,7 @@ void cb_dl_progress(const char *filename, off_t file_xfered, off_t file_total) } /* bogus values : stop here */ - if(xfered > total || xfered < 0) { + if((total && xfered > total) || xfered < 0) { return; } @@ -699,6 +727,7 @@ void cb_dl_progress(const char *filename, off_t file_xfered, off_t file_total) rate_last = 0.0; get_update_timediff(1); } + step = 0; } else if(file_xfered == file_total) { /* compute final values */ int64_t timediff = get_time_ms() - initial_time; @@ -729,15 +758,8 @@ void cb_dl_progress(const char *filename, off_t file_xfered, off_t file_total) xfered_last = xfered; } - if(file_total) { - file_percent = (file_xfered * 100) / file_total; - } else { - file_percent = 100; - } - if(totaldownload) { - total_percent = ((list_xfered + file_xfered) * 100) / - list_total; + total_percent = ((list_xfered + file_xfered) * 100) / list_total; /* if we are at the end, add the completed file to list_xfered */ if(file_xfered == file_total) { @@ -817,21 +839,26 @@ void cb_dl_progress(const char *filename, off_t file_xfered, off_t file_total) printf("%6.1f %3s %4.f%c/s ", xfered_human, xfered_label, rate_human, rate_label[0]); } - if(eta_h == 0) { + if(file_total == 0 || eta_h >= 100) { + fputs("--:--", stdout); + } else if(eta_h == 0) { printf("%02u:%02u", eta_m, eta_s); - } else if(eta_h < 100) { - printf("%02u:%02u:%02u", eta_h, eta_m, eta_s); } else { - fputs("--:--", stdout); + printf("%02u:%02u:%02u", eta_h, eta_m, eta_s); } free(fname); free(wcfname); - if(totaldownload) { - fill_progress(file_percent, total_percent, cols - infolen); + if(file_total == 0) { + fill_progress_step(step++, cols - infolen); } else { - fill_progress(file_percent, file_percent, cols - infolen); + int file_percent = (file_xfered * 100) / file_total; + if(totaldownload) { + fill_progress(file_percent, total_percent, cols - infolen); + } else { + fill_progress(file_percent, file_percent, cols - infolen); + } } return; } -- 2.2.1
On Jan 25, 2015 12:25 PM, "Andrew Gregory" <andrew.gregory.8@gmail.com> wrote:
We only ever use them as off_t, so do the conversion early and avoid the need for multiple casts and DOUBLE_EQ.
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- lib/libalpm/dload.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index c5186be..cae4b7c 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -94,7 +94,7 @@ static int dload_progress_cb(void *file, double
I'd rather we use the newer XFERINFOFUNCTION (might have the name wrong) which passes these as curl_off_t instead of double. dltotal, double dlnow,
double UNUSED ultotal, double UNUSED ulnow) { struct dload_payload *payload = (struct dload_payload *)file; - off_t current_size, total_size; + off_t current_size, total_size, xfer = dlnow, total = dltotal;
/* avoid displaying progress bar for redirects with a body */ if(payload->respcode >= 300) { @@ -106,7 +106,7 @@ static int dload_progress_cb(void *file, double
dltotal, double dlnow,
return 1; }
- current_size = payload->initial_size + (off_t)dlnow; + current_size = payload->initial_size + xfer;
/* is our filesize still under any set limit? */ if(payload->max_size && current_size > payload->max_size) { @@ -119,21 +119,21 @@ static int dload_progress_cb(void *file, double
dltotal, double dlnow,
return 0; }
- total_size = payload->initial_size + (off_t)dltotal; + total_size = payload->initial_size + total;
- if(DOUBLE_EQ(dltotal, 0.0) || payload->prevprogress ==
+ if(total == 0 || payload->prevprogress == total_size) { return 0; }
/* initialize the progress bar here to avoid displaying it when * a repo is up to date and nothing gets downloaded */ if(payload->prevprogress == 0) { - payload->handle->dlcb(payload->remote_name, 0, (off_t)dltotal); + payload->handle->dlcb(payload->remote_name, 0, total); }
/* do NOT include initial_size since it wasn't part of the
total_size) { package's
* download_size (nor included in the total download size
callback) */
- payload->handle->dlcb(payload->remote_name, (off_t)dlnow, (off_t)dltotal); + payload->handle->dlcb(payload->remote_name, xfer, total);
payload->prevprogress = current_size;
-- 2.2.1
participants (2)
-
Andrew Gregory
-
Dave Reisner