Re: [pacman-dev] [PATCH 11/14] Add flag to recurse through optdepends

On Wed, Nov 23, 2011 at 9:51 AM, Benedikt Morbach wrote:

Signed-off-by: Benedikt Morbach ---  doc/pacman.8.txt                                 |    5 +++++  lib/libalpm/alpm.h                               |    4 +++-  lib/libalpm/deps.c                               |    8 ++++----  lib/libalpm/deps.h                               |    2 +-  lib/libalpm/remove.c                             |    6 ++++--  src/pacman/pacman.c                              |    4 ++++  test/pacman/tests/{remove055.py => remove056.py} |    6 +++---  7 files changed, 24 insertions(+), 11 deletions(-)  copy test/pacman/tests/{remove055.py => remove056.py} (69%)

diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt index 62dd908..2c1748a 100644 --- a/doc/pacman.8.txt +++ b/doc/pacman.8.txt @@ -340,6 +340,11 @@ Remove Options[[RO]]        to a backwards '\--sync' operation, and helps keep a clean system without        orphans. If you want to omit condition (B), pass this option twice.

+*-o, \--recursive-optdeps*:: +       When recursively removing packages, also remove optional dependencies that +       might still be in use by other packages. You will be warned which packages +       might still optionally use the removed ones. +  *-u, \--unneeded*::        Removes targets that are not required by any other packages.        This is mostly useful when removing a group without using the '-c' option, diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 34dbaa1..5df833a 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -1006,7 +1006,9 @@ typedef enum _alpm_transflag_t {        /** Remove also explicitly installed unneeded deps (use with ALPM_TRANS_FLAG_RECURSE). */        ALPM_TRANS_FLAG_RECURSEALL = (1 << 16),        /** Do not lock the database during the operation. */ -       ALPM_TRANS_FLAG_NOLOCK = (1 << 17) +       ALPM_TRANS_FLAG_NOLOCK = (1 << 17), +       /** Recurse through optdepends, even if needed by other packages. */ +       ALPM_TRANS_FLAG_RECURSE_OPTDEPS = (1 << 18)  } alpm_transflag_t;

 /** Returns the bitfield of flags for the current transaction. diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c index c9efce4..ee13ed9 100644 --- a/lib/libalpm/deps.c +++ b/lib/libalpm/deps.c @@ -555,7 +555,7 @@ alpm_optdepend_t *_alpm_optdep_dup(const alpm_optdepend_t *optdep)  * target list, or if if the package was explictly installed and  * include_explicit == 0 */  static int can_remove_package(alpm_db_t *db, alpm_pkg_t *pkg, -               alpm_list_t *targets, int include_explicit) +               alpm_list_t *targets, int include_explicit, int include_optdeps)  {        alpm_list_t *i;

@@ -584,7 +584,7 @@ static int can_remove_package(alpm_db_t *db, alpm_pkg_t *pkg,                if(_alpm_dep_edge(lpkg, pkg) && !_alpm_pkg_find(targets, lpkg->name)) {                        return 0;                } -               if(_alpm_optdep_edge(lpkg, pkg) && !_alpm_pkg_find(targets, lpkg->name)) { +               if(!include_optdeps && _alpm_optdep_edge(lpkg, pkg) && !_alpm_pkg_find(targets, lpkg->name)) {                        return 0;                }        } @@ -604,7 +604,7 @@ static int can_remove_package(alpm_db_t *db, alpm_pkg_t *pkg,  * @param include_explicit if 0, explicitly installed packages are not included  * @return 0 on success, -1 on errors  */ -int _alpm_recursedeps(alpm_db_t *db, alpm_list_t *targs, int include_explicit) +int _alpm_recursedeps(alpm_db_t *db, alpm_list_t *targs, int include_explicit, int include_optdeps)  {        alpm_list_t *i, *j;

@@ -617,7 +617,7 @@ int _alpm_recursedeps(alpm_db_t *db, alpm_list_t *targs, int include_explicit)                for(j = _alpm_db_get_pkgcache(db); j; j = j->next) {                        alpm_pkg_t *deppkg = j->data;                        if((_alpm_dep_edge(pkg, deppkg) || _alpm_optdep_edge(pkg, deppkg)) -                                       && can_remove_package(db, deppkg, targs, include_explicit)) { +                                       && can_remove_package(db, deppkg, targs, include_explicit, include_optdeps)) {                                alpm_pkg_t *copy;                                _alpm_log(db->handle, ALPM_LOG_DEBUG, "adding '%s' to the targets\n",                                                deppkg->name); diff --git a/lib/libalpm/deps.h b/lib/libalpm/deps.h index 69b65df..13e92b8 100644 --- a/lib/libalpm/deps.h +++ b/lib/libalpm/deps.h @@ -33,7 +33,7 @@ alpm_depend_t *_alpm_dep_dup(const alpm_depend_t *dep);  alpm_optdepend_t *_alpm_optdep_dup(const alpm_optdepend_t *optdep);  void _alpm_depmiss_free(alpm_depmissing_t *miss);  alpm_list_t *_alpm_sortbydeps(alpm_handle_t *handle, alpm_list_t *targets, int reverse); -int _alpm_recursedeps(alpm_db_t *db, alpm_list_t *targs, int include_explicit); +int _alpm_recursedeps(alpm_db_t *db, alpm_list_t *targs, int include_explicit, int include_optdeps);  int _alpm_resolvedeps(alpm_handle_t *handle, alpm_list_t *localpkgs, alpm_pkg_t *pkg,                alpm_list_t *preferred, alpm_list_t **packages, alpm_list_t *remove,                alpm_list_t **data); diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index 012f9c8..704bc69 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -155,7 +155,8 @@ int _alpm_remove_prepare(alpm_handle_t *handle, alpm_list_t **data)                        && !(trans->flags & ALPM_TRANS_FLAG_CASCADE)) {                _alpm_log(handle, ALPM_LOG_DEBUG, "finding removable dependencies\n");                if(_alpm_recursedeps(db, trans->remove, -                               trans->flags & ALPM_TRANS_FLAG_RECURSEALL)) { +                               trans->flags & ALPM_TRANS_FLAG_RECURSEALL, +                               trans->flags & ALPM_TRANS_FLAG_RECURSE_OPTDEPS)) { Might be a bit cleaner to just pass trans->flags in and do the bitflagging in the method.

                       return -1;                }        } @@ -199,7 +200,8 @@ int _alpm_remove_prepare(alpm_handle_t *handle, alpm_list_t **data)                        && (trans->flags & ALPM_TRANS_FLAG_RECURSE)) {                _alpm_log(handle, ALPM_LOG_DEBUG, "finding removable dependencies\n");                if(_alpm_recursedeps(db, trans->remove, -                                       trans->flags & ALPM_TRANS_FLAG_RECURSEALL)) { +                                       trans->flags & ALPM_TRANS_FLAG_RECURSEALL, +                                       trans->flags & ALPM_TRANS_FLAG_RECURSE_OPTDEPS)) {                        return -1;                }        } diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index fd93133..ec7a4e2 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -128,6 +128,8 @@ static void usage(int op, const char * const myname)                        addlist(_("  -n, --nosave         remove configuration files\n"));                        addlist(_("  -s, --recursive      remove unnecessary dependencies\n"                                  "                       (-ss includes explicitly installed dependencies)\n")); +                       addlist(_("  -o,                  remove optional dependencies,\n" +                                 "     --recurse-optdeps even if they might be used by other packages\n")); Whoa. You can't do this. You are making translations impossible by splitting up this option and long option in some crazy fashion that no other single option does.

This also does not deserve a precious allocation of a short option, especially one that is already used. Please drop the short option completely.

                       addlist(_("  -u, --unneeded       remove unneeded packages\n"));                } else if(op == PM_OP_UPGRADE) {                        printf("%s:  %s {-U --upgrade} [%s] <%s>\n", str_usg, myname, str_opt, str_file); @@ -510,6 +512,7 @@ static int parsearg_remove(int opt)        switch(opt) {                case 'c': config->flags |= ALPM_TRANS_FLAG_CASCADE; break;                case 'n': config->flags |= ALPM_TRANS_FLAG_NOSAVE; break; +               case 'o': config->flags |= ALPM_TRANS_FLAG_RECURSE_OPTDEPS; break;                case 's':                case OP_RECURSIVE:                        /* 's' is the legacy flag here, but since recursive is used in -S without @@ -607,6 +610,7 @@ static int parseargs(int argc, char *argv[])                {"nosave",     no_argument,       0, 'n'},                {"optdeps",    no_argument,       0, 'n'},                {"owns",       no_argument,       0, 'o'}, +               {"recurse-optdeps", no_argument,  0, 'o'},                {"file",       no_argument,       0, 'p'},                {"print",      no_argument,       0, 'p'},                {"quiet",      no_argument,       0, 'q'}, diff --git a/test/pacman/tests/remove055.py b/test/pacman/tests/remove056.py similarity index 69% copy from test/pacman/tests/remove055.py copy to test/pacman/tests/remove056.py index e8134ad..24eb36e 100644 --- a/test/pacman/tests/remove055.py +++ b/test/pacman/tests/remove056.py @@ -1,4 +1,4 @@ -self.description = "-Rs test (needed optdeps)" +self.description = "-Rs test (recurse through needed optdeps)"

 p1 = pmpkg("dummy")  p1.optdepends = ["dep: for foobar"] @@ -12,9 +12,9 @@  p3.reason = 1  self.addpkg2db("local", p3)

-self.args = "-Rs %s" % p1.name +self.args = "-Rs --recurse-optdeps %s" % p1.name

 self.addrule("PACMAN_RETCODE=0")  self.addrule("!PKG_EXIST=dummy") +self.addrule("!PKG_EXIST=dep")  self.addrule("PKG_EXIST=keep") -self.addrule("PKG_EXIST=dep") -- 1.7.7.3