[pacman-dev] [PATCH 4/5] Versioned provisions
This patch introduces versioned provisions in "provision 1.0-1" format. _alpm_db_whatprovides was modified accordingly (added sync500.py) alpm_depcmp was modified accordingly (add043.py passes now; added add044.py and add045.py) Note: alpm_db_search now uses the whole versioned %PROVIDES% string in its search Note: debug logging was simplified in alpm_depcmp Signed-off-by: Nagy Gabor <ngaba@bibl.u-szeged.hu> [Xavier: fixed a few typos, duplicate const strings with strdup before modifying them, put some debugging back in alpm_depcmp, minor code cleanups (var/function renaming), added a note in PKGBUILD man page.] Signed-off-by: Chantry Xavier <shiningxc@gmail.com> --- doc/PKGBUILD.5.txt | 2 + lib/libalpm/db.c | 21 +++++++++++++++++- lib/libalpm/db.h | 1 + lib/libalpm/deps.c | 54 +++++++++++++++++++++++++-------------------- lib/libalpm/sync.c | 4 +- pactest/tests/add044.py | 15 ++++++++++++ pactest/tests/add045.py | 15 ++++++++++++ pactest/tests/sync500.py | 10 ++++++++ 8 files changed, 95 insertions(+), 27 deletions(-) create mode 100644 pactest/tests/add044.py create mode 100644 pactest/tests/add045.py create mode 100644 pactest/tests/sync500.py diff --git a/doc/PKGBUILD.5.txt b/doc/PKGBUILD.5.txt index 6950041..0d3ff3a 100644 --- a/doc/PKGBUILD.5.txt +++ b/doc/PKGBUILD.5.txt @@ -133,6 +133,8 @@ Options and Directives a package to provide dependencies other than its own package name. For example, the dcron package can provide 'cron', which allows packages to depend on 'cron' rather than 'dcron OR fcron'. + Versioned provisions are also possible. For example, dcron can provide + 'cron 2.0' to satisfy the 'cron>=2.0' dependency of other packages. *replaces (array)*:: An array of packages that this package should replace, and can be used diff --git a/lib/libalpm/db.c b/lib/libalpm/db.c index af58387..fd5786e 100644 --- a/lib/libalpm/db.c +++ b/lib/libalpm/db.c @@ -777,6 +777,25 @@ pmdb_t *_alpm_db_register_sync(const char *treename) return(db); } +/* helper function for alpm_list_find and _alpm_db_whatprovides + * + * @return "provision.name" == needle (as string) + */ +int _alpm_prov_cmp(const void *provision, const void *needle) +{ + char *tmpptr; + char *provname = strdup(provision); + int retval = 0; + tmpptr = strchr(provname, ' '); + + if(tmpptr != NULL) { /* provision-version */ + *tmpptr='\0'; + } + retval = strcmp(provname, needle); + free(provname); + return(retval); +} + /* return a alpm_list_t of packages in "db" that provide "package" */ alpm_list_t *_alpm_db_whatprovides(pmdb_t *db, const char *package) @@ -793,7 +812,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, _alpm_prov_cmp)) { pkgs = alpm_list_add(pkgs, info); } } diff --git a/lib/libalpm/db.h b/lib/libalpm/db.h index 5d7173b..c34dfeb 100644 --- a/lib/libalpm/db.h +++ b/lib/libalpm/db.h @@ -57,6 +57,7 @@ pmdb_t *_alpm_db_register_local(void); pmdb_t *_alpm_db_register_sync(const char *treename); /* Provision */ +int _alpm_prov_cmp(const void *provision, const void *needle); alpm_list_t *_alpm_db_whatprovides(pmdb_t *db, const char *package); /* be.c, backend specific calls */ diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c index 679981c..9232984 100644 --- a/lib/libalpm/deps.c +++ b/lib/libalpm/deps.c @@ -373,10 +373,12 @@ alpm_list_t *_alpm_checkdeps(pmdb_t *db, pmtranstype_t op, /* else if still not found... */ if(!found) { + char *depstring = alpm_dep_get_string(depend); _alpm_log(PM_LOG_DEBUG, "missing dependency '%s' for package '%s'\n", - (char*)j->data, alpm_pkg_get_name(tp)); + depstring, alpm_pkg_get_name(tp)); + free(depstring); miss = _alpm_depmiss_new(alpm_pkg_get_name(tp), PM_DEP_TYPE_DEPEND, depend->mod, - depend->name, depend->version); + depend->name, depend->version); if(!_alpm_depmiss_isin(miss, baddeps)) { baddeps = alpm_list_add(baddeps, miss); } else { @@ -419,8 +421,10 @@ alpm_list_t *_alpm_checkdeps(pmdb_t *db, pmtranstype_t op, for(l = _alpm_db_get_pkgcache(db); l; l = l->next) { pmpkg_t *pkg = l->data; if(alpm_depcmp(pkg, depend) && !_alpm_pkg_find(alpm_pkg_get_name(pkg), packages)) { + char *depstring = alpm_dep_get_string(depend); _alpm_log(PM_LOG_DEBUG, "checkdeps: dependency '%s' satisfied by installed package '%s'\n", - (char*)k->data, alpm_pkg_get_name(pkg)); + depstring, alpm_pkg_get_name(pkg)); + free(depstring); satisfied = 1; break; } @@ -469,38 +473,40 @@ static int dep_vercmp(const char *version1, pmdepmod_t mod, int SYMEXPORT alpm_depcmp(pmpkg_t *pkg, pmdepend_t *dep) { - int equal = 0; + alpm_list_t *i; ALPM_LOG_FUNC; const char *pkgname = alpm_pkg_get_name(pkg); - const char *pkgversion = alpm_pkg_get_version(pkg); + const char *pkgver = alpm_pkg_get_version(pkg); + int satisfy = 0; - if(strcmp(pkgname, dep->name) == 0 - || alpm_list_find_str(alpm_pkg_get_provides(pkg), dep->name)) { + /* check (pkg->name, pkg->version) */ + satisfy = (!strcmp(pkgname, dep->name) + && dep_vercmp(pkgver, dep->mod, dep->version)); - equal = dep_vercmp(pkgversion, dep->mod, dep->version); + /* check provisions, format : "name version" */ + for(i = alpm_pkg_get_provides(pkg); i && !satisfy; i = i->next) { + char *provname = strdup(i->data); + char *provver = strchr(provname, ' '); - char *mod = "~="; - switch(dep->mod) { - case PM_DEP_MOD_EQ: mod = "=="; break; - case PM_DEP_MOD_GE: mod = ">="; break; - case PM_DEP_MOD_LE: mod = "<="; break; - default: break; - } - - if(strlen(dep->version) > 0) { - _alpm_log(PM_LOG_DEBUG, "depcmp: %s-%s %s %s-%s => %s\n", - pkgname, pkgversion, - mod, dep->name, dep->version, (equal ? "match" : "no match")); + if(provver == NULL) { /* no provision version */ + satisfy = (dep->mod == PM_DEP_MOD_ANY + && !strcmp(provname, dep->name)); } else { - _alpm_log(PM_LOG_DEBUG, "depcmp: %s-%s %s %s => %s\n", - pkgname, pkgversion, - mod, dep->name, (equal ? "match" : "no match")); + *provver = '\0'; + provver += 1; + satisfy = (!strcmp(provname, dep->name) + && dep_vercmp(provver, dep->mod, dep->version)); } + free(provname); } - return(equal); + char *depstring = alpm_dep_get_string(dep); + _alpm_log(PM_LOG_DEBUG, "alpm_depcmp %s-%s %s : %smatch\n", + pkgname, pkgver, depstring, satisfy ? "" : "no "); + free(depstring); + return(satisfy); } pmdepend_t SYMEXPORT *alpm_splitdep(const char *depstring) diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 6024b6b..1fd8700 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -499,8 +499,8 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync } pmpkg_t *local = _alpm_db_get_pkgfromcache(db_local, miss->depend.name); /* check if this package provides the package it's conflicting with */ - if(alpm_list_find_str(alpm_pkg_get_provides(sync->pkg), - miss->depend.name)) { + if(alpm_list_find(alpm_pkg_get_provides(sync->pkg), + miss->depend.name, _alpm_prov_cmp)) { /* treat like a replaces item so requiredby fields are * inherited properly. */ _alpm_log(PM_LOG_DEBUG, "package '%s' provides its own conflict\n", diff --git a/pactest/tests/add044.py b/pactest/tests/add044.py new file mode 100644 index 0000000..cbc82e9 --- /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=0") +self.addrule("PKG_EXIST=pkg1") +self.addrule("PKG_EXIST=pkg2") diff --git a/pactest/tests/add045.py b/pactest/tests/add045.py new file mode 100644 index 0000000..b53e090 --- /dev/null +++ b/pactest/tests/add045.py @@ -0,0 +1,15 @@ +self.description = "provision>=1.0-2 dependency (3)" + +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-1"] +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") 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") -- 1.5.3.5
On Nov 17, 2007 7:51 AM, Nagy Gabor <shiningxc@gmail.com> wrote:
This patch introduces versioned provisions in "provision 1.0-1" format. _alpm_db_whatprovides was modified accordingly (added sync500.py) alpm_depcmp was modified accordingly (add043.py passes now; added add044.py and add045.py) Note: alpm_db_search now uses the whole versioned %PROVIDES% string in its search Note: debug logging was simplified in alpm_depcmp
int SYMEXPORT alpm_depcmp(pmpkg_t *pkg, pmdepend_t *dep) { - int equal = 0; + alpm_list_t *i;
ALPM_LOG_FUNC;
const char *pkgname = alpm_pkg_get_name(pkg); - const char *pkgversion = alpm_pkg_get_version(pkg); + const char *pkgver = alpm_pkg_get_version(pkg); + int satisfy = 0;
- if(strcmp(pkgname, dep->name) == 0 - || alpm_list_find_str(alpm_pkg_get_provides(pkg), dep->name)) { + /* check (pkg->name, pkg->version) */ + satisfy = (!strcmp(pkgname, dep->name)
== 0 is much clearer, so I'm going to change this back if I pull the patch. Let's not take shortcuts in C just because they work- lets make the code readable for everyone. It will all get compiled the same way anyway. As a general rule, I always want to see an == or != used with strcmp- it is just easier to digest.
+ && dep_vercmp(pkgver, dep->mod, dep->version));
- equal = dep_vercmp(pkgversion, dep->mod, dep->version); + /* check provisions, format : "name version" */ + for(i = alpm_pkg_get_provides(pkg); i && !satisfy; i = i->next) { + char *provname = strdup(i->data); + char *provver = strchr(provname, ' ');
- char *mod = "~="; - switch(dep->mod) { - case PM_DEP_MOD_EQ: mod = "=="; break; - case PM_DEP_MOD_GE: mod = ">="; break; - case PM_DEP_MOD_LE: mod = "<="; break; - default: break; - } - - if(strlen(dep->version) > 0) { - _alpm_log(PM_LOG_DEBUG, "depcmp: %s-%s %s %s-%s => %s\n", - pkgname, pkgversion, - mod, dep->name, dep->version, (equal ? "match" : "no match")); + if(provver == NULL) { /* no provision version */ + satisfy = (dep->mod == PM_DEP_MOD_ANY + && !strcmp(provname, dep->name)); } else { - _alpm_log(PM_LOG_DEBUG, "depcmp: %s-%s %s %s => %s\n", - pkgname, pkgversion, - mod, dep->name, (equal ? "match" : "no match")); + *provver = '\0'; + provver += 1; This is ugly, and I won't take it again without at least a one line comment describing what is being done.
+ satisfy = (!strcmp(provname, dep->name) + && dep_vercmp(provver, dep->mod, dep->version)); } + free(provname); }
- return(equal); + char *depstring = alpm_dep_get_string(dep); + _alpm_log(PM_LOG_DEBUG, "alpm_depcmp %s-%s %s : %smatch\n", + pkgname, pkgver, depstring, satisfy ? "" : "no "); No need for tricks here to save 5 characters with the match/no match stuff. Code should be easily decipherable!
+ free(depstring); + return(satisfy); }
Looks good besides that. I'm pulling it into my tree. -Dan
On Nov 18, 2007 8:06 PM, Dan McGee <dpmcgee@gmail.com> wrote:
== 0 is much clearer, so I'm going to change this back if I pull the patch. Let's not take shortcuts in C just because they work- lets make the code readable for everyone. It will all get compiled the same way anyway. As a general rule, I always want to see an == or != used with strcmp- it is just easier to digest.
This is also in the pacman coding guidelines somewhere, for the record.
On Mon, Nov 19, 2007 at 12:13:14PM -0600, Aaron Griffin wrote:
On Nov 18, 2007 8:06 PM, Dan McGee <dpmcgee@gmail.com> wrote:
== 0 is much clearer, so I'm going to change this back if I pull the patch. Let's not take shortcuts in C just because they work- lets make the code readable for everyone. It will all get compiled the same way anyway. As a general rule, I always want to see an == or != used with strcmp- it is just easier to digest.
This is also in the pacman coding guidelines somewhere, for the record.
Thank you for bringing this up again, I already forgot. That's exactly what I wanted to say : if patches are going to be rejected for a code style reason, then all these reasons should be written somewhere. I don't see what other places to look at, all other coding guidelines are in HACKING, and I can't find any mention of strcmp. Also, this rule only applies to strcmp? Because it returns 0 in case of success? And I just grepped the source for strcmp, it seems like it isn't used with == or != in the majority of the cases. Even though there are also many cases where it is. Note that I don't have any problem with coding guidelines, as long as they are clear and written. I'm all for a consistent style.
On Nov 19, 2007 12:34 PM, Xavier <shiningxc@gmail.com> wrote:
On Mon, Nov 19, 2007 at 12:13:14PM -0600, Aaron Griffin wrote:
On Nov 18, 2007 8:06 PM, Dan McGee <dpmcgee@gmail.com> wrote:
== 0 is much clearer, so I'm going to change this back if I pull the patch. Let's not take shortcuts in C just because they work- lets make the code readable for everyone. It will all get compiled the same way anyway. As a general rule, I always want to see an == or != used with strcmp- it is just easier to digest.
This is also in the pacman coding guidelines somewhere, for the record.
Thank you for bringing this up again, I already forgot. That's exactly what I wanted to say : if patches are going to be rejected for a code style reason, then all these reasons should be written somewhere. I don't see what other places to look at, all other coding guidelines are in HACKING, and I can't find any mention of strcmp.
Also, this rule only applies to strcmp? Because it returns 0 in case of success? And I just grepped the source for strcmp, it seems like it isn't used with == or != in the majority of the cases. Even though there are also many cases where it is.
Note that I don't have any problem with coding guidelines, as long as they are clear and written. I'm all for a consistent style.
OK, I will put this on my TODO list and get it into the GIT repo- I think it makes more sense and we can all refer to it there. I will also add some of these "unwritten rules". I'll even post it here first to see if their are any objections. http://www.archlinux.org/~aaron/pacman-coding.html -Dan
On Nov 19, 2007 12:59 PM, Dan McGee <dpmcgee@gmail.com> wrote:
On Nov 19, 2007 12:34 PM, Xavier <shiningxc@gmail.com> wrote:
On Mon, Nov 19, 2007 at 12:13:14PM -0600, Aaron Griffin wrote:
On Nov 18, 2007 8:06 PM, Dan McGee <dpmcgee@gmail.com> wrote:
== 0 is much clearer, so I'm going to change this back if I pull the patch. Let's not take shortcuts in C just because they work- lets make the code readable for everyone. It will all get compiled the same way anyway. As a general rule, I always want to see an == or != used with strcmp- it is just easier to digest.
This is also in the pacman coding guidelines somewhere, for the record.
Thank you for bringing this up again, I already forgot. That's exactly what I wanted to say : if patches are going to be rejected for a code style reason, then all these reasons should be written somewhere. I don't see what other places to look at, all other coding guidelines are in HACKING, and I can't find any mention of strcmp.
Also, this rule only applies to strcmp? Because it returns 0 in case of success? And I just grepped the source for strcmp, it seems like it isn't used with == or != in the majority of the cases. Even though there are also many cases where it is.
Note that I don't have any problem with coding guidelines, as long as they are clear and written. I'm all for a consistent style.
OK, I will put this on my TODO list and get it into the GIT repo- I think it makes more sense and we can all refer to it there. I will also add some of these "unwritten rules". I'll even post it here first to see if their are any objections.
I thought there was a better version of this somewhere. Either way, the pseudo-rule was something to the effect of: if(!strcmp(a,b)) { /* success case */ } This is bad because it just doesn't read right. You're using a negative to check for a success case. The compiler is going to do the same thing regardless, so why not make this readable: if(strcmp(a,b) == 0) { /* success case */ } Dan, feel free to disagree with me on this one
On Nov 19, 2007 2:00 PM, Aaron Griffin <aaronmgriffin@gmail.com> wrote:
On Nov 19, 2007 12:59 PM, Dan McGee <dpmcgee@gmail.com> wrote:
On Nov 19, 2007 12:34 PM, Xavier <shiningxc@gmail.com> wrote:
On Mon, Nov 19, 2007 at 12:13:14PM -0600, Aaron Griffin wrote:
On Nov 18, 2007 8:06 PM, Dan McGee <dpmcgee@gmail.com> wrote:
== 0 is much clearer, so I'm going to change this back if I pull the patch. Let's not take shortcuts in C just because they work- lets make the code readable for everyone. It will all get compiled the same way anyway. As a general rule, I always want to see an == or != used with strcmp- it is just easier to digest.
This is also in the pacman coding guidelines somewhere, for the record.
Thank you for bringing this up again, I already forgot. That's exactly what I wanted to say : if patches are going to be rejected for a code style reason, then all these reasons should be written somewhere. I don't see what other places to look at, all other coding guidelines are in HACKING, and I can't find any mention of strcmp.
Also, this rule only applies to strcmp? Because it returns 0 in case of success? And I just grepped the source for strcmp, it seems like it isn't used with == or != in the majority of the cases. Even though there are also many cases where it is.
Note that I don't have any problem with coding guidelines, as long as they are clear and written. I'm all for a consistent style.
OK, I will put this on my TODO list and get it into the GIT repo- I think it makes more sense and we can all refer to it there. I will also add some of these "unwritten rules". I'll even post it here first to see if their are any objections.
I thought there was a better version of this somewhere.
Either way, the pseudo-rule was something to the effect of:
if(!strcmp(a,b)) { /* success case */ }
This is bad because it just doesn't read right. You're using a negative to check for a success case. The compiler is going to do the same thing regardless, so why not make this readable:
if(strcmp(a,b) == 0) { /* success case */ }
Dan, feel free to disagree with me on this one
Dude, you quoted me agreeing, thats how the whole debate started. :)
On Nov 18, 2007 8:06 PM, Dan McGee <dpmcgee@gmail.com> wrote:
== 0 is much clearer, so I'm going to change this back if I pull the patch. Let's not take shortcuts in C just because they work- lets make the code readable for everyone. It will all get compiled the same way anyway. As a general rule, I always want to see an == or != used with strcmp- it is just easier to digest.
-Dan
On Nov 19, 2007 2:35 PM, Dan McGee <dpmcgee@gmail.com> wrote:
On Nov 19, 2007 2:00 PM, Aaron Griffin <aaronmgriffin@gmail.com> wrote:
On Nov 19, 2007 12:59 PM, Dan McGee <dpmcgee@gmail.com> wrote:
On Nov 19, 2007 12:34 PM, Xavier <shiningxc@gmail.com> wrote:
On Mon, Nov 19, 2007 at 12:13:14PM -0600, Aaron Griffin wrote:
On Nov 18, 2007 8:06 PM, Dan McGee <dpmcgee@gmail.com> wrote:
== 0 is much clearer, so I'm going to change this back if I pull the patch. Let's not take shortcuts in C just because they work- lets make the code readable for everyone. It will all get compiled the same way anyway. As a general rule, I always want to see an == or != used with strcmp- it is just easier to digest.
This is also in the pacman coding guidelines somewhere, for the record.
Thank you for bringing this up again, I already forgot. That's exactly what I wanted to say : if patches are going to be rejected for a code style reason, then all these reasons should be written somewhere. I don't see what other places to look at, all other coding guidelines are in HACKING, and I can't find any mention of strcmp.
Also, this rule only applies to strcmp? Because it returns 0 in case of success? And I just grepped the source for strcmp, it seems like it isn't used with == or != in the majority of the cases. Even though there are also many cases where it is.
Note that I don't have any problem with coding guidelines, as long as they are clear and written. I'm all for a consistent style.
OK, I will put this on my TODO list and get it into the GIT repo- I think it makes more sense and we can all refer to it there. I will also add some of these "unwritten rules". I'll even post it here first to see if their are any objections.
I thought there was a better version of this somewhere.
Either way, the pseudo-rule was something to the effect of:
if(!strcmp(a,b)) { /* success case */ }
This is bad because it just doesn't read right. You're using a negative to check for a success case. The compiler is going to do the same thing regardless, so why not make this readable:
if(strcmp(a,b) == 0) { /* success case */ }
Dan, feel free to disagree with me on this one
Dude, you quoted me agreeing, thats how the whole debate started. :)
I meant on the explanation 8)
participants (4)
-
Aaron Griffin
-
Dan McGee
-
Nagy Gabor
-
Xavier