On Sun, Apr 10, 2011 at 5:58 AM, Sebastian Nowicki <sebnow@gmail.com> wrote:
In addition to the names of the conflicting packages, the origin and versions will be displayed to the user.
This introduces an API change in the PM_TRANS_CONV_CONFLICT_PKG conversation callback. All three arguments were strings. The first two arguments are now pmpkg_t structures (sync and local, respectively).
Fixes FS#12536 ---
In FS#12536 there was some discussion about changing pmconflict_t to contain pmpkg_t structures rather than just strings. This would be a much larger change, which I didn't think was worthwhile for a cosmetic change.
Even so, this does introduce an API change, and a sneaky one at that. The conversation callback had three string arguments, now it has two pmpkg_t and a string. Due to these being all void pointers, end-users won't get any compiler warnings/errors if they don't update their code!
Sample output without the patch applied:
$ sudo pacman -S qemu-kvm resolving dependencies... looking for inter-conflicts... :: qemu-kvm and qemu are in conflict. Remove qemu? [y/N] n error: unresolvable package conflicts detected error: failed to prepare transaction (conflicting dependencies) :: qemu-kvm and qemu are in conflict
Sample output with the patch applied:
$ sudo ./src/pacman/pacman -S qemu-kvm resolving dependencies... looking for inter-conflicts... :: extra/qemu-kvm-0.14.0-1 and qemu-0.14.0-1 are in conflict. Remove qemu? [y/N] n error: unresolvable package conflicts detected error: failed to prepare transaction (conflicting dependencies) :: qemu-kvm and qemu are in conflict
This patch was generated against master (commit d3d18a).
lib/libalpm/sync.c | 4 ++-- src/pacman/callback.c | 31 +++++++++++++++++++++---------- 2 files changed, 23 insertions(+), 12 deletions(-)
diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 5428e40..4b54969 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -486,8 +486,8 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync pmpkg_t *sync = _alpm_pkg_find(trans->add, conflict->package1); pmpkg_t *local = _alpm_db_get_pkgfromcache(db_local, conflict->package2); int doremove = 0; - QUESTION(trans, PM_TRANS_CONV_CONFLICT_PKG, conflict->package1, - conflict->package2, conflict->reason, &doremove); + QUESTION(trans, PM_TRANS_CONV_CONFLICT_PKG, sync, local, + conflict->reason, &doremove); if(doremove) { /* append to the removes list */ _alpm_log(PM_LOG_DEBUG, "electing '%s' for removal\n", conflict->package2); diff --git a/src/pacman/callback.c b/src/pacman/callback.c index 08c1cf3..b253546 100644 --- a/src/pacman/callback.c +++ b/src/pacman/callback.c @@ -266,19 +266,30 @@ void cb_trans_conv(pmtransconv_t event, void *data1, void *data2, alpm_pkg_get_name(data2)); break; case PM_TRANS_CONV_CONFLICT_PKG: - /* data parameters: target package, local package, conflict (strings) */ + /* data parameters: target package, local package (pmpkg_t), conflict (string) */ /* print conflict only if it contains new information */ - if(strcmp(data1, data3) == 0 || strcmp(data2, data3) == 0) { - *response = noyes(_(":: %s and %s are in conflict. Remove %s?"), - (char *)data1, - (char *)data2, - (char *)data2); + if(strcmp(alpm_pkg_get_name(data1), data3) == 0 + || strcmp(alpm_pkg_get_name(data2), data3) == 0) { + *response = noyes(_(":: %s%s%s-%s and %s-%s are in conflict. Remove %s?"), + /* The db will be NULL if the package was read from file. */ + alpm_pkg_get_db(data1) == NULL ? "" : alpm_db_get_name(alpm_pkg_get_db(data1)), + alpm_pkg_get_db(data1) == NULL ? "" : "/", + alpm_pkg_get_name(data1), + alpm_pkg_get_version(data1), + alpm_pkg_get_name(data2), + alpm_pkg_get_version(data2), + alpm_pkg_get_name(data2)); } else { - *response = noyes(_(":: %s and %s are in conflict (%s). Remove %s?"), - (char *)data1, - (char *)data2, + *response = noyes(_(":: %s%s%s-%s and %s-%s are in conflict (%s). Remove %s?"), + /* The db will be NULL if the package was read from file. */ + alpm_pkg_get_db(data1) == NULL ? "" : alpm_db_get_name(alpm_pkg_get_db(data1)), + alpm_pkg_get_db(data1) == NULL ? "" : "/", + alpm_pkg_get_name(data1), + alpm_pkg_get_version(data1), + alpm_pkg_get_name(data2), + alpm_pkg_get_version(data2), This makes me cringe a bit...we should have a helper function for
Any consideration to just building the relevant string in the backend so we don't need 1) an API change and 2) the awfully ugly code in the frontend to print what you are printing? I also notice this fixes the callback case, but the other two cases touched by 12b55958d8884bd are left the same. Not quite sure what to make of that though. this, not try to do it inline in a print format string.
(char *)data3, - (char *)data2); + alpm_pkg_get_name(data2)); } break; case PM_TRANS_CONV_REMOVE_PKGS: -- 1.7.4.4