[pacman-dev] [PATCH] Partial fix for the phonon/qt issue
This patch fixes the phonon/qt issue, if all to-be-upgraded packages are explicit targets (ie. only not-yet-installed packages are pulled by resolvedeps). This condition covers the most common situations, for example it should hold with every -Su operation. After this patch sync405.py passes, but sync406.py doesn't. The work is inspired by the patch of Henning Garus, thanks for his work: http://mailman.archlinux.org/pipermail/pacman-dev/2010-February/010429.html (I moved the alpm_list_diff computation to sync.c in order to compute it only once.) Signed-off-by: Nagy Gabor <ngaba@bibl.u-szeged.hu> --- lib/libalpm/deps.c | 10 +++------- lib/libalpm/deps.h | 2 +- lib/libalpm/sync.c | 6 +++++- pactest/tests/sync405.py | 2 -- 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c index 7dc734d..26f9b16 100644 --- a/lib/libalpm/deps.c +++ b/lib/libalpm/deps.c @@ -578,7 +578,7 @@ pmpkg_t *_alpm_resolvedep(pmdepend_t *dep, alpm_list_t *dbs, /* Computes resolvable dependencies for a given package and adds that package * and those resolvable dependencies to a list. * - * @param local is the local database + * @param localpkgs is the list of local packages * @param dbs_sync are the sync databases * @param pkg is the package to resolve * @param packages is a pointer to a list of packages which will be @@ -594,7 +594,7 @@ pmpkg_t *_alpm_resolvedep(pmdepend_t *dep, alpm_list_t *dbs, * unresolvable dependency, in which case the [*packages] list will be * unmodified by this function */ -int _alpm_resolvedeps(pmdb_t *local, alpm_list_t *dbs_sync, pmpkg_t *pkg, +int _alpm_resolvedeps(alpm_list_t *localpkgs, alpm_list_t *dbs_sync, pmpkg_t *pkg, alpm_list_t *preferred, alpm_list_t **packages, alpm_list_t *remove, alpm_list_t **data) { @@ -605,10 +605,6 @@ int _alpm_resolvedeps(pmdb_t *local, alpm_list_t *dbs_sync, pmpkg_t *pkg, ALPM_LOG_FUNC; - if(local == NULL) { - return(-1); - } - if(_alpm_pkg_find(*packages, pkg->name) != NULL) { return(0); } @@ -624,7 +620,7 @@ int _alpm_resolvedeps(pmdb_t *local, alpm_list_t *dbs_sync, pmpkg_t *pkg, for(i = alpm_list_last(*packages); i; i = i->next) { pmpkg_t *tpkg = i->data; targ = alpm_list_add(NULL, tpkg); - deps = alpm_checkdeps(_alpm_db_get_pkgcache(local), 0, remove, targ); + deps = alpm_checkdeps(localpkgs, 0, remove, targ); alpm_list_free(targ); for(j = deps; j; j = j->next) { pmdepmissing_t *miss = j->data; diff --git a/lib/libalpm/deps.h b/lib/libalpm/deps.h index 6681c59..ffc3aee 100644 --- a/lib/libalpm/deps.h +++ b/lib/libalpm/deps.h @@ -49,7 +49,7 @@ void _alpm_depmiss_free(pmdepmissing_t *miss); alpm_list_t *_alpm_sortbydeps(alpm_list_t *targets, int reverse); void _alpm_recursedeps(pmdb_t *db, alpm_list_t *targs, int include_explicit); pmpkg_t *_alpm_resolvedep(pmdepend_t *dep, alpm_list_t *dbs, alpm_list_t *excluding, int prompt); -int _alpm_resolvedeps(pmdb_t *local, alpm_list_t *dbs_sync, pmpkg_t *pkg, +int _alpm_resolvedeps(alpm_list_t *localpkgs, alpm_list_t *dbs_sync, pmpkg_t *pkg, alpm_list_t *preferred, alpm_list_t **packages, alpm_list_t *remove, alpm_list_t **data); int _alpm_dep_edge(pmpkg_t *pkg1, pmpkg_t *pkg2); diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 6b625ed..be36941 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -431,17 +431,21 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync } } + /* Compute the fake local database for resolvedeps (partial fix for the phonon/qt issue) */ + alpm_list_t *localpkgs = alpm_list_diff(_alpm_db_get_pkgcache(db_local), trans->add, _alpm_pkg_cmp); + /* Resolve packages in the transaction one at a time, in addtion building up a list of packages which could not be resolved. */ for(i = trans->add; i; i = i->next) { pmpkg_t *pkg = i->data; - if(_alpm_resolvedeps(db_local, dbs_sync, pkg, trans->add, + if(_alpm_resolvedeps(localpkgs, dbs_sync, pkg, trans->add, &resolved, remove, data) == -1) { unresolvable = alpm_list_add(unresolvable, pkg); } /* Else, [resolved] now additionally contains [pkg] and all of its dependencies not already on the list */ } + alpm_list_free(localpkgs); /* If there were unresolvable top-level packages, prompt the user to see if they'd like to ignore them rather than failing the sync */ diff --git a/pactest/tests/sync405.py b/pactest/tests/sync405.py index d170cec..941a1af 100644 --- a/pactest/tests/sync405.py +++ b/pactest/tests/sync405.py @@ -22,5 +22,3 @@ self.addrule("PKG_EXIST=qt") self.addrule("PKG_EXIST=phonon") self.addrule("PKG_VERSION=qt|4.6.1-1") - -self.expectfailure = True -- 1.7.1
On Sun, May 16, 2010 at 7:33 PM, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
This patch fixes the phonon/qt issue, if all to-be-upgraded packages are explicit targets (ie. only not-yet-installed packages are pulled by resolvedeps). This condition covers the most common situations, for example it should hold with every -Su operation.
After this patch sync405.py passes, but sync406.py doesn't.
The work is inspired by the patch of Henning Garus, thanks for his work: http://mailman.archlinux.org/pipermail/pacman-dev/2010-February/010429.html (I moved the alpm_list_diff computation to sync.c in order to compute it only once.)
Signed-off-by: Nagy Gabor <ngaba@bibl.u-szeged.hu>
While not the most pretty, this does seem like the best way to fix this without doing tons of extra work. I'm fine with this if others are. Thanks for looking at this.
--- lib/libalpm/deps.c | 10 +++------- lib/libalpm/deps.h | 2 +- lib/libalpm/sync.c | 6 +++++- pactest/tests/sync405.py | 2 -- 4 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c index 7dc734d..26f9b16 100644 --- a/lib/libalpm/deps.c +++ b/lib/libalpm/deps.c @@ -578,7 +578,7 @@ pmpkg_t *_alpm_resolvedep(pmdepend_t *dep, alpm_list_t *dbs, /* Computes resolvable dependencies for a given package and adds that package * and those resolvable dependencies to a list. * - * @param local is the local database + * @param localpkgs is the list of local packages * @param dbs_sync are the sync databases * @param pkg is the package to resolve * @param packages is a pointer to a list of packages which will be @@ -594,7 +594,7 @@ pmpkg_t *_alpm_resolvedep(pmdepend_t *dep, alpm_list_t *dbs, * unresolvable dependency, in which case the [*packages] list will be * unmodified by this function */ -int _alpm_resolvedeps(pmdb_t *local, alpm_list_t *dbs_sync, pmpkg_t *pkg, +int _alpm_resolvedeps(alpm_list_t *localpkgs, alpm_list_t *dbs_sync, pmpkg_t *pkg, alpm_list_t *preferred, alpm_list_t **packages, alpm_list_t *remove, alpm_list_t **data) { @@ -605,10 +605,6 @@ int _alpm_resolvedeps(pmdb_t *local, alpm_list_t *dbs_sync, pmpkg_t *pkg,
ALPM_LOG_FUNC;
- if(local == NULL) { - return(-1); - } - if(_alpm_pkg_find(*packages, pkg->name) != NULL) { return(0); } @@ -624,7 +620,7 @@ int _alpm_resolvedeps(pmdb_t *local, alpm_list_t *dbs_sync, pmpkg_t *pkg, for(i = alpm_list_last(*packages); i; i = i->next) { pmpkg_t *tpkg = i->data; targ = alpm_list_add(NULL, tpkg); - deps = alpm_checkdeps(_alpm_db_get_pkgcache(local), 0, remove, targ); + deps = alpm_checkdeps(localpkgs, 0, remove, targ); alpm_list_free(targ); for(j = deps; j; j = j->next) { pmdepmissing_t *miss = j->data; diff --git a/lib/libalpm/deps.h b/lib/libalpm/deps.h index 6681c59..ffc3aee 100644 --- a/lib/libalpm/deps.h +++ b/lib/libalpm/deps.h @@ -49,7 +49,7 @@ void _alpm_depmiss_free(pmdepmissing_t *miss); alpm_list_t *_alpm_sortbydeps(alpm_list_t *targets, int reverse); void _alpm_recursedeps(pmdb_t *db, alpm_list_t *targs, int include_explicit); pmpkg_t *_alpm_resolvedep(pmdepend_t *dep, alpm_list_t *dbs, alpm_list_t *excluding, int prompt); -int _alpm_resolvedeps(pmdb_t *local, alpm_list_t *dbs_sync, pmpkg_t *pkg, +int _alpm_resolvedeps(alpm_list_t *localpkgs, alpm_list_t *dbs_sync, pmpkg_t *pkg, alpm_list_t *preferred, alpm_list_t **packages, alpm_list_t *remove, alpm_list_t **data); int _alpm_dep_edge(pmpkg_t *pkg1, pmpkg_t *pkg2); diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 6b625ed..be36941 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -431,17 +431,21 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync } }
+ /* Compute the fake local database for resolvedeps (partial fix for the phonon/qt issue) */ + alpm_list_t *localpkgs = alpm_list_diff(_alpm_db_get_pkgcache(db_local), trans->add, _alpm_pkg_cmp); + /* Resolve packages in the transaction one at a time, in addtion building up a list of packages which could not be resolved. */ for(i = trans->add; i; i = i->next) { pmpkg_t *pkg = i->data; - if(_alpm_resolvedeps(db_local, dbs_sync, pkg, trans->add, + if(_alpm_resolvedeps(localpkgs, dbs_sync, pkg, trans->add, &resolved, remove, data) == -1) { unresolvable = alpm_list_add(unresolvable, pkg); } /* Else, [resolved] now additionally contains [pkg] and all of its dependencies not already on the list */ } + alpm_list_free(localpkgs);
/* If there were unresolvable top-level packages, prompt the user to see if they'd like to ignore them rather than failing the sync */ diff --git a/pactest/tests/sync405.py b/pactest/tests/sync405.py index d170cec..941a1af 100644 --- a/pactest/tests/sync405.py +++ b/pactest/tests/sync405.py @@ -22,5 +22,3 @@ self.addrule("PKG_EXIST=qt") self.addrule("PKG_EXIST=phonon") self.addrule("PKG_VERSION=qt|4.6.1-1") - -self.expectfailure = True -- 1.7.1
On Mon, May 17, 2010 at 5:14 PM, Dan McGee <dpmcgee@gmail.com> wrote:
On Sun, May 16, 2010 at 7:33 PM, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
This patch fixes the phonon/qt issue, if all to-be-upgraded packages are explicit targets (ie. only not-yet-installed packages are pulled by resolvedeps). This condition covers the most common situations, for example it should hold with every -Su operation.
After this patch sync405.py passes, but sync406.py doesn't.
The work is inspired by the patch of Henning Garus, thanks for his work: http://mailman.archlinux.org/pipermail/pacman-dev/2010-February/010429.html (I moved the alpm_list_diff computation to sync.c in order to compute it only once.)
Signed-off-by: Nagy Gabor <ngaba@bibl.u-szeged.hu>
While not the most pretty, this does seem like the best way to fix this without doing tons of extra work. I'm fine with this if others are. Thanks for looking at this.
ACK
On Sun, May 16, 2010 at 7:33 PM, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
This patch fixes the phonon/qt issue, if all to-be-upgraded packages are explicit targets (ie. only not-yet-installed packages are pulled by resolvedeps). This condition covers the most common situations, for example it should hold with every -Su operation.
After this patch sync405.py passes, but sync406.py doesn't.
The work is inspired by the patch of Henning Garus, thanks for his work: http://mailman.archlinux.org/pipermail/pacman-dev/2010-February/010429.html (I moved the alpm_list_diff computation to sync.c in order to compute it only once.)
Signed-off-by: Nagy Gabor <ngaba@bibl.u-szeged.hu>
While not the most pretty, this does seem like the best way to fix this without doing tons of extra work. I'm fine with this if others are. Thanks for looking at this.
I agree, this is ugly. I think the old _alpm_resolvedeps() is also ugly because of its parameter-hell. With code restructure, we could move the loop of sync.c to deps.c, and our source-code would be less hackish (now a weakness of resolvedeps is fixed in sync.c). But since _alpm_resolvedeps() is an internal function, I am not sure this is worth the work (however, this work should be straightforward). In addition, I must mention one more thing, that I forget in the commit message: As usual, remove_unresolvable can complicate things. This whole patch assumes that all targets are resolvable (which is the "normal" case). First I thought that the remove_unresolvable case is harmless, but now I am a bit unsure about this: If a target, say foo, is unresolvable, then resolvedeps can't resolve any other target packages that depend on foo (if no other foo satisfiers exists in the sync repos), because local foo is "invisible" in the local database (because of the new list_diff). So this patch somewhat breaks our remove_unresolvable stuff. I can see no other solution (without complicated graph structure): We have to "predict" whether qt will be upgraded or not; if we predict no, then we have the phonon/qt issue, if we predict yes, then we may have the above remove_unresolvable issue. A crucial fact is that pacman (hopefully) never breaks any dependencies, because the checkconflict and checkdeps tasks run _after_ resolvedeps. Resolvedeps is simply "stupid" in some cases, but the next checks will catch its negligence, and the user has to do some manual work. Personally I still prefer the new behaviour, I have never run into remove_unresolvable situation (like most other users), and the new remove_unresolvable issue can be interpreted as a prevention for "-Sy readline" issue. If there is an unresolvable target in the target list (including -Su), the new issue can be interpreted as pacman is too safe, it won't install any targets depending on the problematic targets. So I need new ACKS. :-) NG PS: I hope my explanation about the issue was clear, if needed, I can provide a pactest file...
If there is an unresolvable target in the target list (including -Su), the new issue can be interpreted as pacman is too safe, it won't install any targets depending on the problematic targets.
A little correction: Pacman will search for other "problematic target" satisfiers, which is unwanted in some cases...
If there is an unresolvable target in the target list (including -Su), the new issue can be interpreted as pacman is too safe, it won't install any targets depending on the problematic targets.
A little correction: Pacman will search for other "problematic target" satisfiers, which is unwanted in some cases...
Sorry, forget it, the correction is wrong, pacman won't search for "problematic target" satisfiers, which is good for us... (Oh, our resolvedeps stuff is quite complicated.)
participants (3)
-
Dan McGee
-
Nagy Gabor
-
Xavier Chantry