On 28/11/19 6:40 am, morganamilo wrote: > When ever pacman prints a conflict, it now prints pkgname-version, > instead of just pkgname. > > alpm_conflict_t now carries pointers to alpm_pkg_t instead of just the > names of each package. > > Fixes FS#12536 (point 2) > > --- > > v2: dupe each alpm_pkg_t
One typo spotted, and a suggestion for improving output. Allan > > diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h > index 956284bd..b0aa0671 100644 > --- a/lib/libalpm/alpm.h > +++ b/lib/libalpm/alpm.h > @@ -247,10 +247,8 @@ typedef struct _alpm_depmissing_t { > > /** Conflict */ > typedef struct _alpm_conflict_t { > - unsigned long package1_hash; > - unsigned long package2_hash; > - char *package1; > - char *package2; > + alpm_pkg_t *package1; > + alpm_pkg_t *package2; > alpm_depend_t *reason; > } alpm_conflict_t; > OK > diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c > index 80827ed6..08ae86cb 100644 > --- a/lib/libalpm/conflict.c > +++ b/lib/libalpm/conflict.c > @@ -49,11 +49,8 @@ static alpm_conflict_t *conflict_new(alpm_pkg_t *pkg1, > alpm_pkg_t *pkg2, > alpm_conflict_t *conflict; > > CALLOC(conflict, 1, sizeof(alpm_conflict_t), return NULL); > - > - conflict->package1_hash = pkg1->name_hash; > - conflict->package2_hash = pkg2->name_hash; > - STRDUP(conflict->package1, pkg1->name, goto error); > - STRDUP(conflict->package2, pkg2->name, goto error); > + ASSERT(_alpm_pkg_dup(pkg1, &conflict->package1) == 0, goto error); > + ASSERT(_alpm_pkg_dup(pkg2, &conflict->package2) == 0, goto error); > conflict->reason = reason; > OK > return conflict; > @@ -69,8 +66,8 @@ error: > void SYMEXPORT alpm_conflict_free(alpm_conflict_t *conflict) > { > ASSERT(conflict != NULL, return); > - FREE(conflict->package2); > - FREE(conflict->package1); > + _alpm_pkg_free(conflict->package1); > + _alpm_pkg_free(conflict->package2); > FREE(conflict); > } > OK > @@ -79,20 +76,7 @@ void SYMEXPORT alpm_conflict_free(alpm_conflict_t > *conflict) > */ > alpm_conflict_t *_alpm_conflict_dup(const alpm_conflict_t *conflict) > { > - alpm_conflict_t *newconflict; > - CALLOC(newconflict, 1, sizeof(alpm_conflict_t), return NULL); > - > - newconflict->package1_hash = conflict->package1_hash; > - newconflict->package2_hash = conflict->package2_hash; > - STRDUP(newconflict->package1, conflict->package1, goto error); > - STRDUP(newconflict->package2, conflict->package2, goto error); > - newconflict->reason = conflict->reason; > - > - return newconflict; > - > -error: > - alpm_conflict_free(newconflict); > - return NULL; > + return conflict_new(conflict->package1, conflict->package2, > conflict->reason); OK - now a one line function! > } > > /** > @@ -108,10 +92,10 @@ static int conflict_isin(alpm_conflict_t *needle, > alpm_list_t *haystack) > alpm_list_t *i; > for(i = haystack; i; i = i->next) { > alpm_conflict_t *conflict = i->data; > - if(needle->package1_hash == conflict->package1_hash > - && needle->package2_hash == > conflict->package2_hash > - && strcmp(needle->package1, conflict->package1) > == 0 > - && strcmp(needle->package2, conflict->package2) > == 0) { > + if(needle->package1->name_hash == conflict->package2->name_hash conflict->package1->name_hash > + && needle->package2->name_hash == > conflict->package2->name_hash > + && strcmp(needle->package1->name, > conflict->package1->name) == 0 > + && strcmp(needle->package2->name, > conflict->package2->name) == 0) { > return 1; > } > } > diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c > index 97a351fe..d26a1323 100644 > --- a/lib/libalpm/sync.c > +++ b/lib/libalpm/sync.c > @@ -510,21 +510,23 @@ int _alpm_sync_prepare(alpm_handle_t *handle, > alpm_list_t **data) > > for(i = deps; i; i = i->next) { > alpm_conflict_t *conflict = i->data; > + const char *name1 = conflict->package1->name; > + const char *name2 = conflict->package2->name; > alpm_pkg_t *rsync, *sync, *sync1, *sync2; > > /* have we already removed one of the conflicting > targets? */ > - sync1 = alpm_pkg_find(trans->add, conflict->package1); > - sync2 = alpm_pkg_find(trans->add, conflict->package2); > + sync1 = alpm_pkg_find(trans->add, name1); > + sync2 = alpm_pkg_find(trans->add, name2); > if(!sync1 || !sync2) { > continue; > } > > _alpm_log(handle, ALPM_LOG_DEBUG, "conflicting packages > in the sync list: '%s' <-> '%s'\n", > - conflict->package1, conflict->package2); > + name1, name2); > > /* if sync1 provides sync2, we remove sync2 from the > targets, and vice versa */ > - alpm_depend_t *dep1 = > alpm_dep_from_string(conflict->package1); > - alpm_depend_t *dep2 = > alpm_dep_from_string(conflict->package2); > + alpm_depend_t *dep1 = alpm_dep_from_string(name1); > + alpm_depend_t *dep2 = alpm_dep_from_string(name2); > if(_alpm_depcmp(sync1, dep2)) { > rsync = sync2; > sync = sync1; OK > @@ -552,8 +554,8 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t > **data) > > /* Prints warning */ > _alpm_log(handle, ALPM_LOG_WARNING, > - _("removing '%s' from target list > because it conflicts with '%s'\n"), > - rsync->name, sync->name); > + _("removing '%s-%s' from target list > because it conflicts with '%s-%s'\n"), > + rsync->name, rsync->version, > sync->name, sync->version); > trans->add = alpm_list_remove(trans->add, rsync, > _alpm_pkg_cmp, NULL); > /* rsync is not a transaction target anymore */ > trans->unresolvable = > alpm_list_add(trans->unresolvable, rsync); OK > @@ -574,16 +576,18 @@ int _alpm_sync_prepare(alpm_handle_t *handle, > alpm_list_t **data) > .conflict = i->data > }; > alpm_conflict_t *conflict = i->data; > + const char *name1 = conflict->package1->name; > + const char *name2 = conflict->package2->name; > int found = 0; > > - /* if conflict->package2 (the local package) is not > elected for removal, > + /* if name2 (the local package) is not elected for > removal, > we ask the user */ > - if(alpm_pkg_find(trans->remove, conflict->package2)) { > + if(alpm_pkg_find(trans->remove, name2)) { > found = 1; > } > for(j = trans->add; j && !found; j = j->next) { > alpm_pkg_t *spkg = j->data; > - if(alpm_pkg_find(spkg->removes, > conflict->package2)) { > + if(alpm_pkg_find(spkg->removes, name2)) { > found = 1; > } > } OK > @@ -592,14 +596,14 @@ int _alpm_sync_prepare(alpm_handle_t *handle, > alpm_list_t **data) > } > > _alpm_log(handle, ALPM_LOG_DEBUG, "package '%s' > conflicts with '%s'\n", > - conflict->package1, conflict->package2); > + name1, name2); > > QUESTION(handle, &question); > if(question.remove) { > /* append to the removes list */ > - alpm_pkg_t *sync = alpm_pkg_find(trans->add, > conflict->package1); > - alpm_pkg_t *local = > _alpm_db_get_pkgfromcache(handle->db_local, conflict->package2); > - _alpm_log(handle, ALPM_LOG_DEBUG, "electing > '%s' for removal\n", conflict->package2); > + alpm_pkg_t *sync = alpm_pkg_find(trans->add, > name1); > + alpm_pkg_t *local = > _alpm_db_get_pkgfromcache(handle->db_local, name2); > + _alpm_log(handle, ALPM_LOG_DEBUG, "electing > '%s' for removal\n", name2); > sync->removes = alpm_list_add(sync->removes, > local); > } else { /* abort */ > _alpm_log(handle, ALPM_LOG_ERROR, > _("unresolvable package conflicts detected\n")); OK > diff --git a/src/pacman/callback.c b/src/pacman/callback.c > index 08da4bb7..812b64e3 100644 > --- a/src/pacman/callback.c > +++ b/src/pacman/callback.c > @@ -393,18 +393,22 @@ void cb_question(alpm_question_t *question) > { > alpm_question_conflict_t *q = > &question->conflict; > /* print conflict only if it contains new > information */ > - if(strcmp(q->conflict->package1, > q->conflict->reason->name) == 0 > - || > strcmp(q->conflict->package2, q->conflict->reason->name) == 0) { > - q->remove = noyes(_("%s and %s are in > conflict. Remove %s?"), > - q->conflict->package1, > - q->conflict->package2, > - q->conflict->package2); > + > if(strcmp(alpm_pkg_get_name(q->conflict->package1), > q->conflict->reason->name) == 0 > + || > strcmp(alpm_pkg_get_name(q->conflict->package2), q->conflict->reason->name) > == 0) { > + q->remove = noyes(_("%s-%s and %s-%s > are in conflict. Remove %s?"), > + > alpm_pkg_get_name(q->conflict->package1), > + > alpm_pkg_get_version(q->conflict->package1), > + > alpm_pkg_get_name(q->conflict->package2), > + > alpm_pkg_get_version(q->conflict->package2), > + > alpm_pkg_get_name(q->conflict->package2)); I had a look at this and the line is getting too long with the extra information. How about splitting the line like this: colon_printf(_("%s-%s and %s-%s are in conflict\n"), alpm_pkg_get_name(q->conflict->package1), alpm_pkg_get_version(q->conflict->package1), alpm_pkg_get_name(q->conflict->package2), alpm_pkg_get_version(q->conflict->package2)); q->remove = noyes(_("Remove %s?"), alpm_pkg_get_name(q->conflict->package2)); > } else { > - q->remove = noyes(_("%s and %s are in > conflict (%s). Remove %s?"), > - q->conflict->package1, > - q->conflict->package2, > + q->remove = noyes(_("%s-%s and %s-%s > are in conflict (%s). Remove %s?"), > + > alpm_pkg_get_name(q->conflict->package1), > + > alpm_pkg_get_version(q->conflict->package1), > + > alpm_pkg_get_name(q->conflict->package2), > + > alpm_pkg_get_version(q->conflict->package2), > > q->conflict->reason->name, > - q->conflict->package2); > + > alpm_pkg_get_name(q->conflict->package2)); > } > } > break; > diff --git a/src/pacman/database.c b/src/pacman/database.c > index 378edc8c..db8ed2de 100644 > --- a/src/pacman/database.c > +++ b/src/pacman/database.c > @@ -157,7 +157,7 @@ static int check_db_local_package_conflicts(alpm_list_t > *pkglist) > for(i = data; i; i = i->next) { > alpm_conflict_t *conflict = i->data; > pm_printf(ALPM_LOG_ERROR, "'%s' conflicts with '%s'\n", > - conflict->package1, conflict->package2); > + alpm_pkg_get_name(conflict->package1), > alpm_pkg_get_name(conflict->package2)); > ret++; > } > alpm_list_free_inner(data, (alpm_list_fn_free)alpm_conflict_free); OK > diff --git a/src/pacman/sync.c b/src/pacman/sync.c > index 4bdeff7b..73e0496e 100644 > --- a/src/pacman/sync.c > +++ b/src/pacman/sync.c > @@ -777,12 +777,19 @@ int sync_prepare_execute(void) > alpm_conflict_t *conflict = i->data; > /* only print reason if it contains new > information */ > if(conflict->reason->mod == > ALPM_DEP_MOD_ANY) { > - colon_printf(_("%s and %s are > in conflict\n"), > - > conflict->package1, conflict->package2); > + colon_printf(_("%s-%s and %s-%s > are in conflict\n"), > + > alpm_pkg_get_name(conflict->package1), > + > alpm_pkg_get_version(conflict->package1), > + > alpm_pkg_get_name(conflict->package2), > + > alpm_pkg_get_version(conflict->package2)); > } else { > char *reason = > alpm_dep_compute_string(conflict->reason); > - colon_printf(_("%s and %s are > in conflict (%s)\n"), > - > conflict->package1, conflict->package2, reason); > + colon_printf(_("%s-%s and %s-%s > are in conflict (%s)\n"), > + > alpm_pkg_get_name(conflict->package1), > + > alpm_pkg_get_version(conflict->package1), > + > alpm_pkg_get_name(conflict->package2), > + > alpm_pkg_get_version(conflict->package2), > + reason); OK