[pacman-dev] [PATCH] Introduce event types for start/end database list download

[Allan McRae]

On 17/4/20 12:31 pm, Anatol Pomozov wrote:
> Multiplexed database/files downloads will use multiple progress bars.
> The UI logic is quite complicated and printing error messages while
> handling multiple progress bars is going to be challenging.
> 
> Instead we are going to save all ALPM error messages to a list and flush
> it at the end of the download process. Use on_progress variable that
> blocks error messages printing.
> 

This is going to be fun... If you have a 100 package update and your
first mirror is down, you will end up with 100 error messages at the
end.  e.g.

$ sudo ./build/pacman -Sy
:: Synchronizing package databases...
 testing                45.4 KiB  24.5 KiB/s 00:02 [##############] 100%
 core                  134.6 KiB  61.8 KiB/s 00:02 [##############] 100%
 extra                1646.0 KiB   741 KiB/s 00:02 [##############] 100%
 community-testing     621.3 KiB   468 KiB/s 00:01 [##############] 100%
 community               4.9 MiB  2.12 MiB/s 00:02 [##############] 100%
error: failed retrieving file 'extra.db' from allanmcrae.com : The
requested URL returned error: 404
error: failed retrieving file 'testing.db' from allanmcrae.com : The
requested URL returned error: 404
error: failed retrieving file 'core.db' from allanmcrae.com : The
requested URL returned error: 404
error: failed retrieving file 'community-testing.db' from allanmcrae.com
: The requested URL returned error: 404
error: failed retrieving file 'community.db' from allanmcrae.com : The
requested URL returned error: 404

This is patch is fine, but we should perhaps change the output to count
the failures from each server.

Also, it is not an error to fail retrieving a package or database if you
get if from the next server.  So this could be changed to a warning.

e.g.
warning: failed retrieving 5 files from allanmcrae.com

This is nothing directly to do with this patch - just something we need
to tidy.


> Signed-off-by: Anatol Pomozov <anatol.pomozov at gmail.com>
> ---
>  lib/libalpm/alpm.h    |  6 ++++++
>  lib/libalpm/be_sync.c |  7 +++++++
>  src/pacman/callback.c | 13 +++++++++++--
>  3 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
> index 2cf20343..aee5aa0d 100644
> --- a/lib/libalpm/alpm.h
> +++ b/lib/libalpm/alpm.h
> @@ -403,6 +403,12 @@ typedef enum _alpm_event_type_t {
>  	 * arguments. */
>  	ALPM_EVENT_SCRIPTLET_INFO,
>  	/** Files will be downloaded from a repository. */

/** Database files will be....

> +	ALPM_EVENT_DB_RETRIEVE_START,
> +	/** Files were downloaded from a repository. */
> +	ALPM_EVENT_DB_RETRIEVE_DONE,
> +	/** Not all files were successfully downloaded from a repository. */
> +	ALPM_EVENT_DB_RETRIEVE_FAILED,
> +	/** Files will be downloaded from a repository. */

/** Package files will be...

>  	ALPM_EVENT_RETRIEVE_START,

As we are splitting db and package events, this should be changed to:

ALPM_EVENT_PKG_RETRIEVE_START

>  	/** Files were downloaded from a repository. */
>  	ALPM_EVENT_RETRIEVE_DONE,
> diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
> index 1be40650..826c11e6 100644
> --- a/lib/libalpm/be_sync.c
> +++ b/lib/libalpm/be_sync.c
> @@ -308,6 +308,7 @@ int SYMEXPORT alpm_dbs_update(alpm_handle_t *handle, alpm_list_t *dbs, int force
>  	int ret = -1;
>  	mode_t oldmask;
>  	alpm_list_t *payloads = NULL;
> +	alpm_event_t event;
>  
>  	/* Sanity checks */
>  	CHECK_HANDLE(handle, return -1);
> @@ -382,10 +383,16 @@ int SYMEXPORT alpm_dbs_update(alpm_handle_t *handle, alpm_list_t *dbs, int force
>  		}
>  	}
>  
> +	event.type = ALPM_EVENT_DB_RETRIEVE_START;
> +	EVENT(handle, &event);
>  	ret = _alpm_multi_download(handle, payloads, syncpath);
>  	if(ret < 0) {
> +		event.type = ALPM_EVENT_DB_RETRIEVE_FAILED;
> +		EVENT(handle, &event);
>  		goto cleanup;
>  	}
> +	event.type = ALPM_EVENT_DB_RETRIEVE_DONE;
> +	EVENT(handle, &event);
>  
>  	for(i = dbs; i; i = i->next) {
>  		alpm_db_t *db = i->data;
> diff --git a/src/pacman/callback.c b/src/pacman/callback.c
> index 8fb89b39..585eae09 100644
> --- a/src/pacman/callback.c
> +++ b/src/pacman/callback.c
> @@ -280,8 +280,12 @@ void cb_event(alpm_event_t *event)
>  		case ALPM_EVENT_SCRIPTLET_INFO:
>  			fputs(event->scriptlet_info.line, stdout);
>  			break;
> +		case ALPM_EVENT_DB_RETRIEVE_START:
> +			on_progress = 1;
> +			break;
>  		case ALPM_EVENT_RETRIEVE_START:
>  			colon_printf(_("Retrieving packages...\n"));
> +			on_progress = 1;
>  			break;
>  		case ALPM_EVENT_DISKSPACE_START:
>  			if(config->noprogressbar) {
> @@ -338,6 +342,13 @@ void cb_event(alpm_event_t *event)
>  				}
>  			}
>  			break;
> +		case ALPM_EVENT_DB_RETRIEVE_DONE:
> +		case ALPM_EVENT_DB_RETRIEVE_FAILED:
> +		case ALPM_EVENT_RETRIEVE_DONE:
> +		case ALPM_EVENT_RETRIEVE_FAILED:
> +			flush_output_list();
> +			on_progress = 0;
> +			break;
>  		/* all the simple done events, with fallthrough for each */
>  		case ALPM_EVENT_FILECONFLICTS_DONE:
>  		case ALPM_EVENT_CHECKDEPS_DONE:
> @@ -349,8 +360,6 @@ void cb_event(alpm_event_t *event)
>  		case ALPM_EVENT_KEY_DOWNLOAD_DONE:
>  		case ALPM_EVENT_LOAD_DONE:
>  		case ALPM_EVENT_DISKSPACE_DONE:
> -		case ALPM_EVENT_RETRIEVE_DONE:
> -		case ALPM_EVENT_RETRIEVE_FAILED:
>  		case ALPM_EVENT_HOOK_DONE:
>  		case ALPM_EVENT_HOOK_RUN_DONE:
>  		/* we can safely ignore those as well */
>