[pacman-dev] [PATCH 1/6] Replacements refactor: extract check_literal()
This moves code that was inline in alpm_sync_sysupgrade() to its own method. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/sync.c | 79 ++++++++++++++++++++++++++++++--------------------- 1 files changed, 46 insertions(+), 33 deletions(-) diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index b9cb1fb..09800d4 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -81,6 +81,45 @@ alpm_pkg_t SYMEXPORT *alpm_sync_newversion(alpm_pkg_t *pkg, alpm_list_t *dbs_syn return NULL; } +static int check_literal(alpm_handle_t *handle, alpm_pkg_t *lpkg, + alpm_pkg_t *spkg, int enable_downgrade) +{ + /* 1. literal was found in sdb */ + int cmp = _alpm_pkg_compare_versions(spkg, lpkg); + if(cmp > 0) { + _alpm_log(handle, ALPM_LOG_DEBUG, "new version of '%s' found (%s => %s)\n", + lpkg->name, lpkg->version, spkg->version); + /* check IgnorePkg/IgnoreGroup */ + if(_alpm_pkg_should_ignore(handle, spkg) + || _alpm_pkg_should_ignore(handle, lpkg)) { + _alpm_log(handle, ALPM_LOG_WARNING, _("%s: ignoring package upgrade (%s => %s)\n"), + lpkg->name, lpkg->version, spkg->version); + } else { + _alpm_log(handle, ALPM_LOG_DEBUG, "adding package %s-%s to the transaction targets\n", + spkg->name, spkg->version); + return 1; + } + } else if(cmp < 0) { + if(enable_downgrade) { + /* check IgnorePkg/IgnoreGroup */ + if(_alpm_pkg_should_ignore(handle, spkg) + || _alpm_pkg_should_ignore(handle, lpkg)) { + _alpm_log(handle, ALPM_LOG_WARNING, _("%s: ignoring package downgrade (%s => %s)\n"), + lpkg->name, lpkg->version, spkg->version); + } else { + _alpm_log(handle, ALPM_LOG_WARNING, _("%s: downgrading from version %s to version %s\n"), + lpkg->name, lpkg->version, spkg->version); + return 1; + } + } else { + alpm_db_t *sdb = alpm_pkg_get_db(spkg); + _alpm_log(handle, ALPM_LOG_WARNING, _("%s: local (%s) is newer than %s (%s)\n"), + lpkg->name, lpkg->version, sdb->treename, spkg->version); + } + } + return 0; +} + /** Search for packages to upgrade and add them to the transaction. */ int SYMEXPORT alpm_sync_sysupgrade(alpm_handle_t *handle, int enable_downgrade) { @@ -101,50 +140,24 @@ int SYMEXPORT alpm_sync_sysupgrade(alpm_handle_t *handle, int enable_downgrade) continue; } - /* Search for literal then replacers in each sync database. - * If found, don't check other databases */ + /* Search for literal then replacers in each sync database. */ for(j = handle->dbs_sync; j; j = j->next) { alpm_db_t *sdb = j->data; /* Check sdb */ alpm_pkg_t *spkg = _alpm_db_get_pkgfromcache(sdb, lpkg->name); + int literal_upgrade = 0; if(spkg) { - /* 1. literal was found in sdb */ - int cmp = _alpm_pkg_compare_versions(spkg, lpkg); - if(cmp > 0) { - _alpm_log(handle, ALPM_LOG_DEBUG, "new version of '%s' found (%s => %s)\n", - lpkg->name, lpkg->version, spkg->version); - /* check IgnorePkg/IgnoreGroup */ - if(_alpm_pkg_should_ignore(handle, spkg) - || _alpm_pkg_should_ignore(handle, lpkg)) { - _alpm_log(handle, ALPM_LOG_WARNING, _("%s: ignoring package upgrade (%s => %s)\n"), - lpkg->name, lpkg->version, spkg->version); - } else { - _alpm_log(handle, ALPM_LOG_DEBUG, "adding package %s-%s to the transaction targets\n", - spkg->name, spkg->version); - trans->add = alpm_list_add(trans->add, spkg); - } - } else if(cmp < 0) { - if(enable_downgrade) { - /* check IgnorePkg/IgnoreGroup */ - if(_alpm_pkg_should_ignore(handle, spkg) - || _alpm_pkg_should_ignore(handle, lpkg)) { - _alpm_log(handle, ALPM_LOG_WARNING, _("%s: ignoring package downgrade (%s => %s)\n"), - lpkg->name, lpkg->version, spkg->version); - } else { - _alpm_log(handle, ALPM_LOG_WARNING, _("%s: downgrading from version %s to version %s\n"), - lpkg->name, lpkg->version, spkg->version); - trans->add = alpm_list_add(trans->add, spkg); - } - } else { - _alpm_log(handle, ALPM_LOG_WARNING, _("%s: local (%s) is newer than %s (%s)\n"), - lpkg->name, lpkg->version, sdb->treename, spkg->version); - } + literal_upgrade = check_literal(handle, lpkg, spkg, enable_downgrade); + if(literal_upgrade) { + trans->add = alpm_list_add(trans->add, spkg); } /* jump to next local package */ break; } else { /* 2. search for replacers in sdb */ alpm_list_t *k, *l; + _alpm_log(handle, ALPM_LOG_DEBUG, + "searching for replacements for %s\n", lpkg->name); for(k = _alpm_db_get_pkgcache(sdb); k; k = k->next) { int found = 0; spkg = k->data; -- 1.7.6
This moves code that was inline in alpm_sync_sysupgrade() to its own method. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/sync.c | 130 +++++++++++++++++++++++++++++----------------------- 1 files changed, 73 insertions(+), 57 deletions(-) diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 09800d4..46e3045 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -120,6 +120,75 @@ static int check_literal(alpm_handle_t *handle, alpm_pkg_t *lpkg, return 0; } +static alpm_list_t *check_replacers(alpm_handle_t *handle, alpm_pkg_t *lpkg, + alpm_db_t *sdb) +{ + /* 2. search for replacers in sdb */ + alpm_list_t *replacers = NULL; + alpm_list_t *k; + _alpm_log(handle, ALPM_LOG_DEBUG, + "searching for replacements for %s\n", lpkg->name); + for(k = _alpm_db_get_pkgcache(sdb); k; k = k->next) { + int found = 0; + alpm_pkg_t *spkg = k->data; + alpm_list_t *l; + for(l = alpm_pkg_get_replaces(spkg); l; l = l->next) { + alpm_depend_t *replace = l->data; + if(_alpm_depcmp(lpkg, replace)) { + found = 1; + break; + } + } + if(found) { + int doreplace = 0; + alpm_pkg_t *tpkg; + /* check IgnorePkg/IgnoreGroup */ + if(_alpm_pkg_should_ignore(handle, spkg) + || _alpm_pkg_should_ignore(handle, lpkg)) { + _alpm_log(handle, ALPM_LOG_WARNING, + _("ignoring package replacement (%s-%s => %s-%s)\n"), + lpkg->name, lpkg->version, spkg->name, spkg->version); + continue; + } + + QUESTION(handle->trans, ALPM_TRANS_CONV_REPLACE_PKG, lpkg, spkg, + sdb->treename, &doreplace); + if(!doreplace) { + continue; + } + + /* If spkg is already in the target list, we append lpkg to spkg's + * removes list */ + tpkg = _alpm_pkg_find(handle->trans->add, spkg->name); + if(tpkg) { + /* sanity check, multiple repos can contain spkg->name */ + if(tpkg->origin_data.db != sdb) { + _alpm_log(handle, ALPM_LOG_WARNING, _("cannot replace %s by %s\n"), + lpkg->name, spkg->name); + continue; + } + _alpm_log(handle, ALPM_LOG_DEBUG, "appending %s to the removes list of %s\n", + lpkg->name, tpkg->name); + tpkg->removes = alpm_list_add(tpkg->removes, lpkg); + /* check the to-be-replaced package's reason field */ + if(alpm_pkg_get_reason(lpkg) == ALPM_PKG_REASON_EXPLICIT) { + tpkg->reason = ALPM_PKG_REASON_EXPLICIT; + } + } else { + /* add spkg to the target list */ + /* copy over reason */ + spkg->reason = alpm_pkg_get_reason(lpkg); + spkg->removes = alpm_list_add(NULL, lpkg); + _alpm_log(handle, ALPM_LOG_DEBUG, + "adding package %s-%s to the transaction targets\n", + spkg->name, spkg->version); + replacers = alpm_list_add(replacers, spkg); + } + } + } + return replacers; +} + /** Search for packages to upgrade and add them to the transaction. */ int SYMEXPORT alpm_sync_sysupgrade(alpm_handle_t *handle, int enable_downgrade) { @@ -154,63 +223,10 @@ int SYMEXPORT alpm_sync_sysupgrade(alpm_handle_t *handle, int enable_downgrade) /* jump to next local package */ break; } else { - /* 2. search for replacers in sdb */ - alpm_list_t *k, *l; - _alpm_log(handle, ALPM_LOG_DEBUG, - "searching for replacements for %s\n", lpkg->name); - for(k = _alpm_db_get_pkgcache(sdb); k; k = k->next) { - int found = 0; - spkg = k->data; - for(l = alpm_pkg_get_replaces(spkg); l; l = l->next) { - alpm_depend_t *replace = l->data; - if(_alpm_depcmp(lpkg, replace)) { - found = 1; - break; - } - } - if(found) { - /* check IgnorePkg/IgnoreGroup */ - if(_alpm_pkg_should_ignore(handle, spkg) - || _alpm_pkg_should_ignore(handle, lpkg)) { - _alpm_log(handle, ALPM_LOG_WARNING, - _("ignoring package replacement (%s-%s => %s-%s)\n"), - lpkg->name, lpkg->version, spkg->name, spkg->version); - continue; - } - - int doreplace = 0; - QUESTION(trans, ALPM_TRANS_CONV_REPLACE_PKG, lpkg, spkg, sdb->treename, &doreplace); - if(!doreplace) { - continue; - } - - /* If spkg is already in the target list, we append lpkg to spkg's - * removes list */ - alpm_pkg_t *tpkg = _alpm_pkg_find(trans->add, spkg->name); - if(tpkg) { - /* sanity check, multiple repos can contain spkg->name */ - if(tpkg->origin_data.db != sdb) { - _alpm_log(handle, ALPM_LOG_WARNING, _("cannot replace %s by %s\n"), - lpkg->name, spkg->name); - continue; - } - _alpm_log(handle, ALPM_LOG_DEBUG, "appending %s to the removes list of %s\n", - lpkg->name, tpkg->name); - tpkg->removes = alpm_list_add(tpkg->removes, lpkg); - /* check the to-be-replaced package's reason field */ - if(alpm_pkg_get_reason(lpkg) == ALPM_PKG_REASON_EXPLICIT) { - tpkg->reason = ALPM_PKG_REASON_EXPLICIT; - } - } else { - /* add spkg to the target list */ - /* copy over reason */ - spkg->reason = alpm_pkg_get_reason(lpkg); - spkg->removes = alpm_list_add(NULL, lpkg); - _alpm_log(handle, ALPM_LOG_DEBUG, "adding package %s-%s to the transaction targets\n", - spkg->name, spkg->version); - trans->add = alpm_list_add(trans->add, spkg); - } - } + alpm_list_t *replacers; + replacers = check_replacers(handle, lpkg, sdb); + if(replacers) { + trans->add = alpm_list_join(trans->add, replacers); } } } -- 1.7.6
This omits the finding of matching provisions and only checks the package itself against the provided dep. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/deps.c | 23 +++++++++++++---------- lib/libalpm/deps.h | 1 + 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c index de9ae44..48e8e77 100644 --- a/lib/libalpm/deps.c +++ b/lib/libalpm/deps.c @@ -368,20 +368,23 @@ static int dep_vercmp(const char *version1, alpm_depmod_t mod, return equal; } +int _alpm_depcmp_literal(alpm_pkg_t *pkg, alpm_depend_t *dep) +{ + if(pkg->name_hash != dep->name_hash + || strcmp(pkg->name, dep->name) != 0) { + /* skip more expensive checks */ + return 0; + } + return dep_vercmp(pkg->version, dep->mod, dep->version); +} + int _alpm_depcmp(alpm_pkg_t *pkg, alpm_depend_t *dep) { alpm_list_t *i; - int satisfy = 0; + int satisfy = _alpm_depcmp_literal(pkg, dep); - /* check (pkg->name, pkg->version) */ - if(pkg->name_hash != dep->name_hash) { - /* skip more expensive checks */ - } else { - satisfy = (strcmp(pkg->name, dep->name) == 0 - && dep_vercmp(pkg->version, dep->mod, dep->version)); - if(satisfy) { - return satisfy; - } + if(satisfy) { + return satisfy; } /* check provisions, name and version if available */ diff --git a/lib/libalpm/deps.h b/lib/libalpm/deps.h index 6ef4cbb..29e69eb 100644 --- a/lib/libalpm/deps.h +++ b/lib/libalpm/deps.h @@ -36,6 +36,7 @@ int _alpm_resolvedeps(alpm_handle_t *handle, alpm_list_t *localpkgs, alpm_pkg_t alpm_list_t *preferred, alpm_list_t **packages, alpm_list_t *remove, alpm_list_t **data); alpm_depend_t *_alpm_splitdep(const char *depstring); +int _alpm_depcmp_literal(alpm_pkg_t *pkg, alpm_depend_t *dep); int _alpm_depcmp(alpm_pkg_t *pkg, alpm_depend_t *dep); #endif /* _ALPM_DEPS_H */ -- 1.7.6
When we switched to using alpm_depcmp() in resolving replacments, we had some interesting behavior with regard to providers and packages not found in repositories. Teach the replacement resolving code to not look at provisions at all to be slightly more sane. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/sync.c | 3 ++- test/pacman/tests/replace103.py | 2 -- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 46e3045..11ee817 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -134,7 +134,8 @@ static alpm_list_t *check_replacers(alpm_handle_t *handle, alpm_pkg_t *lpkg, alpm_list_t *l; for(l = alpm_pkg_get_replaces(spkg); l; l = l->next) { alpm_depend_t *replace = l->data; - if(_alpm_depcmp(lpkg, replace)) { + /* we only want to consider literal matches at this point. */ + if(_alpm_depcmp_literal(lpkg, replace)) { found = 1; break; } diff --git a/test/pacman/tests/replace103.py b/test/pacman/tests/replace103.py index 955e22d..538d103 100644 --- a/test/pacman/tests/replace103.py +++ b/test/pacman/tests/replace103.py @@ -17,5 +17,3 @@ self.addrule("PACMAN_RETCODE=0") self.addrule("PKG_EXIST=util-linux-git") self.addrule("!PKG_EXIST=util-linux") - -self.expectfailure = True -- 1.7.6
The whole first loop is trying to check literals only, so teach it to do so. Also, reorder operations to make more sense by putting the strcmp() first in the literal loop, and using a very cheap name_hash check first in the second loop. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/deps.c | 15 ++++++++++----- 1 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c index 48e8e77..704a904 100644 --- a/lib/libalpm/deps.c +++ b/lib/libalpm/deps.c @@ -561,14 +561,16 @@ static alpm_pkg_t *resolvedep(alpm_handle_t *handle, alpm_depend_t *dep, /* 1. literals */ for(i = dbs; i; i = i->next) { alpm_pkg_t *pkg = _alpm_db_get_pkgfromcache(i->data, dep->name); - if(pkg && _alpm_depcmp(pkg, dep) && !_alpm_pkg_find(excluding, pkg->name)) { + if(pkg && _alpm_depcmp_literal(pkg, dep) + && !_alpm_pkg_find(excluding, pkg->name)) { if(_alpm_pkg_should_ignore(handle, pkg)) { int install = 0; if(prompt) { QUESTION(handle->trans, ALPM_TRANS_CONV_INSTALL_IGNOREPKG, pkg, NULL, NULL, &install); } else { - _alpm_log(handle, ALPM_LOG_WARNING, _("ignoring package %s-%s\n"), pkg->name, pkg->version); + _alpm_log(handle, ALPM_LOG_WARNING, _("ignoring package %s-%s\n"), + pkg->name, pkg->version); } if(!install) { ignored = 1; @@ -582,15 +584,18 @@ static alpm_pkg_t *resolvedep(alpm_handle_t *handle, alpm_depend_t *dep, for(i = dbs; i; i = i->next) { for(j = _alpm_db_get_pkgcache(i->data); j; j = j->next) { alpm_pkg_t *pkg = j->data; - if(_alpm_depcmp(pkg, dep) && strcmp(pkg->name, dep->name) != 0 && - !_alpm_pkg_find(excluding, pkg->name)) { + /* with hash != hash, we can even skip the strcmp() as we know they can't + * possibly be the same string */ + if(pkg->name_hash != dep->name_hash && _alpm_depcmp(pkg, dep) + && !_alpm_pkg_find(excluding, pkg->name)) { if(_alpm_pkg_should_ignore(handle, pkg)) { int install = 0; if(prompt) { QUESTION(handle->trans, ALPM_TRANS_CONV_INSTALL_IGNOREPKG, pkg, NULL, NULL, &install); } else { - _alpm_log(handle, ALPM_LOG_WARNING, _("ignoring package %s-%s\n"), pkg->name, pkg->version); + _alpm_log(handle, ALPM_LOG_WARNING, _("ignoring package %s-%s\n"), + pkg->name, pkg->version); } if(!install) { ignored = 1; -- 1.7.6
Add the info that versioned replaces are now supported, as well as beefing up some of the other places touching on versioned fields. Signed-off-by: Dan McGee <dan@archlinux.org> --- doc/PKGBUILD.5.txt | 21 ++++++++++++--------- 1 files changed, 12 insertions(+), 9 deletions(-) diff --git a/doc/PKGBUILD.5.txt b/doc/PKGBUILD.5.txt index 17d0b47..60d8e53 100644 --- a/doc/PKGBUILD.5.txt +++ b/doc/PKGBUILD.5.txt @@ -178,18 +178,19 @@ disables auto detection. *conflicts (array)*:: An array of packages that will conflict with this package (i.e. they cannot both be installed at the same time). This directive follows the - same format as depends. Versioned conflicts are also supported. + same format as depends. Versioned conflicts are supported using the + operators as described in `depends`. *provides (array)*:: An array of ``virtual provisions'' this package provides. This allows 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, in the 'name=version' format. - For example, dcron can provide 'cron=2.0' to satisfy the 'cron>=2.0' - dependency of other packages. Provisions involving the `>` and `<` - operators are invalid as only specific versions of a package may be - provided. ++ +Versioned provisions are also possible, in the 'name=version' format. For +example, dcron can provide 'cron=2.0' to satisfy the 'cron>=2.0' dependency of +other packages. Provisions involving the `>` and `<` operators are invalid as +only specific versions of a package may be provided. + If the provision name appears to be a library (ends with .so), makepkg will try to find the library in the built package and append the correct @@ -199,9 +200,11 @@ version. Appending the version yourself disables auto detection. An array of packages this package should replace. This can be used to handle renamed/combined packages. For example, if the 'j2re' package is renamed to 'jre', this directive allows future upgrades to continue - as expected even though the package has moved. Sysupgrade is currently - the only pacman operation that utilizes this field, a normal sync will - not use its value. + as expected even though the package has moved. Versioned replaces are + supported using the operators as described in `depends`. ++ +Sysupgrade is currently the only pacman operation that utilizes this field. +A normal sync or upgrade will not use its value. *options (array)*:: This array allows you to override some of makepkg's default behavior -- 1.7.6
participants (1)
-
Dan McGee