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.gregor...@gmail.com>
> Signed-off-by: Michael Straube <michael.stra...@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
