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 :)