On 12/13/18 at 11:44am, Michael Straube wrote:

sync: As pointed out by Andrew Gregory there could be an error when adding duplicates if they are two separate packages with the same name. Add a check in alpm_add_pkg() to test whether the duplicate is actually the same package, and if so, log a debug message and return success to skip the package. If the duplicate is a different package return ALPM_ERR_TRANS_DUP_TARGET and treat that error just like any other error in pacman.

remove: Change alpm_remove_pkg() to just log a debug message and return success to skip duplicates. Remove the handling of ALPM_ERR_TRANS_DUP_TARGET in pacman.

Also fixes FS#49377.

Suggested-by: Andrew Gregory <andrew.gregory.8@gmail.com> Signed-off-by: Michael Straube <michael.straube@posteo.de> --- lib/libalpm/add.c | 9 ++++++++- lib/libalpm/remove.c | 3 ++- src/pacman/remove.c | 10 ++-------- src/pacman/sync.c | 11 ++--------- 4 files changed, 14 insertions(+), 19 deletions(-)

Looks good, just a couple minor notes.

diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c index e415bb17..f357c076 100644 --- a/lib/libalpm/add.c +++ b/lib/libalpm/add.c @@ -53,6 +53,7 @@ int SYMEXPORT alpm_add_pkg(alpm_handle_t *handle, alpm_pkg_t *pkg) const char *pkgname, *pkgver; alpm_trans_t *trans; alpm_pkg_t *local; + alpm_pkg_t *dup;

/* Sanity checks */ CHECK_HANDLE(handle, return -1); @@ -70,7 +71,13 @@ int SYMEXPORT alpm_add_pkg(alpm_handle_t *handle, alpm_pkg_t *pkg)

_alpm_log(handle, ALPM_LOG_DEBUG, "adding package '%s'\n", pkgname);

- if(alpm_pkg_find(trans->add, pkgname)) { + dup = alpm_pkg_find(trans->add, pkgname); + if(dup) {

This isn't an official style rule, but for short assignments that immediately get tested, we typically put them directly in the conditional.

+ if(dup == pkg) { + _alpm_log(handle, ALPM_LOG_DEBUG, _("skipping duplicate target: %s\n"), pkgname);

Debug messages shouldn't be translated.

+ return 0; + } + /* error for separate packages with the same name */ RET_ERR(handle, ALPM_ERR_TRANS_DUP_TARGET, -1); }

diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index 78ca5be7..ffdc0da5 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -74,7 +74,8 @@ int SYMEXPORT alpm_remove_pkg(alpm_handle_t *handle, alpm_pkg_t *pkg) pkgname = pkg->name;

if(alpm_pkg_find(trans->remove, pkgname)) { - RET_ERR(handle, ALPM_ERR_TRANS_DUP_TARGET, -1); + _alpm_log(handle, ALPM_LOG_DEBUG, _("skipping duplicate target: %s\n"), pkgname);

Ditto.

+ return 0; }

_alpm_log(handle, ALPM_LOG_DEBUG, "adding package %s to the transaction remove list\n", diff --git a/src/pacman/remove.c b/src/pacman/remove.c index a2269ed8..9d44cf53 100644 --- a/src/pacman/remove.c +++ b/src/pacman/remove.c @@ -44,14 +44,8 @@ static int remove_target(const char *target) if((pkg = alpm_db_get_pkg(db_local, target)) != NULL) { if(alpm_remove_pkg(config->handle, pkg) == -1) { alpm_errno_t err = alpm_errno(config->handle); - if(err == ALPM_ERR_TRANS_DUP_TARGET) { - /* just skip duplicate targets */ - pm_printf(ALPM_LOG_WARNING, _("skipping target: %s\n"), target); - return 0; - } else { - pm_printf(ALPM_LOG_ERROR, "'%s': %s\n", target, alpm_strerror(err)); - return -1; - } + pm_printf(ALPM_LOG_ERROR, "'%s': %s\n", target, alpm_strerror(err)); + return -1; } config->explicit_removes = alpm_list_add(config->explicit_removes, pkg); return 0; diff --git a/src/pacman/sync.c b/src/pacman/sync.c index 57677a42..2406fed5 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -521,15 +521,8 @@ static int process_pkg(alpm_pkg_t *pkg)

if(ret == -1) { alpm_errno_t err = alpm_errno(config->handle); - if(err == ALPM_ERR_TRANS_DUP_TARGET) { - /* just skip duplicate targets */ - pm_printf(ALPM_LOG_WARNING, _("skipping target: %s\n"), alpm_pkg_get_name(pkg)); - return 0; - } else { - pm_printf(ALPM_LOG_ERROR, "'%s': %s\n", alpm_pkg_get_name(pkg), - alpm_strerror(err)); - return 1; - } + pm_printf(ALPM_LOG_ERROR, "'%s': %s\n", alpm_pkg_get_name(pkg), alpm_strerror(err)); + return 1; } config->explicit_adds = alpm_list_add(config->explicit_adds, pkg); return 0; -- 2.20.0