[pacman-dev] [PATCH 1/3] Always enable TotalDownload
Previously TotalDownload would switch the % download from per package to overall. Meaning you had a choice of which information to dispplay. Now with parallel downloads TotalDownload adds an extra progress bar. There's no reason to have this an off by default feature. Let's just make it always on. --- README | 4 ++-- doc/pacman.conf.5.asciidoc | 5 ----- etc/pacman.conf.in | 1 - scripts/completion/zsh_completion.in | 1 - src/pacman/callback.c | 2 +- src/pacman/conf.c | 3 --- src/pacman/conf.h | 3 --- src/pacman/pacman-conf.c | 3 --- 8 files changed, 3 insertions(+), 19 deletions(-) diff --git a/README b/README index 470ccf3c..4465432a 100644 --- a/README +++ b/README @@ -194,8 +194,8 @@ remove.c and sync.c). The frontend is using a configuration file, usually "/etc/pacman.conf". Some of these options are only useful for the frontend only (mainly the ones used to -control the output like totaldownload, or the behavior with cleanmethod and -syncfirst). The rest is used to configure the library. +control the output like verbosepkglist, or the behavior with cleanmethod). +The rest is used to configure the library. [UPGRADE/REMOVE/SYNC] diff --git a/doc/pacman.conf.5.asciidoc b/doc/pacman.conf.5.asciidoc index 9bd31916..a024c3a7 100644 --- a/doc/pacman.conf.5.asciidoc +++ b/doc/pacman.conf.5.asciidoc @@ -190,11 +190,6 @@ Options Disables progress bars. This is useful for terminals which do not support escape characters. -*TotalDownload*:: - 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*:: Performs an approximate check for adequate available disk space before installing packages. diff --git a/etc/pacman.conf.in b/etc/pacman.conf.in index 634ecc06..1799efc7 100644 --- a/etc/pacman.conf.in +++ b/etc/pacman.conf.in @@ -32,7 +32,6 @@ Architecture = auto #UseSyslog #Color #NoProgressBar -#TotalDownload CheckSpace #VerbosePkgLists ParallelDownloads = 5 diff --git a/scripts/completion/zsh_completion.in b/scripts/completion/zsh_completion.in index e4bf3312..5fd8aebc 100644 --- a/scripts/completion/zsh_completion.in +++ b/scripts/completion/zsh_completion.in @@ -511,7 +511,6 @@ _pacman_conf_general_directives=( 'XferCommand' 'UseSyslog' 'Color' - 'TotalDownload' 'CheckSpace' 'VerbosePkgLists' 'DisableDownloadTimeout' diff --git a/src/pacman/callback.c b/src/pacman/callback.c index a28a79a9..2b79812e 100644 --- a/src/pacman/callback.c +++ b/src/pacman/callback.c @@ -340,7 +340,7 @@ void cb_event(alpm_event_t *event) on_progress = 1; list_total_pkgs = event->pkg_retrieve.num; list_total = event->pkg_retrieve.total_size; - total_enabled = config->totaldownload && list_total; + total_enabled = list_total; if(total_enabled) { init_total_progressbar(); } diff --git a/src/pacman/conf.c b/src/pacman/conf.c index a4f2ba35..2349e638 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -601,9 +601,6 @@ static int _parse_options(const char *key, char *value, } else if(strcmp(key, "VerbosePkgLists") == 0) { config->verbosepkglists = 1; pm_printf(ALPM_LOG_DEBUG, "config: verbosepkglists\n"); - } else if(strcmp(key, "TotalDownload") == 0) { - config->totaldownload = 1; - pm_printf(ALPM_LOG_DEBUG, "config: totaldownload\n"); } else if(strcmp(key, "CheckSpace") == 0) { config->checkspace = 1; } else if(strcmp(key, "Color") == 0) { diff --git a/src/pacman/conf.h b/src/pacman/conf.h index 1b9fb337..773fb6a9 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -113,9 +113,6 @@ typedef struct __config_t { unsigned short chomp; /* format target pkg lists as table */ unsigned short verbosepkglists; - /* When downloading, display the amount downloaded, rate, ETA, and percent - * downloaded of the total download list */ - unsigned short totaldownload; /* number of parallel download streams */ unsigned int parallel_downloads; /* select -Sc behavior */ diff --git a/src/pacman/pacman-conf.c b/src/pacman/pacman-conf.c index f8fac75d..91986ba4 100644 --- a/src/pacman/pacman-conf.c +++ b/src/pacman/pacman-conf.c @@ -263,7 +263,6 @@ static void dump_config(void) show_bool("UseSyslog", config->usesyslog); show_bool("Color", config->color); - show_bool("TotalDownload", config->totaldownload); show_bool("CheckSpace", config->checkspace); show_bool("VerbosePkgLists", config->verbosepkglists); show_bool("DisableDownloadTimeout", config->disable_dl_timeout); @@ -372,8 +371,6 @@ static int list_directives(void) show_bool("UseSyslog", config->usesyslog); } else if(strcasecmp(i->data, "Color") == 0) { show_bool("Color", config->color); - } else if(strcasecmp(i->data, "TotalDownload") == 0) { - show_bool("TotalDownload", config->totaldownload); } else if(strcasecmp(i->data, "CheckSpace") == 0) { show_bool("CheckSpace", config->checkspace); } else if(strcasecmp(i->data, "VerbosePkgLists") == 0) { -- 2.31.1
Otherwise the total progress will just match the one package and be pretty useless. --- src/pacman/callback.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pacman/callback.c b/src/pacman/callback.c index 2b79812e..00360e2c 100644 --- a/src/pacman/callback.c +++ b/src/pacman/callback.c @@ -340,7 +340,7 @@ void cb_event(alpm_event_t *event) on_progress = 1; list_total_pkgs = event->pkg_retrieve.num; list_total = event->pkg_retrieve.total_size; - total_enabled = list_total; + total_enabled = list_total && list_total_pkgs > 1; if(total_enabled) { init_total_progressbar(); } -- 2.31.1
When initially downloading a package, pacman will display a message like: wine-6.6-1-x86_64.pkg.tar.zst downloading... Then when the download progresses the message will change to: wine-6.6-1-x86_64.pkg.tar.zst So instead lets match the progress message so there's no sudden change. --- src/pacman/callback.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pacman/callback.c b/src/pacman/callback.c index 00360e2c..c683dcd5 100644 --- a/src/pacman/callback.c +++ b/src/pacman/callback.c @@ -876,9 +876,9 @@ static void draw_pacman_progress_bar(struct pacman_progress_bar *bar) static void dload_init_event(const char *filename, alpm_download_event_init_t *data) { (void)data; + char *cleaned_filename = clean_filename(filename); if(!dload_progressbar_enabled()) { - char *cleaned_filename = clean_filename(filename); printf(_(" %s downloading...\n"), cleaned_filename); free(cleaned_filename); return; @@ -892,7 +892,7 @@ static void dload_init_event(const char *filename, alpm_download_event_init_t *d multibar_ui.active_downloads = alpm_list_add(multibar_ui.active_downloads, bar); console_cursor_move_end(); - printf(_(" %s downloading...\n"), filename); + printf(" %s\n", cleaned_filename); multibar_ui.cursor_lineno++; multibar_ui.active_downloads_num++; -- 2.31.1
On 19/4/21 5:59 am, morganamilo wrote:
Previously TotalDownload would switch the % download from per package to overall. Meaning you had a choice of which information to dispplay.
Now with parallel downloads TotalDownload adds an extra progress bar. There's no reason to have this an off by default feature. Let's just make it always on. ---
allan@mando ~/arch/code/pacman (patchqueue) $ PACTEST_VALGRIND=1 ninja -C build test Summary of Failures: 332/335 xfercommand001.py FAIL 2.51s 0/1 subtests passed Memory leak somewhere - I have have not looked beyond attributing it to this patch. How this patch causes it is another issue!
On 20/4/21 2:09 pm, Allan McRae wrote:
On 19/4/21 5:59 am, morganamilo wrote:
Previously TotalDownload would switch the % download from per package to overall. Meaning you had a choice of which information to dispplay.
Now with parallel downloads TotalDownload adds an extra progress bar. There's no reason to have this an off by default feature. Let's just make it always on. ---
allan@mando ~/arch/code/pacman (patchqueue) $ PACTEST_VALGRIND=1 ninja -C build test
Summary of Failures:
332/335 xfercommand001.py FAIL 2.51s 0/1 subtests passed
Memory leak somewhere - I have have not looked beyond attributing it to this patch. How this patch causes it is another issue! .
The total progressbar is unconditionally initialised, but not used/freed with xfercommand. ==1615702== 80 bytes in 1 blocks are still reachable in loss record 3 of 3 ==1615702== at 0x4840B65: calloc (vg_replace_malloc.c:760) ==1615702== by 0x11C29C: init_total_progressbar (callback.c:743) ==1615702== by 0x11C29C: cb_event (callback.c:345) ==1615702== by 0x48A336E: download_files (sync.c:796) ==1615702== by 0x48A336E: _alpm_sync_load (sync.c:1131) ==1615702== by 0x48A4ECA: alpm_trans_commit (trans.c:173) ==1615702== by 0x11A4C9: sync_prepare_execute (sync.c:829) ==1615702== by 0x10F218: main (pacman.c:1257) ==1615702== { <insert_a_suppression_name_here> Memcheck:Leak match-leak-kinds: reachable fun:calloc fun:init_total_progressbar fun:cb_event fun:download_files fun:_alpm_sync_load fun:alpm_trans_commit fun:sync_prepare_execute fun:main }
participants (2)
-
Allan McRae
-
morganamilo