[pacman-dev] [PATCH 8/9] Allow invalid sync DBs to be returned by the library

[Allan McRae]

On 08/06/11 17:51, Dan McGee wrote:
> They are placeholders, but important for things like trying to re-sync a
> database missing a signature. By using the alpm_db_valid() method at the
> right time, a client can take the appropriate action with these invalid
> databases as necessary.
>
> In pacman's case, we disallow just about anything that involves looking
> at a sync database outside of an '-Sy' operation (although we do check
> the validity immediately after). A few operations are still permitted-
> '-Q' ops that don't touch sync databases as well as '-R'.
>
> Signed-off-by: Dan McGee<dan at archlinux.org>
> ---
>   lib/libalpm/be_sync.c |    6 +-----
>   src/pacman/database.c |    2 +-
>   src/pacman/query.c    |    7 ++-----
>   src/pacman/remove.c   |    2 +-
>   src/pacman/sync.c     |   21 ++++++++++-----------
>   src/pacman/upgrade.c  |    2 +-
>   src/pacman/util.c     |   27 ++++++++++++++++++++++++++-
>   src/pacman/util.h     |    3 ++-
>   8 files changed, 44 insertions(+), 26 deletions(-)
>
> diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
> index 6f4c36c..69c7e21 100644
> --- a/lib/libalpm/be_sync.c
> +++ b/lib/libalpm/be_sync.c
> @@ -592,14 +592,10 @@ pmdb_t *_alpm_db_register_sync(pmhandle_t *handle, const char *treename,
>   	db->handle = handle;
>   	db->pgp_verify = level;
>
> -	if(sync_db_validate(db)) {
> -		_alpm_db_free(db);
> -		return NULL;
> -	}
> +	sync_db_validate(db);
>
>   	handle->dbs_sync = alpm_list_add(handle->dbs_sync, db);
>   	return db;
>   }
>
> -
>   /* vim: set ts=2 sw=2 noet: */
> diff --git a/src/pacman/database.c b/src/pacman/database.c
> index 292fa54..5a5c398 100644
> --- a/src/pacman/database.c
> +++ b/src/pacman/database.c
> @@ -59,7 +59,7 @@ int pacman_database(alpm_list_t *targets)
>   	}
>
>   	/* Lock database */
> -	if(trans_init(0) == -1) {
> +	if(trans_init(0, 0) == -1) {
>   		return 1;
>   	}
>
> diff --git a/src/pacman/query.c b/src/pacman/query.c
> index 76d9d73..938fff6 100644
> --- a/src/pacman/query.c
> +++ b/src/pacman/query.c
> @@ -502,11 +502,8 @@ int pacman_query(alpm_list_t *targets)
>   		return ret;
>   	}
>
> -	if(config->op_q_foreign) {
> -		/* ensure we have at least one valid sync db set up */
> -		alpm_list_t *sync_dbs = alpm_option_get_syncdbs(config->handle);
> -		if(sync_dbs == NULL || alpm_list_count(sync_dbs) == 0) {
> -			pm_printf(PM_LOG_ERROR, _("no usable package repositories configured.\n"));
> +	if(config->op_q_foreign || config->op_q_upgrade) {
> +		if(check_syncdbs(1, 1)) {
>   			return 1;
>   		}
>   	}
> diff --git a/src/pacman/remove.c b/src/pacman/remove.c
> index 59698e7..595263a 100644
> --- a/src/pacman/remove.c
> +++ b/src/pacman/remove.c
> @@ -81,7 +81,7 @@ int pacman_remove(alpm_list_t *targets)
>   	}
>
>   	/* Step 0: create a new transaction */
> -	if(trans_init(config->flags) == -1) {
> +	if(trans_init(config->flags, 0) == -1) {
>   		return 1;
>   	}
>
> diff --git a/src/pacman/sync.c b/src/pacman/sync.c
> index 6d224e4..36c7799 100644
> --- a/src/pacman/sync.c
> +++ b/src/pacman/sync.c
> @@ -283,7 +283,7 @@ static int sync_synctree(int level, alpm_list_t *syncs)
>   	alpm_list_t *i;
>   	int success = 0, ret;
>
> -	if(trans_init(0) == -1) {
> +	if(trans_init(0, 0) == -1) {
>   		return 0;
>   	}
>
> @@ -305,10 +305,6 @@ static int sync_synctree(int level, alpm_list_t *syncs)
>   	if(trans_release() == -1) {
>   		return 0;
>   	}
> -	/* We should always succeed if at least one DB was upgraded - we may possibly
> -	 * fail later with unresolved deps, but that should be rare, and would be
> -	 * expected
> -	 */
>   	if(!success) {
>   		pm_fprintf(stderr, PM_LOG_ERROR, _("failed to synchronize any databases\n"));
>   	}
> @@ -746,7 +742,7 @@ static int sync_trans(alpm_list_t *targets)
>   	alpm_list_t *i;
>
>   	/* Step 1: create a new transaction... */
> -	if(trans_init(config->flags) == -1) {
> +	if(trans_init(config->flags, 1) == -1) {
>   		return 1;
>   	}
>
> @@ -900,7 +896,7 @@ int pacman_sync(alpm_list_t *targets)
>   	if(config->op_s_clean) {
>   		int ret = 0;
>
> -		if(trans_init(0) == -1) {
> +		if(trans_init(0, 0) == -1) {
>   			return 1;
>   		}
>
> @@ -915,13 +911,12 @@ int pacman_sync(alpm_list_t *targets)
>   		return ret;
>   	}
>
> -	/* ensure we have at least one valid sync db set up */
> -	sync_dbs = alpm_option_get_syncdbs(config->handle);
> -	if(sync_dbs == NULL || alpm_list_count(sync_dbs) == 0) {
> -		pm_printf(PM_LOG_ERROR, _("no usable package repositories configured.\n"));
> +	if(check_syncdbs(1, 0)) {
>   		return 1;
>   	}
>
> +	sync_dbs = alpm_option_get_syncdbs(config->handle);
> +
>   	if(config->op_s_sync) {
>   		/* grab a fresh package list */
>   		printf(_(":: Synchronizing package databases...\n"));
> @@ -931,6 +926,10 @@ int pacman_sync(alpm_list_t *targets)
>   		}
>   	}
>
> +	if(check_syncdbs(1, 1)) {
> +		return 1;
> +	}
> +
>   	/* search for a package */
>   	if(config->op_s_search) {
>   		return sync_search(sync_dbs, targets);
> diff --git a/src/pacman/upgrade.c b/src/pacman/upgrade.c
> index cdbbffe..ef273cf 100644
> --- a/src/pacman/upgrade.c
> +++ b/src/pacman/upgrade.c
> @@ -67,7 +67,7 @@ int pacman_upgrade(alpm_list_t *targets)
>   	}
>
>   	/* Step 1: create a new transaction */
> -	if(trans_init(config->flags) == -1) {
> +	if(trans_init(config->flags, 1) == -1) {
>   		return 1;
>   	}
>
> diff --git a/src/pacman/util.c b/src/pacman/util.c
> index 2388e5c..ce305a4 100644
> --- a/src/pacman/util.c
> +++ b/src/pacman/util.c
> @@ -49,9 +49,12 @@
>   #include "callback.h"
>
>
> -int trans_init(pmtransflag_t flags)
> +int trans_init(pmtransflag_t flags, int check_valid)
>   {
>   	int ret;
> +
> +	check_syncdbs(0, check_valid);
> +
>   	if(config->print) {
>   		ret = alpm_trans_init(config->handle, flags, NULL, NULL, NULL);
>   	} else {
> @@ -100,6 +103,28 @@ int needs_root(void)
>   	}
>   }
>
> +int check_syncdbs(int min_count, int check_valid)
> +{
> +	alpm_list_t *i, *sync_dbs;
> +	sync_dbs = alpm_option_get_syncdbs(config->handle);
> +	if(sync_dbs == NULL || alpm_list_count(sync_dbs)<  min_count) {
> +		pm_printf(PM_LOG_ERROR, _("no usable package repositories configured.\n"));
> +		return 1;
> +	}
> +	if(check_valid) {
> +		/* ensure all known dbs are valid */
> +		for(i = sync_dbs; i; i = alpm_list_next(i)) {
> +			pmdb_t *db = i->data;
> +			if(alpm_db_valid(db)) {
> +				pm_printf(PM_LOG_ERROR, _("database '%s' is not valid (%s)\n"),
> +						alpm_db_get_name(db), alpm_strerror(alpm_errno(config->handle)));
> +				return 1;

Instead of returning immediately here, would it be good to finish 
checking all databases for validity?