Re: [pacman-dev] [PATCH] Implement TotalDownload functionality
Hi
On Sun, Dec 6, 2020 at 9:46 AM Anatol Pomozov wrote:
> I need to localize this string. Will resend an updated patch for it.
I localized it in the v2 of the patch
> > > +
> > > +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.
>
> These 2 functions (update and finalupdate for total bar) are slightly
> different.
> But there is some duplication in "update file" and "update total" bar
> logic. I can try to refactor and see if it makes things cleaner.
I refactored the code and introduced "update stats" and "update final
stats" functions that work both for "file progress" and "total
progress".
Re: [pacman-dev] [PATCH] Implement TotalDownload functionality
Hi
Missed 2 other comments:
On Sun, Dec 6, 2020 at 12:02 AM Allan McRae wrote:
>
> 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
> > ---
> > 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?
Yes. During the UI refactoring the old "TotalDownload" drawing logic
has been dropped. list_xfered was used for "amount of total downloaded
data".
Now we have a dedicated structure for a progress bar (struct
pacman_progress_bar) and "totalbar->xfered" becomes the new
"list_xfered".
>
> > +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?
Yes, I want to keep the progress bar handling logic together and it
appears after the cb_event() function that uses these declarations.
>
> > 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
Re: [pacman-dev] [PATCH] Implement TotalDownload functionality
Hi
On Sun, Dec 6, 2020 at 12:02 AM Allan McRae wrote:
>
> 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
> > ---
> > 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,
Re: [pacman-dev] [PATCH] Implement TotalDownload functionality
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
> ---
> 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)
> {
>
[pacman-dev] [PATCH] Implement TotalDownload functionality
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
Signed-off-by: Anatol Pomozov
---
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.
*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);
- }
-
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;
+static int total_enabled = 0;
static off_t list_total = 0.0;
+static struct pacman_progress_bar *totalbar;
/* 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);
+
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();
+ }
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;
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;
- }
}
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;
+}
+
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");
+
