| Message ID | 20191127204045.501505-1-morganamilo@archlinux.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [pacman-dev,v2] pacman+libalpm: print version names for conflicting packages | expand |
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
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; 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; 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); } @@ -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); } /** @@ -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 + && 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; @@ -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); @@ -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; } } @@ -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")); 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)); } 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); 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); free(reason); } alpm_conflict_free(conflict);