[pacman-dev] [PATCH] Versioned provisions

[Xavier]

On Fri, Nov 16, 2007 at 11:20:30PM +0100, Nagy Gabor wrote:
> diff --git a/lib/libalpm/provide.c b/lib/libalpm/provide.c
> index df600be..0e3664f 100644
> --- a/lib/libalpm/provide.c
> +++ b/lib/libalpm/provide.c
> @@ -31,6 +31,25 @@
>  #include "db.h"
>  #include "log.h"
>  
> +/* helper function for alpm_list_find and _alpm_db_whatprovides
> + * 
> + * @return "provision.name" == needle (as string)
> + */
> +int provisioncmp(const void *provision, const void *needle)
> +{
> +	char *tmpptr;
> +	tmpptr = strchr((char *)provision, ' ');
> +	
> +	if(tmpptr == NULL) { /* no provision-version */
> +		return(strcmp(provision, needle));
> +	} else { 
> +		*tmpptr='\0'; /* we will restore this, so we won't break the function definition */
> +		int retval = strcmp(provision, needle);
> +		*tmpptr=' ';
> +		return(retval);
> +	}
> +}
> +

Hmm ok, you restore it, but does that count ? :)
I thought a const shouldn't be modified in any cases.
But gcc doesn't seem to complain about it.

Also, I think something like _alpm_prov_cmp would be more consistent with the
other function names.

>  /* return a alpm_list_t of packages in "db" that provide "package"
>   */
>  alpm_list_t *_alpm_db_whatprovides(pmdb_t *db, const char *package)
> @@ -47,7 +66,7 @@ alpm_list_t *_alpm_db_whatprovides(pmdb_t *db, const char *package)
>  	for(lp = _alpm_db_get_pkgcache(db); lp; lp = lp->next) {
>  		pmpkg_t *info = lp->data;
>  
> -		if(alpm_list_find_str(alpm_pkg_get_provides(info), package)) {
> +		if(alpm_list_find(alpm_pkg_get_provides(info), (const void *)package, provisioncmp)) {
>  			pkgs = alpm_list_add(pkgs, info);
>  		}
>  	}
> diff --git a/lib/libalpm/provide.h b/lib/libalpm/provide.h
> index b5c55db..80c6f8b 100644
> --- a/lib/libalpm/provide.h
> +++ b/lib/libalpm/provide.h
> @@ -25,6 +25,7 @@
>  #include "alpm_list.h"
>  #include "config.h"
>  
> +int provisioncmp(const void *provision, const void *needle);
>  alpm_list_t *_alpm_db_whatprovides(pmdb_t *db, const char *package);
>  
>  #endif /* _ALPM_PROVIDE_H */

It isn't a comment on your patch, but on the actual state of pacman :
why do we have a provide.c and provide.h just for the private
_alpm_db_whatprovides function ?
What's even more strange is that the public alpm_db_whatprovides is defined
in db.c . So any objections for eliminating provide.{c,h} , and moving
_alpm_db_whatprovides to db.c ?

> diff --git a/pactest/tests/add044.py b/pactest/tests/add044.py
> new file mode 100644
> index 0000000..929a9af
> --- /dev/null
> +++ b/pactest/tests/add044.py
> @@ -0,0 +1,15 @@
> +self.description = "provision>=1.0-2 dependency (2)"
> +
> +p = pmpkg("pkg1", "1.0-2")
> +p.depends = ["provision>=1.0-2"]
> +self.addpkg(p)
> +
> +lp = pmpkg("pkg2", "1.0-2")
> +lp.provides = ["provision 1.0-2"]
> +self.addpkg2db("local", lp)
> +
> +self.args = "-A %s" % p.filename()
> +
> +self.addrule("PACMAN_RETCODE=1")
> +self.addrule("!PKG_EXIST=pkg1")
> +self.addrule("PKG_EXIST=pkg2")

I don't understand the rules, why should this pactest fail?

> diff --git a/pactest/tests/sync500.py b/pactest/tests/sync500.py
> new file mode 100644
> index 0000000..36364c1
> --- /dev/null
> +++ b/pactest/tests/sync500.py
> @@ -0,0 +1,10 @@
> +self.description = "-S provision"
> +
> +sp = pmpkg("pkg1")
> +sp.provides = ["provision 1.0-1"]
> +self.addpkg2db("sync", sp)
> +
> +self.args = "-S provision"
> +
> +self.addrule("PACMAN_RETCODE=0")
> +self.addrule("PKG_EXIST=pkg1")

Interesting, the code for handling this is both in libalpm/sync.c (addtarget)
and in pacman/sync.c .
I suppose if we remove the part from the frontend, and this pactest still
pass, it would be a good confirmation :)