[pacman-dev] [PATCH] Implement TotalDownload functionality
Allan McRae
allan at archlinux.org
Sun Dec 6 08:02:22 UTC 2020
On 6/12/20 4:39 am, Anatol Pomozov wrote:
> With the recent 'multibar' interface changes TotalDownload has been disabled.
> Now we have a new UI and we need to find another way to display this
> information.
>
> When 'TotalDownload' config option is enabled we are going to have an extra
> progress bar at the bottom of the screen that shows how much of the entire
> download has been completed.
>
> Closes FS#68202
Thanks - that progress bar looks good.
>
> Signed-off-by: Anatol Pomozov <anatol.pomozov at gmail.com>
> ---
> doc/pacman.conf.5.asciidoc | 6 +--
> lib/libalpm/sync.c | 5 --
> src/pacman/callback.c | 108 ++++++++++++++++++++++++++++++++-----
> 3 files changed, 96 insertions(+), 23 deletions(-)
>
> diff --git a/doc/pacman.conf.5.asciidoc b/doc/pacman.conf.5.asciidoc
> index dd1d3d5e..32612eb9 100644
> --- a/doc/pacman.conf.5.asciidoc
> +++ b/doc/pacman.conf.5.asciidoc
> @@ -191,10 +191,8 @@ Options
> not support escape characters.
>
> *TotalDownload*::
> - When downloading, display the amount downloaded, download rate, ETA,
> - and completed percentage of the entire download list rather
> - than the percent of each individual download target. The progress
> - bar is still based solely on the current file download.
> + When downloading, display an extra progress bar with the amount downloaded,
> + download rate, ETA, and completed percentage of the entire download list.
> This option won't work if XferCommand is used.
OK.
>
> *CheckSpace*::
> diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
> index 5d8652a5..e25e56d4 100644
> --- a/lib/libalpm/sync.c
> +++ b/lib/libalpm/sync.c
> @@ -846,11 +846,6 @@ finish:
> pkg->download_size = 0;
> }
>
> - /* clear out value to let callback know we are done */
> - if(handle->totaldlcb) {
> - handle->totaldlcb(0);
> - }
> -
OK.
> return ret;
> }
>
> diff --git a/src/pacman/callback.c b/src/pacman/callback.c
> index 12ab952f..49bc3df4 100644
> --- a/src/pacman/callback.c
> +++ b/src/pacman/callback.c
> @@ -40,8 +40,9 @@
> #include "conf.h"
>
> /* download progress bar */
> -static off_t list_xfered = 0.0;
Appears that should have been cleaned up a while ago?
> +static int total_enabled = 0;
> static off_t list_total = 0.0;
> +static struct pacman_progress_bar *totalbar;
>
OK
> /* delayed output during progress bar */
> static int on_progress = 0;
> @@ -92,6 +93,10 @@ struct pacman_multibar_ui {
>
> struct pacman_multibar_ui multibar_ui = {0};
>
> +static int dload_progressbar_enabled(void);
> +static void init_total_progressbar(void);
> +static void update_total_finalstats(void);
> +
Why do these need forward declared? Just to avoid moving code around in
the file?
> void multibar_move_completed_up(bool value) {
> multibar_ui.move_completed_up = value;
> }
> @@ -327,6 +332,10 @@ void cb_event(alpm_event_t *event)
> case ALPM_EVENT_PKG_RETRIEVE_START:
> colon_printf(_("Retrieving packages...\n"));
> on_progress = 1;
> + total_enabled = config->totaldownload && list_total;
> + if(total_enabled) {
> + init_total_progressbar();
> + }
OK
> break;
> case ALPM_EVENT_DISKSPACE_START:
> if(config->noprogressbar) {
> @@ -388,6 +397,11 @@ void cb_event(alpm_event_t *event)
> case ALPM_EVENT_PKG_RETRIEVE_DONE:
> case ALPM_EVENT_PKG_RETRIEVE_FAILED:
> console_cursor_move_end();
> + if(total_enabled && dload_progressbar_enabled()) {
> + update_total_finalstats();
> + printf("\n");
> + }
> + total_enabled = 0;
OK.
> flush_output_list();
> on_progress = 0;
> break;
> @@ -679,11 +693,6 @@ void cb_progress(alpm_progress_t event, const char *pkgname, int percent,
> void cb_dl_total(off_t total)
> {
> list_total = total;
> - /* if we get a 0 value, it means this list has finished downloading,
> - * so clear out our list_xfered as well */
> - if(total == 0) {
> - list_xfered = 0;
> - }
> }
OK
>
> static int dload_progressbar_enabled(void)
> @@ -726,6 +735,16 @@ static bool find_bar_for_filename(const char *filename, int *index, struct pacma
> return false;
> }
>
> +static void init_total_progressbar(void)
> +{
> + totalbar = calloc(1, sizeof(struct pacman_progress_bar));
> + assert(totalbar);
> + totalbar->filename = "Total progress:";
> + totalbar->init_time = get_time_ms();
> + totalbar->total_size = list_total;
> + totalbar->rate = 0.0;
> +}
> +
OK.
> static void draw_pacman_progress_bar(struct pacman_progress_bar *bar)
> {
> int infolen;
> @@ -851,10 +870,17 @@ static void dload_init_event(const char *filename, alpm_download_event_init_t *d
> printf(_(" %s downloading...\n"), filename);
> multibar_ui.cursor_lineno++;
> multibar_ui.active_downloads_num++;
> +
> + if(total_enabled) {
> + /* redraw the total download progress bar */
> + draw_pacman_progress_bar(totalbar);
> + printf("\n");
> + multibar_ui.cursor_lineno++;
> + }
> }
OK.
>
> -/* Draws download progress */
> -static void dload_progress_event(const char *filename, alpm_download_event_progress_t *data)
> +
> +static off_t update_file_dload_bar(const char *filename, alpm_download_event_progress_t *data)
> {
> int index;
> struct pacman_progress_bar *bar;
> @@ -862,10 +888,7 @@ static void dload_progress_event(const char *filename, alpm_download_event_progr
> double last_chunk_rate;
> int64_t timediff;
> bool ok;
> -
> - if(!dload_progressbar_enabled()) {
> - return;
> - }
> + off_t last_chunk_amount;
>
OK.
> ok = find_bar_for_filename(filename, &index, &bar);
> assert(ok);
> @@ -875,11 +898,12 @@ static void dload_progress_event(const char *filename, alpm_download_event_progr
>
> if(timediff < UPDATE_SPEED_MS) {
> /* return if the calling interval was too short */
> - return;
> + return 0;
> }
> bar->sync_time = curr_time;
>
> - last_chunk_rate = (double)(data->downloaded - bar->xfered) / (timediff / 1000.0);
> + last_chunk_amount = data->downloaded - bar->xfered;
> + last_chunk_rate = (double)last_chunk_amount * 1000 / timediff;
OK.
> /* average rate to reduce jumpiness */
> bar->rate = (last_chunk_rate + 2 * bar->rate) / 3;
> if(bar->rate > 0.0) {
> @@ -894,6 +918,62 @@ static void dload_progress_event(const char *filename, alpm_download_event_progr
>
> console_cursor_goto_bar(index);
> draw_pacman_progress_bar(bar);
> +
> + return last_chunk_amount;
> +}
> +
> +static void update_total_dload_bar(off_t last_chunk_amount)
> +{
> + int64_t curr_time = get_time_ms();
> + int64_t timediff;
> + double last_chunk_rate;
> +
> + timediff = curr_time - totalbar->init_time;
> + totalbar->xfered += last_chunk_amount;
> + last_chunk_rate = (double)totalbar->xfered * 1000 / timediff;
> + totalbar->rate = (last_chunk_rate + 2 * totalbar->rate) / 3;
> + totalbar->eta = (totalbar->total_size - totalbar->xfered) / totalbar->rate;
> +
> + console_cursor_move_end();
> + draw_pacman_progress_bar(totalbar);
> +}
OK.
> +
> +static void update_total_finalstats(void)
> +{
> + int64_t timediff;
> +
> + /* compute final values */
> + totalbar->xfered = totalbar->total_size;
> + timediff = get_time_ms() - totalbar->init_time;
> +
> + /* if transfer was too fast, treat it as a 1ms transfer, for the sake
> + * of the rate calculation */
> + if(timediff < 1)
> + timediff = 1;
> +
> + totalbar->rate = (double)totalbar->xfered * 1000 / timediff;
> + /* round elapsed time (in ms) to the nearest second */
> + totalbar->eta = (unsigned int)(timediff + 500) / 1000;
> +
> + console_cursor_move_end();
> + draw_pacman_progress_bar(totalbar);
> +}
OK. I get the feeling that this and the above function are duplicating
a bit of code from normal progress bars. But a quick look leads me to
think it is not worth refactoring.
> +
> +/* Handles download progress event */
> +static void dload_progress_event(const char *filename, alpm_download_event_progress_t *data)
> +{
> + off_t last_chunk_amount;
> +
> + if(!dload_progressbar_enabled()) {
> + return;
> + }
> +
> + last_chunk_amount = update_file_dload_bar(filename, data);
> +
> + if(total_enabled && last_chunk_amount) {
> + update_total_dload_bar(last_chunk_amount);
> + }
> +
> fflush(stdout);
> }
>
OK.
More information about the pacman-dev
mailing list