[pacman-dev] [PATCH] Consolidate --foreign/--native filtering

[Dave Reisner]

On Tue, Dec 11, 2012 at 05:59:17AM -0500, Simon Gomizelj wrote:
> Also fix a small bug where pacman won't check if the sync dbs are first
> downloaded when invoked with --native (it should).
> 
> Signed-off-by: Simon Gomizelj <simongmzlj at gmail.com>
> ---
>  lib/libalpm/alpm.h  |  7 +++++++
>  src/pacman/conf.h   |  3 +--
>  src/pacman/pacman.c |  4 ++--
>  src/pacman/query.c  | 21 ++++++---------------
>  4 files changed, 16 insertions(+), 19 deletions(-)
> 
> diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
> index 1d6a8c6..5cddd34 100644
> --- a/lib/libalpm/alpm.h
> +++ b/lib/libalpm/alpm.h
> @@ -64,6 +64,13 @@ typedef enum _alpm_pkgfrom_t {
>  	ALPM_PKG_FROM_SYNCDB
>  } alpm_pkgfrom_t;
>  
> +/** Locality of a package. */
> +typedef enum _alpm_locality_t {
> +	ALPM_LOCALITY_NONE = 0,
> +	ALPM_LOCALITY_LOCAL = (1 << 0),
> +	ALPM_LOCALITY_FOREIGN = (1 << 1)
> +} alpm_locality_t;
> +

I realize this was my initial suggestion, but I don't think it's needed
to expose this in the header as we never actually use it in the library.
It's unfortunate we have no sync.h or query.h in src/pacman/. In lieu of
that, I suppose it just belongs in conf.h.

...and to be a pain in the butt, if we're going to make the enum local
to pacman, then it should be renamed. Suggestion:

  typedef enum _pkg_locality_t {
    PKG_LOCALITY_UNKNOWN = 0,
    PKG_LOCALITY_LOCAL,
    PKG_LOCALITY_FOREIGN
  } pkg_locality_t;

>  /** Location a package object was loaded from. */
>  typedef enum _alpm_pkgvalidation_t {
>  	ALPM_PKG_VALIDATION_UNKNOWN = 0,
> diff --git a/src/pacman/conf.h b/src/pacman/conf.h
> index aee6859..b2d67f0 100644
> --- a/src/pacman/conf.h
> +++ b/src/pacman/conf.h
> @@ -50,8 +50,6 @@ typedef struct __config_t {
>  	unsigned short op_q_isfile;
>  	unsigned short op_q_info;
>  	unsigned short op_q_list;
> -	unsigned short op_q_foreign;
> -	unsigned short op_q_native;
>  	unsigned short op_q_unrequired;
>  	unsigned short op_q_deps;
>  	unsigned short op_q_explicit;
> @@ -60,6 +58,7 @@ typedef struct __config_t {
>  	unsigned short op_q_changelog;
>  	unsigned short op_q_upgrade;
>  	unsigned short op_q_check;
> +	alpm_locality_t op_q_filter;
>  
>  	unsigned short op_s_clean;
>  	unsigned short op_s_downloadonly;
> diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
> index 1ca746d..83b2350 100644
> --- a/src/pacman/pacman.c
> +++ b/src/pacman/pacman.c
> @@ -462,8 +462,8 @@ static int parsearg_query(int opt)
>  		case 'i': (config->op_q_info)++; break;
>  		case 'k': config->op_q_check = 1; break;
>  		case 'l': config->op_q_list = 1; break;
> -		case 'm': config->op_q_foreign = 1; break;
> -		case 'n': config->op_q_native = 1; break;
> +		case 'm': config->op_q_filter |= ALPM_LOCALITY_LOCAL; break;
> +		case 'n': config->op_q_filter |= ALPM_LOCALITY_FOREIGN; break;

May want to consider giving this a less generic name -- op_q_locality
perhaps.

>  		case 'o': config->op_q_owns = 1; break;
>  		case 'p': config->op_q_isfile = 1; break;
>  		case 'q': config->quiet = 1; break;
> diff --git a/src/pacman/query.c b/src/pacman/query.c
> index 2736672..b92f430 100644
> --- a/src/pacman/query.c
> +++ b/src/pacman/query.c
> @@ -337,25 +337,20 @@ static int query_group(alpm_list_t *targets)
>  	return ret;
>  }
>  
> -static int is_foreign(alpm_pkg_t *pkg)
> +static alpm_locality_t pkg_get_locality(alpm_pkg_t *pkg)
>  {
>  	const char *pkgname = alpm_pkg_get_name(pkg);
>  	alpm_list_t *j;
>  	alpm_list_t *sync_dbs = alpm_get_syncdbs(config->handle);
>  
> -	int match = 0;
>  	for(j = sync_dbs; j; j = alpm_list_next(j)) {
>  		alpm_db_t *db = j->data;
>  		alpm_pkg_t *findpkg = alpm_db_get_pkg(db, pkgname);
>  		if(findpkg) {
> -			match = 1;
> -			break;
> +			return ALPM_LOCALITY_LOCAL;
>  		}
>  	}
> -	if(match == 0) {
> -		return 1;
> -	}
> -	return 0;
> +	return ALPM_LOCALITY_FOREIGN;
>  }
>  
>  static int is_unrequired(alpm_pkg_t *pkg)
> @@ -380,12 +375,8 @@ static int filter(alpm_pkg_t *pkg)
>  			alpm_pkg_get_reason(pkg) != ALPM_PKG_REASON_DEPEND) {
>  		return 0;
>  	}
> -	/* check if this pkg is in a sync DB */
> -	if(config->op_q_native && is_foreign(pkg)) {
> -		return 0;
> -	}
> -	/* check if this pkg isn't in a sync DB */
> -	if(config->op_q_foreign && !is_foreign(pkg)) {
> +	/* check if this pkg is or isn't in a sync DB */
> +	if(config->op_q_filter && config->op_q_filter & pkg_get_locality(pkg)) {
>  		return 0;
>  	}
>  	/* check if this pkg is unrequired */
> @@ -507,7 +498,7 @@ int pacman_query(alpm_list_t *targets)
>  		return ret;
>  	}
>  
> -	if(config->op_q_foreign || config->op_q_upgrade) {
> +	if(config->op_q_filter || config->op_q_upgrade) {
>  		if(check_syncdbs(1, 1)) {
>  			return 1;
>  		}
> -- 
> 1.8.0.1
> 
>