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

No new behaviour introduced, everything should work exactly as before.

Signed-off-by: Benedikt Morbach ---  lib/libalpm/be_local.c   |   12 ++++++-  lib/libalpm/be_package.c |    5 ++-  lib/libalpm/be_sync.c    |    8 ++++-  lib/libalpm/package.c    |    7 +++-  src/pacman/package.c     |   18 ++++++++++-  src/pacman/util.c        |   73 ++++++++++++++++++++++++++++++++++++++-------  6 files changed, 102 insertions(+), 21 deletions(-)

diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index 21d2748..6a578ff 100644 --- a/lib/libalpm/be_local.c +++ b/lib/libalpm/be_local.c @@ -517,6 +517,12 @@ char *_alpm_local_db_pkgpath(alpm_db_t *db, alpm_pkg_t *info, const char *filena        f = alpm_list_add(f, _alpm_splitdep(line)); \  } while(1) /* note the while(1) and not (0) */

+#define READ_AND_SPLITOPTDEP(f) do { \ +       if(fgets(line, sizeof(line), fp) == NULL && !feof(fp)) goto error; \ +       if(_alpm_strip_newline(line) == 0) break; \ +       f = alpm_list_add(f, _alpm_splitoptdep(line)); \ +} while(1) /* note the while(1) and not (0) */ + This and the one in be_sync can go away if you implement my suggestion in patch #1; READ_AND_SPLITDEP can simply be reused.

 static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq)  {        FILE *fp = NULL; @@ -613,7 +619,7 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq)                        } else if(strcmp(line, "%DEPENDS%") == 0) {                                READ_AND_SPLITDEP(info->depends);                        } else if(strcmp(line, "%OPTDEPENDS%") == 0) { -                               READ_AND_STORE_ALL(info->optdepends); +                               READ_AND_SPLITOPTDEP(info->optdepends);                        } else if(strcmp(line, "%CONFLICTS%") == 0) {                                READ_AND_SPLITDEP(info->conflicts);                        } else if(strcmp(line, "%PROVIDES%") == 0) { @@ -824,7 +830,9 @@ int _alpm_local_db_write(alpm_db_t *db, alpm_pkg_t *info, alpm_dbinfrq_t inforeq                if(info->optdepends) {                        fputs("%OPTDEPENDS%\n", fp);                        for(lp = info->optdepends; lp; lp = lp->next) { -                               fprintf(fp, "%s\n", (char *)lp->data); +                               char *optstring = alpm_optdep_compute_string(lp->data); +                               fprintf(fp, "%s\n", optstring); +                               free(optstring);                        }                        fprintf(fp, "\n");                } diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index 4f530e0..be6e4c8 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -38,7 +38,7 @@  #include "log.h"  #include "handle.h"  #include "package.h" -#include "deps.h" /* _alpm_splitdep */ +#include "deps.h"

 struct package_changelog {        struct archive *archive; @@ -213,7 +213,8 @@ static int parse_descfile(alpm_handle_t *handle, struct archive *a, alpm_pkg_t *                                alpm_depend_t *dep = _alpm_splitdep(ptr);                                newpkg->depends = alpm_list_add(newpkg->depends, dep);                        } else if(strcmp(key, "optdepend") == 0) { -                               newpkg->optdepends = alpm_list_add(newpkg->optdepends, strdup(ptr)); +                               alpm_optdepend_t *optdep = _alpm_splitoptdep(ptr); +                               newpkg->optdepends = alpm_list_add(newpkg->optdepends, optdep);                        } else if(strcmp(key, "conflict") == 0) {                                alpm_depend_t *conflict = _alpm_splitdep(ptr);                                newpkg->conflicts = alpm_list_add(newpkg->conflicts, conflict); diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c index 54c4f87..0aaf085 100644 --- a/lib/libalpm/be_sync.c +++ b/lib/libalpm/be_sync.c @@ -504,6 +504,12 @@ cleanup:        f = alpm_list_add(f, _alpm_splitdep(line)); \  } while(1) /* note the while(1) and not (0) */

+#define READ_AND_SPLITOPTDEP(f) do { \ +       if(_alpm_archive_fgets(archive, &buf) != ARCHIVE_OK) goto error; \ +       if(_alpm_strip_newline(buf.line) == 0) break; \ +       f = alpm_list_add(f, _alpm_splitoptdep(line)); \ +} while(1) /* note the while(1) and not (0) */ +  static int sync_db_read(alpm_db_t *db, struct archive *archive,                struct archive_entry *entry, alpm_pkg_t **likely_pkg)  { @@ -590,7 +596,7 @@ static int sync_db_read(alpm_db_t *db, struct archive *archive,                        } else if(strcmp(line, "%DEPENDS%") == 0) {                                READ_AND_SPLITDEP(pkg->depends);                        } else if(strcmp(line, "%OPTDEPENDS%") == 0) { -                               READ_AND_STORE_ALL(pkg->optdepends); +                               READ_AND_SPLITOPTDEP(pkg->optdepends);                        } else if(strcmp(line, "%CONFLICTS%") == 0) {                                READ_AND_SPLITDEP(pkg->conflicts);                        } else if(strcmp(line, "%PROVIDES%") == 0) { diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c index 2a97177..451506b 100644 --- a/lib/libalpm/package.c +++ b/lib/libalpm/package.c @@ -531,7 +531,9 @@ int _alpm_pkg_dup(alpm_pkg_t *pkg, alpm_pkg_t **new_ptr)        for(i = pkg->depends; i; i = i->next) {                newpkg->depends = alpm_list_add(newpkg->depends, _alpm_dep_dup(i->data));        } -       newpkg->optdepends = alpm_list_strdup(pkg->optdepends); +       for(i = pkg->optdepends; i; i = i->next) { +               newpkg->optdepends = alpm_list_add(newpkg->optdepends, _alpm_optdep_dup(i->data)); +       } If we move to all of these being alpm_depend_t typed; implementing a static method to help the dup process might be worthwhile. Patch on the way; feel free to include it in your patch series next submission if it isn't merged already.

       for(i = pkg->conflicts; i; i = i->next) {                newpkg->conflicts = alpm_list_add(newpkg->conflicts, _alpm_dep_dup(i->data));        } @@ -593,7 +595,8 @@ void _alpm_pkg_free(alpm_pkg_t *pkg)        alpm_list_free(pkg->backup);        alpm_list_free_inner(pkg->depends, (alpm_list_fn_free)_alpm_dep_free);        alpm_list_free(pkg->depends); -       FREELIST(pkg->optdepends); +       alpm_list_free_inner(pkg->optdepends, (alpm_list_fn_free)_alpm_optdep_free); +       alpm_list_free(pkg->optdepends);        alpm_list_free_inner(pkg->conflicts, (alpm_list_fn_free)_alpm_dep_free);        alpm_list_free(pkg->conflicts);        alpm_list_free_inner(pkg->provides, (alpm_list_fn_free)_alpm_dep_free); diff --git a/src/pacman/package.c b/src/pacman/package.c index d4bbf88..c7bfb14 100644 --- a/src/pacman/package.c +++ b/src/pacman/package.c @@ -40,7 +40,6 @@

 /** Turn a depends list into a text list.  * @param deps a list with items of type alpm_depend_t - * @return a string list, must be freed  */  static void deplist_display(const char *title,                alpm_list_t *deps) @@ -54,6 +53,21 @@ static void deplist_display(const char *title,        FREELIST(text);  }

+/** Turn a optdepends list into a text list. + * @param optdeps a list with items of type alpm_optdepend_t + */ +static void optdeplist_display(const char *title, +               alpm_list_t *optdeps) +{ +       alpm_list_t *i, *text = NULL; +       for(i = optdeps; i; i = alpm_list_next(i)) { +               alpm_optdepend_t *optdep = i->data; +               text = alpm_list_add(text, alpm_optdep_compute_string(optdep)); +       } +       list_display_linebreak(title, text); +       FREELIST(text); +} +  /**  * Display the details of a package.  * Extra information entails 'required by' info for sync packages and backup @@ -113,7 +127,7 @@ void dump_pkg_full(alpm_pkg_t *pkg, int extra)        list_display(_("Groups         :"), alpm_pkg_get_groups(pkg));        deplist_display(_("Provides       :"), alpm_pkg_get_provides(pkg));        deplist_display(_("Depends On     :"), alpm_pkg_get_depends(pkg)); -       list_display_linebreak(_("Optional Deps  :"), alpm_pkg_get_optdepends(pkg)); +       optdeplist_display(_("Optional Deps  :"), alpm_pkg_get_optdepends(pkg));        if(extra || from == PKG_FROM_LOCALDB) {                list_display(_("Required By    :"), requiredby);        } diff --git a/src/pacman/util.c b/src/pacman/util.c index c0dcb9f..2363e6f 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1139,32 +1139,81 @@ void print_packages(const alpm_list_t *packages)        }  }

-/* Helper function for comparing strings using the - * alpm "compare func" signature */ -int str_cmp(const void *s1, const void *s2) +/* Helper function for comparing optdepends using the + * alpm "compare func" signature. */ +static int opt_cmp(const void *o1, const void *o2) optdep_cmp at least (depend_cmp if we refactor the type), "opt" all by itself is pretty hard to grok.  { -       return strcmp(s1, s2); +       const alpm_optdepend_t *od1 = o1; +       const alpm_optdepend_t *od2 = o2; +       int ret; + +       ret = strcmp(od1->depend->name, od2->depend->name); +       if(ret == 0) { +               ret = od1->depend->mod - od2->depend->mod; +       } +       if(ret == 0 && od1->depend->version != od2->depend->version) { +               if(od1->depend->version && od2->depend->version) { +                       ret = strcmp(od1->depend->version, od2->depend->version); +               } else if(!od1->depend->version && od2->depend->version) { +                       return -1; +               } else if(od1->depend->version && !od2->depend->version) { +                       return  1; +               } +       } +       if(ret == 0 && od1->description != od2->description) { +               if(od1->description && od2->description) { +                       ret = strcmp(od1->description, od2->description); +               } else if(!od1->description && od2->description) { +                       return -1; +               } else if(od1->description && !od2->description) { +                       return  1; +               } +       } + +       return ret; I feel like this could be simplified a bit, but we can tackle that later.

 }

 void display_new_optdepends(alpm_pkg_t *oldpkg, alpm_pkg_t *newpkg)  { -       alpm_list_t *old = alpm_pkg_get_optdepends(oldpkg); -       alpm_list_t *new = alpm_pkg_get_optdepends(newpkg); -       alpm_list_t *optdeps = alpm_list_diff(new,old,str_cmp); -       if(optdeps) { +       alpm_list_t *i, *old, *new, *optdeps, *optstrings = NULL; + +       old = alpm_pkg_get_optdepends(oldpkg); +       new = alpm_pkg_get_optdepends(newpkg); +       optdeps = alpm_list_diff(new, old, opt_cmp); + +       /* turn optdepends list into a text list */ +       for(i = optdeps; i; i = alpm_list_next(i)) { +               alpm_optdepend_t *optdep = i->data; +               optstrings = alpm_list_add(optstrings, alpm_optdep_compute_string(optdep)); +       } + +       if(optstrings) {                printf(_("New optional dependencies for %s\n"), alpm_pkg_get_name(newpkg)); -               list_display_linebreak("   ", optdeps); +               list_display_linebreak("   ", optstrings);        } +        alpm_list_free(optdeps); +       FREELIST(optstrings);  }

 void display_optdepends(alpm_pkg_t *pkg)  { -       alpm_list_t *optdeps = alpm_pkg_get_optdepends(pkg); -       if(optdeps) { +       alpm_list_t *i, *optdeps, *optstrings = NULL; + +       optdeps = alpm_pkg_get_optdepends(pkg); + +       /* turn optdepends list into a text list */ +       for(i = optdeps; i; i = alpm_list_next(i)) { +               alpm_optdepend_t *optdep = i->data; +               optstrings = alpm_list_add(optstrings, alpm_optdep_compute_string(optdep)); +       } + +       if(optstrings) {                printf(_("Optional dependencies for %s\n"), alpm_pkg_get_name(pkg)); -               list_display_linebreak("   ", optdeps); +               list_display_linebreak("   ", optstrings);        } + +       FREELIST(optstrings);  }

 static void display_repo_list(const char *dbname, alpm_list_t *list) -- 1.7.7.3

On Wed, Nov 23, 2011 at 9:51 AM, Benedikt Morbach
 wrote:
> Signed-off-by: Benedikt Morbach 
> ---
>  doc/pacman.8.txt              |    6 ++++++
>  lib/libalpm/alpm.h            |    2 +-
>  lib/libalpm/package.c         |   33 +++++++++++++++++++++++----------
>  src/pacman/conf.h             |    1 +
>  src/pacman/package.c          |    2 +-
>  src/pacman/pacman.c           |    3 +++
>  src/pacman/query.c            |   16 +++++++++++++---
>  src/util/pactree.c            |    2 +-
>  test/pacman/tests/query020.py |   14 ++++++++++++++
>  test/pacman/tests/query021.py |   14 ++++++++++++++
>  10 files changed, 77 insertions(+), 16 deletions(-)
>  create mode 100644 test/pacman/tests/query020.py
>  create mode 100644 test/pacman/tests/query021.py
>
> diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt
> index 39551e1..62dd908 100644
> --- a/doc/pacman.8.txt
> +++ b/doc/pacman.8.txt
> @@ -280,6 +280,12 @@ Query Options[[QO]]
>        database(s).  Typically these are packages that were downloaded manually
>        and installed with '\--upgrade'.
>
> +*-n, \--optdeps*::
> +       When using the '-t' option with this flag, do not treat installed
> +       optional dependencies as if they were normal dependencies. This option
> +       can be used to list packages which were installed as dependencies but are
> +       only optionally used by other packages.
> +
>  *-o, \--owns* ::
>        Search for packages that own the specified file(s). The path can be
>        relative or absolute and one or more files can be specified.
> diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
> index 943ceec..d06ad0d 100644
> --- a/lib/libalpm/alpm.h
> +++ b/lib/libalpm/alpm.h
> @@ -715,7 +715,7 @@ int alpm_pkg_vercmp(const char *a, const char *b);
>  * @param pkg a package
>  * @return the list of packages requiring pkg
>  */
> -alpm_list_t *alpm_pkg_compute_requiredby(alpm_pkg_t *pkg);
> +alpm_list_t *alpm_pkg_compute_requiredby(alpm_pkg_t *pkg, const int find_optdeps);
1. You didn't add the new param to the documentation
2. This is definitely not self-documenting; when I looked at the code
I was completely surprised to find it either finds depends OR
optdepends, but not both.

I'm honestly not sure overloading this *public* method makes sense.
Given you ask for alpm_pkg_get_depends() or alpm_pkg_get_optdepends(),
I would expect the reciprocals to also have two methods. Something
like alpm_pkg_compute_requiredby() and
alpm_pkg_compute_optrequiredby().

>  /** @name Package Property Accessors
>  * Any pointer returned by these functions points to internal structures
> diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c
> index 451506b..3c217b7 100644
> --- a/lib/libalpm/package.c
> +++ b/lib/libalpm/package.c
> @@ -382,7 +382,8 @@ int SYMEXPORT alpm_pkg_has_scriptlet(alpm_pkg_t *pkg)
>        return pkg->ops->has_scriptlet(pkg);
>  }
>
> -static void find_requiredby(alpm_pkg_t *pkg, alpm_db_t *db, alpm_list_t **reqs)
> +static void find_requiredby(alpm_pkg_t *pkg, alpm_db_t *db,
> +                            alpm_list_t **reqs, const int find_optdeps)
Making the int const here doesn't really help much; we don't tend to
do this in any of our functions.

>  {
>        const alpm_list_t *i;
>        pkg->handle->pm_errno = 0;
> @@ -390,11 +391,23 @@ static void find_requiredby(alpm_pkg_t *pkg, alpm_db_t *db, alpm_list_t **reqs)
>        for(i = _alpm_db_get_pkgcache(db); i; i = i->next) {
>                alpm_pkg_t *cachepkg = i->data;
>                alpm_list_t *j;
> -               for(j = alpm_pkg_get_depends(cachepkg); j; j = j->next) {
> -                       if(_alpm_depcmp(pkg, j->data)) {
> -                               const char *cachepkgname = cachepkg->name;
> -                               if(alpm_list_find_str(*reqs, cachepkgname) == NULL) {
> -                                       *reqs = alpm_list_add(*reqs, strdup(cachepkgname));
> +               if(find_optdeps) {
> +                       for(j = alpm_pkg_get_optdepends(cachepkg); j; j = j->next) {
> +                               alpm_optdepend_t *optdep = j->data;
> +                               if(_alpm_depcmp(pkg, optdep->depend)) {
> +                                       const char *cachepkgname = cachepkg->name;
> +                                       if(alpm_list_find_str(*reqs, cachepkgname) == NULL) {
> +                                               *reqs = alpm_list_add(*reqs, strdup(cachepkgname));
> +                                       }
> +                               }
> +                       }
> +               } else {
> +                       for(j = alpm_pkg_get_depends(cachepkg); j; j = j->next) {
> +                               if(_alpm_depcmp(pkg, j->data)) {
> +                                       const char *cachepkgname = cachepkg->name;
> +                                       if(alpm_list_find_str(*reqs, cachepkgname) == NULL) {
> +                                               *reqs = alpm_list_add(*reqs, strdup(cachepkgname));
> +                                       }
This won't have to change near as much if both are the same type;
simply initialize j based on find_optdeps() and keep the rest of the
loop body intact and unchanged indent.
>                                }
>                        }
>                }
> @@ -402,7 +415,7 @@ static void find_requiredby(alpm_pkg_t *pkg, alpm_db_t *db, alpm_list_t **reqs)
>  }
>
>  /** Compute the packages requiring a given package. */
> -alpm_list_t SYMEXPORT *alpm_pkg_compute_requiredby(alpm_pkg_t *pkg)
> +alpm_list_t SYMEXPORT *alpm_pkg_compute_requiredby(alpm_pkg_t *pkg, const int find_optdeps)
>  {
>        const alpm_list_t *i;
>        alpm_list_t *reqs = NULL;
> @@ -413,17 +426,17 @@ alpm_list_t SYMEXPORT *alpm_pkg_compute_requiredby(alpm_pkg_t *pkg)
>
>        if(pkg->origin == PKG_FROM_FILE) {
>                /* The sane option; search locally for things that require this. */
> -               find_requiredby(pkg, pkg->handle->db_local, &reqs);
> +               find_requiredby(pkg, pkg->handle->db_local, &reqs, find_optdeps);
>        } else {
>                /* We have a DB package. if it is a local package, then we should
>                 * only search the local DB; else search all known sync databases. */
>                db = pkg->origin_data.db;
>                if(db->status & DB_STATUS_LOCAL) {
> -                       find_requiredby(pkg, db, &reqs);
> +                       find_requiredby(pkg, db, &reqs, find_optdeps);
>                } else {
>                        for(i = pkg->handle->dbs_sync; i; i = i->next) {
>                                db = i->data;
> -                               find_requiredby(pkg, db, &reqs);
> +                               find_requiredby(pkg, db, &reqs, find_optdeps);
>                        }
>                        reqs = alpm_list_msort(reqs, alpm_list_count(reqs), _alpm_str_cmp);
>                }
> diff --git a/src/pacman/conf.h b/src/pacman/conf.h
> index 7ff37ff..9858f7d 100644
> --- a/src/pacman/conf.h
> +++ b/src/pacman/conf.h
> @@ -54,6 +54,7 @@ typedef struct __config_t {
>        unsigned short op_q_unrequired;
>        unsigned short op_q_deps;
>        unsigned short op_q_explicit;
> +       unsigned short op_q_optdeps;
>        unsigned short op_q_owns;
>        unsigned short op_q_search;
>        unsigned short op_q_changelog;
> diff --git a/src/pacman/package.c b/src/pacman/package.c
> index 49192bf..642307c 100644
> --- a/src/pacman/package.c
> +++ b/src/pacman/package.c
> @@ -109,7 +109,7 @@ void dump_pkg_full(alpm_pkg_t *pkg, int extra)
>
>        if(extra || from == PKG_FROM_LOCALDB) {
>                /* compute this here so we don't get a pause in the middle of output */
> -               requiredby = alpm_pkg_compute_requiredby(pkg);
> +               requiredby = alpm_pkg_compute_requiredby(pkg, 0);
>        }
>
>        /* actual output */
> diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
> index fa35e8d..fd93133 100644
> --- a/src/pacman/pacman.c
> +++ b/src/pacman/pacman.c
> @@ -145,6 +145,7 @@ static void usage(int op, const char * const myname)
>                        addlist(_("  -k, --check          check that the files owned by the package(s) are present\n"));
>                        addlist(_("  -l, --list           list the contents of the queried package\n"));
>                        addlist(_("  -m, --foreign        list installed packages not found in sync db(s) [filter]\n"));
> +                       addlist(_("  -n, --optdeps        don't consider installed optdepends to be required\n"));
>                        addlist(_("  -o, --owns     query the package that owns \n"));
>                        addlist(_("  -p, --file  query a package file instead of the database\n"));
>                        addlist(_("  -q, --quiet          show less information for query and search\n"));
> @@ -461,6 +462,7 @@ static int parsearg_query(int opt)
>                case 'c': config->op_q_changelog = 1; break;
>                case 'd': config->op_q_deps = 1; break;
>                case 'e': config->op_q_explicit = 1; break;
> +               case 'n': config->op_q_optdeps = 1; break;
>                case 'g': (config->group)++; break;
>                case 'i': (config->op_q_info)++; break;
>                case 'k': config->op_q_check = 1; break;
> @@ -603,6 +605,7 @@ static int parseargs(int argc, char *argv[])
>                {"list",       no_argument,       0, 'l'},
>                {"foreign",    no_argument,       0, 'm'},
>                {"nosave",     no_argument,       0, 'n'},
> +               {"optdeps",    no_argument,       0, 'n'},
-2 on using 'n' for the shortopt here:
-1) Doubling the meaning of -n
-2) This seems like a very useful and implementable flag down the line
for -R, similar to existing -c and -s, but you have precluded this by
choosing -n.

Also, the name '--optdepts' is quite deceiving, as it seems specifying
it makes pacman *ignore* them rather than include them. Quite odd from
an outside perspective.

>                {"owns",       no_argument,       0, 'o'},
>                {"file",       no_argument,       0, 'p'},
>                {"print",      no_argument,       0, 'p'},
> diff --git a/src/pacman/query.c b/src/pacman/query.c
> index 4c2ea81..fbff341 100644
> --- a/src/pacman/query.c
> +++ b/src/pacman/query.c
> @@ -363,9 +363,15 @@ static int is_foreign(alpm_pkg_t *pkg)
>        return 0;
>  }
>
> -static int is_unrequired(alpm_pkg_t *pkg)
> +static int is_unrequired(alpm_pkg_t *pkg, const int find_optdeps)
>  {
> -       alpm_list_t *requiredby = alpm_pkg_compute_requiredby(pkg);
> +       alpm_list_t *requiredby;
> +       if(find_optdeps) {
> +               requiredby = alpm_pkg_compute_requiredby(pkg, 1);
> +       } else {
> +               requiredby = alpm_pkg_compute_requiredby(pkg, 0);
> +       }
> +
>        if(requiredby == NULL) {
>                return 1;
>        }
> @@ -390,7 +396,11 @@ static int filter(alpm_pkg_t *pkg)
>                return 0;
>        }
>        /* check if this pkg is unrequired */
> -       if(config->op_q_unrequired && !is_unrequired(pkg)) {
> +       if(config->op_q_unrequired && !is_unrequired(pkg, 0)) {
> +               return 0;
> +       }
> +       /* check if this pkg is optionally required */
> +       if(config->op_q_unrequired && !config->op_q_optdeps && !is_unrequired(pkg, 1)) {
>                return 0;
>        }
>        /* check if this pkg is outdated */
> diff --git a/src/util/pactree.c b/src/util/pactree.c
> index f95c5e8..700bfe8 100644
> --- a/src/util/pactree.c
> +++ b/src/util/pactree.c
> @@ -383,7 +383,7 @@ static void walk_reverse_deps(alpm_list_t *dblist, alpm_pkg_t *pkg, int depth)
>        }
>
>        walked = alpm_list_add(walked, (void *)alpm_pkg_get_name(pkg));
> -       required_by = alpm_pkg_compute_requiredby(pkg);
> +       required_by = alpm_pkg_compute_requiredby(pkg, 0);
>
>        for(i = required_by; i; i = alpm_list_next(i)) {
>                const char *pkgname = i->data;
> diff --git a/test/pacman/tests/query020.py b/test/pacman/tests/query020.py
> new file mode 100644
> index 0000000..a6c8ffe
> --- /dev/null
> +++ b/test/pacman/tests/query020.py
> @@ -0,0 +1,14 @@
> +self.description = "Query unneeded deps (installed optdeps required)"
> +
> +p1 = pmpkg("dummy", "1.0-2")
> +p1.optdepends = ["dep: for foobar"]
> +self.addpkg2db("local", p1)
> +
> +p2 = pmpkg("dep")
> +p2.reason = 1
> +self.addpkg2db("local", p2)
> +
> +self.args = "-Qtd"
> +
> +self.addrule("PACMAN_RETCODE=1")
> +self.addrule("!PACMAN_OUTPUT=^dep")
> diff --git a/test/pacman/tests/query021.py b/test/pacman/tests/query021.py
> new file mode 100644
> index 0000000..1a0ad40
> --- /dev/null
> +++ b/test/pacman/tests/query021.py
> @@ -0,0 +1,14 @@
> +self.description = "Query unneeded deps (installed optdeps not required)"
> +
> +p1 = pmpkg("dummy", "1.0-2")
> +p1.optdepends = ["dep: for foobar"]
> +self.addpkg2db("local", p1)
> +
> +p2 = pmpkg("dep")
> +p2.reason = 1
> +self.addpkg2db("local", p2)
> +
> +self.args = "-Qtdn"
> +
> +self.addrule("PACMAN_RETCODE=0")
> +self.addrule("PACMAN_OUTPUT=^dep")
> --
> 1.7.7.3
>
>

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

Signed-off-by: Benedikt Morbach --- Can you provide some example output from this?

Overall, I'm not liking what I think this is, as it seems like an extremely narrow implementation of https://bugs.archlinux.org/task/25236 ...

 src/pacman/query.c            |   11 ++++++++++-  src/pacman/util.c             |   27 ++++++++++++++++++++-------  src/pacman/util.h             |    1 +  test/pacman/tests/query022.py |   14 ++++++++++++++  4 files changed, 45 insertions(+), 8 deletions(-)  create mode 100644 test/pacman/tests/query022.py

diff --git a/src/pacman/query.c b/src/pacman/query.c index fbff341..0017918 100644 --- a/src/pacman/query.c +++ b/src/pacman/query.c @@ -488,7 +488,16 @@ static int display(alpm_pkg_t *pkg)        if(!config->op_q_info && !config->op_q_list                        && !config->op_q_changelog && !config->op_q_check) {                if(!config->quiet) { -                       printf("%s %s\n", alpm_pkg_get_name(pkg), alpm_pkg_get_version(pkg)); +                       printf("%s %s", alpm_pkg_get_name(pkg), alpm_pkg_get_version(pkg)); + +                       alpm_list_t *optrequires; +                       if(config->op_q_unrequired && config->op_q_optdeps && +                               (optrequires = alpm_pkg_compute_requiredby(pkg, 1)) != NULL) { +                               list_display_extra(_(" (optdepend for:"), optrequires, ", ", ")"); +                               FREELIST(optrequires); +                       } else { +                               printf("\n"); +                       }                } else {                        printf("%s\n", alpm_pkg_get_name(pkg));                } diff --git a/src/pacman/util.c b/src/pacman/util.c index 1f89663..c54975e 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -657,16 +657,24 @@ int table_display(const char *title, const alpm_list_t *header,

 void list_display(const char *title, const alpm_list_t *list)  { +       list_display_extra(title, list, NULL, NULL); +} + +void list_display_extra(const char *title, const alpm_list_t *list, +                        const char *delim, const char *after) +{        const alpm_list_t *i;        size_t len = 0;

+       delim = delim ? delim : "  "; +        if(title) {                len = string_length(title) + 1;                printf("%s ", title);        }

       if(!list) { -               printf("%s\n", _("None")); +               printf("%s", _("None"));        } else {                const unsigned short maxcols = getcols();                size_t cols = len; @@ -680,20 +688,25 @@ void list_display(const char *title, const alpm_list_t *list)                        if(maxcols > len && cols + s + 2 >= maxcols) {                                size_t j;                                cols = len; -                               printf("\n"); +                               putchar('\n');                                for (j = 1; j <= len; j++) { -                                       printf(" "); +                                       putchar(' ');                                }                        } else if(cols != len) { -                               /* 2 spaces are added if this is not the first element on a line. */ -                               printf("  "); -                               cols += 2; +                               /* delimiter is added if this is not the first element on a line. */ +                               fputs(delim, stdout); +                               cols += strlen(delim);                        }                        fputs(str, stdout);                        cols += s;                } -               putchar('\n');        } + +       if(after) { +               fputs(after, stdout); +       } + +       putchar('\n');  }

 void list_display_linebreak(const char *title, const alpm_list_t *list) diff --git a/src/pacman/util.h b/src/pacman/util.h index ca33752..ffff4b5 100644 --- a/src/pacman/util.h +++ b/src/pacman/util.h @@ -63,6 +63,7 @@ void string_display(const char *title, const char *string);  double humanize_size(off_t bytes, const char target_unit, const char **label);  int table_display(const char *title, const alpm_list_t *header, const alpm_list_t *rows);  void list_display(const char *title, const alpm_list_t *list); +void list_display_extra(const char *title, const alpm_list_t *list, const char *delim, const char *after);  void list_display_linebreak(const char *title, const alpm_list_t *list);  void signature_display(const char *title, alpm_siglist_t *siglist);  void display_targets(void); diff --git a/test/pacman/tests/query022.py b/test/pacman/tests/query022.py new file mode 100644 index 0000000..f5e3458 --- /dev/null +++ b/test/pacman/tests/query022.py @@ -0,0 +1,14 @@ +self.description = "Query unneeded deps (installed optdeps not required)" + +pkg = pmpkg("dummy") +pkg.optdepends = ["dep: for foobar"] +self.addpkg2db("local", pkg) + +dep = pmpkg("dep") +self.addpkg2db("local", dep) +dep.reason = 1 + +self.args = "-Qtdn" + +self.addrule("PACMAN_RETCODE=0") +self.addrule("PACMAN_OUTPUT=^dep.*dummy") -- 1.7.7.3

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

Issue a warning if the user for whatever reason tries to remove an optdepend of an installed package.

Signed-off-by: Benedikt Morbach ---  lib/libalpm/alpm.h   |    5 ++++-  lib/libalpm/deps.c   |   35 ++++++++++++++++++++++++++---------  lib/libalpm/error.c  |    2 ++  lib/libalpm/remove.c |   16 +++++++++++++---  lib/libalpm/sync.c   |    2 +-  lib/libalpm/trans.c  |   10 ++++++++--  src/pacman/remove.c  |   23 +++++++++++++++++++----  src/util/testdb.c    |    2 +-  8 files changed, 74 insertions(+), 21 deletions(-)

diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index d06ad0d..34dbaa1 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -162,6 +162,8 @@ typedef struct _alpm_depmissing_t {        alpm_depend_t *depend;        /* this is used in case of remove dependency error only */        char *causingpkg; +       /* this is used for optdepends only */ +       char *description; This is needed only because depmissing_t points at a depend_t and not an optdepend_t? Yet another reason to make them one and the same.

 } alpm_depmissing_t;

 /** Conflict */ @@ -1096,7 +1098,7 @@ int alpm_remove_pkg(alpm_handle_t *handle, alpm_pkg_t *pkg);  */

 alpm_list_t *alpm_checkdeps(alpm_handle_t *handle, alpm_list_t *pkglist, -               alpm_list_t *remove, alpm_list_t *upgrade, int reversedeps); +               alpm_list_t *remove, alpm_list_t *upgrade, int reversedeps, int consider_optdeps);  alpm_pkg_t *alpm_find_satisfier(alpm_list_t *pkgs, const char *depstring);  alpm_pkg_t *alpm_find_dbs_satisfier(alpm_handle_t *handle,                alpm_list_t *dbs, const char *depstring); @@ -1184,6 +1186,7 @@ typedef enum _alpm_errno_t {        ALPM_ERR_DLT_PATCHFAILED,        /* Dependencies */        ALPM_ERR_UNSATISFIED_DEPS, +       ALPM_ERR_UNSATISFIED_OPTDEPS,        ALPM_ERR_CONFLICTING_DEPS,        ALPM_ERR_FILE_CONFLICTS,        /* Misc */ diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c index 83bc619..c9efce4 100644 --- a/lib/libalpm/deps.c +++ b/lib/libalpm/deps.c @@ -52,7 +52,7 @@ void _alpm_optdep_free(alpm_optdepend_t *optdep)  }

 static alpm_depmissing_t *depmiss_new(const char *target, alpm_depend_t *dep, -               const char *causingpkg) +               const char *causingpkg, const char *description)  {        alpm_depmissing_t *miss;

@@ -61,6 +61,7 @@ static alpm_depmissing_t *depmiss_new(const char *target, alpm_depend_t *dep,        STRDUP(miss->target, target, return NULL);        miss->depend = _alpm_dep_dup(dep);        STRDUP(miss->causingpkg, causingpkg, return NULL); +       STRDUP(miss->description, description, return NULL);

       return miss;  } @@ -70,6 +71,7 @@ void _alpm_depmiss_free(alpm_depmissing_t *miss)        _alpm_dep_free(miss->depend);        FREE(miss->target);        FREE(miss->causingpkg); +       FREE(miss->description);        FREE(miss);  }

@@ -283,11 +285,12 @@ alpm_pkg_t SYMEXPORT *alpm_find_satisfier(alpm_list_t *pkgs, const char *depstri  * @param remove an alpm_list_t* of packages to be removed  * @param upgrade an alpm_list_t* of packages to be upgraded (remove-then-upgrade)  * @param reversedeps handles the backward dependencies + * @param consider_optdeps handles optional dependencies instead of normal ones  * @return an alpm_list_t* of alpm_depmissing_t pointers.  */  alpm_list_t SYMEXPORT *alpm_checkdeps(alpm_handle_t *handle,                alpm_list_t *pkglist, alpm_list_t *remove, alpm_list_t *upgrade, -               int reversedeps) +               int reversedeps, int consider_optdeps)  {        alpm_list_t *i, *j;        alpm_list_t *dblist = NULL, *modified = NULL; @@ -326,7 +329,7 @@ alpm_list_t SYMEXPORT *alpm_checkdeps(alpm_handle_t *handle,                                _alpm_log(handle, ALPM_LOG_DEBUG, "checkdeps: missing dependency '%s' for package '%s'\n",                                                missdepstring, tp->name);                                free(missdepstring); -                               miss = depmiss_new(tp->name, depend, NULL); +                               miss = depmiss_new(tp->name, depend, NULL, NULL);                                baddeps = alpm_list_add(baddeps, miss);                        }                        release_filtered_depend(depend, nodepversion); @@ -338,8 +341,17 @@ alpm_list_t SYMEXPORT *alpm_checkdeps(alpm_handle_t *handle,                 * the packages listed in the requiredby field. */                for(i = dblist; i; i = i->next) {                        alpm_pkg_t *lp = i->data; -                       for(j = alpm_pkg_get_depends(lp); j; j = j->next) { -                               alpm_depend_t *depend = j->data; +                       j = consider_optdeps ? alpm_pkg_get_optdepends(lp) : alpm_pkg_get_depends(lp); +                       for(; j; j = j->next) { +                               alpm_depend_t *depend; +                               const char *description = NULL; +                               if(consider_optdeps) { +                                       alpm_optdepend_t *optdep = j->data; +                                       depend = optdep->depend; +                                       description = optdep->description; +                               } else { +                                       depend = j->data; +                               } OMG please unify depend_t and optdepend_t. :)

                               depend = filtered_depend(depend, nodepversion);                                alpm_pkg_t *causingpkg = find_dep_satisfier(modified, depend);                                /* we won't break this depend, if it is already broken, we ignore it */ @@ -350,10 +362,15 @@ alpm_list_t SYMEXPORT *alpm_checkdeps(alpm_handle_t *handle,                                   !find_dep_satisfier(dblist, depend)) {                                        alpm_depmissing_t *miss;                                        char *missdepstring = alpm_dep_compute_string(depend); -                                       _alpm_log(handle, ALPM_LOG_DEBUG, "checkdeps: transaction would break '%s' dependency of '%s'\n", -                                                       missdepstring, lp->name); +                                       if(consider_optdeps) { +                                               _alpm_log(handle, ALPM_LOG_DEBUG, "checkdeps: transaction would break '%s' optional dependency of '%s'\n", We're way over the normal line wrap width here, so please move the start of the string to the next line. +                                                                                       missdepstring, lp->name); +                                       } else { +                                               _alpm_log(handle, ALPM_LOG_DEBUG, "checkdeps: transaction would break '%s' dependency of '%s'\n", +                                                                                       missdepstring, lp->name); +                                       }                                        free(missdepstring); -                                       miss = depmiss_new(lp->name, depend, causingpkg->name); +                                       miss = depmiss_new(lp->name, depend, causingpkg->name, description);                                        baddeps = alpm_list_add(baddeps, miss);                                }                                release_filtered_depend(depend, nodepversion); @@ -801,7 +818,7 @@ int _alpm_resolvedeps(alpm_handle_t *handle, alpm_list_t *localpkgs,        for(i = alpm_list_last(*packages); i; i = i->next) {                alpm_pkg_t *tpkg = i->data;                targ = alpm_list_add(NULL, tpkg); -               deps = alpm_checkdeps(handle, localpkgs, remove, targ, 0); +               deps = alpm_checkdeps(handle, localpkgs, remove, targ, 0, 0);                alpm_list_free(targ);

               for(j = deps; j; j = j->next) { diff --git a/lib/libalpm/error.c b/lib/libalpm/error.c index 044dec7..5283392 100644 --- a/lib/libalpm/error.c +++ b/lib/libalpm/error.c @@ -136,6 +136,8 @@ const char SYMEXPORT *alpm_strerror(alpm_errno_t err)                /* Dependencies */                case ALPM_ERR_UNSATISFIED_DEPS:                        return _("could not satisfy dependencies"); +               case ALPM_ERR_UNSATISFIED_OPTDEPS: +                       return _("could not satisfy optional dependencies");                case ALPM_ERR_CONFLICTING_DEPS:                        return _("conflicting dependencies");                case ALPM_ERR_FILE_CONFLICTS: diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index d7e06bc..012f9c8 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -102,7 +102,7 @@ static int remove_prepare_cascade(alpm_handle_t *handle, alpm_list_t *lp)                alpm_list_free_inner(lp, (alpm_list_fn_free)_alpm_depmiss_free);                alpm_list_free(lp);                lp = alpm_checkdeps(handle, _alpm_db_get_pkgcache(handle->db_local), -                               trans->remove, NULL, 1); +                               trans->remove, NULL, 1, 0);        }        return 0;  } @@ -133,7 +133,7 @@ static void remove_prepare_keep_needed(alpm_handle_t *handle, alpm_list_t *lp)                alpm_list_free_inner(lp, (alpm_list_fn_free)_alpm_depmiss_free);                alpm_list_free(lp);                lp = alpm_checkdeps(handle, _alpm_db_get_pkgcache(handle->db_local), -                               trans->remove, NULL, 1); +                               trans->remove, NULL, 1, 0);        }  }

@@ -164,7 +164,7 @@ int _alpm_remove_prepare(alpm_handle_t *handle, alpm_list_t **data)                EVENT(handle, ALPM_EVENT_CHECKDEPS_START, NULL, NULL);

               _alpm_log(handle, ALPM_LOG_DEBUG, "looking for unsatisfied dependencies\n"); -               lp = alpm_checkdeps(handle, _alpm_db_get_pkgcache(db), trans->remove, NULL, 1); +               lp = alpm_checkdeps(handle, _alpm_db_get_pkgcache(db), trans->remove, NULL, 1, 0);                if(lp != NULL) {

                       if(trans->flags & ALPM_TRANS_FLAG_CASCADE) { @@ -206,6 +206,16 @@ int _alpm_remove_prepare(alpm_handle_t *handle, alpm_list_t **data)

       if(!(trans->flags & ALPM_TRANS_FLAG_NODEPS)) {                EVENT(handle, ALPM_EVENT_CHECKDEPS_DONE, NULL, NULL); +               lp = alpm_checkdeps(handle, _alpm_db_get_pkgcache(db), trans->remove, NULL, 1, 1); +               if(lp != NULL) { +                       if(data) { +                               *data = lp; +                       } else { +                               alpm_list_free_inner(lp, (alpm_list_fn_free)_alpm_depmiss_free); +                               alpm_list_free(lp); +                       } +                       RET_ERR(handle, ALPM_ERR_UNSATISFIED_OPTDEPS, -1); +               } We should not be checking deps, whether optional or required, after we have issued CHECKDEPS_DONE. This can't be done here, you need to move this to a more logical place earlier in teh method.

       }

       return 0; diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 3817ec8..b501807 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -605,7 +605,7 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data)        if(!(trans->flags & ALPM_TRANS_FLAG_NODEPS)) {                _alpm_log(handle, ALPM_LOG_DEBUG, "checking dependencies\n");                deps = alpm_checkdeps(handle, _alpm_db_get_pkgcache(handle->db_local), -                               trans->remove, trans->add, 1); +                               trans->remove, trans->add, 1, 0);                if(deps) {                        handle->pm_errno = ALPM_ERR_UNSATISFIED_DEPS;                        ret = -1; diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c index cb97a4a..40e6786 100644 --- a/lib/libalpm/trans.c +++ b/lib/libalpm/trans.c @@ -101,6 +101,7 @@ static alpm_list_t *check_arch(alpm_handle_t *handle, alpm_list_t *pkgs)  int SYMEXPORT alpm_trans_prepare(alpm_handle_t *handle, alpm_list_t **data)  {        alpm_trans_t *trans; +       int retval = 0;

       /* Sanity checks */        CHECK_HANDLE(handle, return -1); @@ -127,7 +128,12 @@ int SYMEXPORT alpm_trans_prepare(alpm_handle_t *handle, alpm_list_t **data)        if(trans->add == NULL) {                if(_alpm_remove_prepare(handle, data) == -1) {                        /* pm_errno is set by _alpm_remove_prepare() */ -                       return -1; +                       /* UNSATISFIED_OPTDEPS is nonfatal. */ +                       if(alpm_errno(handle) == ALPM_ERR_UNSATISFIED_OPTDEPS) { +                               retval = -1; +                       } else { +                         return -1; +                       } I don't like this special case at all, -1 from me implemented this way. Please use callbacks and resolve it during checkdeps resolution, rather than whatever we have going on here.

               }        }       else {                if(_alpm_sync_prepare(handle, data) == -1) { @@ -138,7 +144,7 @@ int SYMEXPORT alpm_trans_prepare(alpm_handle_t *handle, alpm_list_t **data)

       trans->state = STATE_PREPARED;

-       return 0; +       return retval;  }

 /** Commit a transaction. */ diff --git a/src/pacman/remove.c b/src/pacman/remove.c index f56c1ec..739a6eb 100644 --- a/src/pacman/remove.c +++ b/src/pacman/remove.c @@ -111,8 +111,8 @@ int pacman_remove(alpm_list_t *targets)        /* Step 2: prepare the transaction based on its type, targets and flags */        if(alpm_trans_prepare(config->handle, &data) == -1) {                alpm_errno_t err = alpm_errno(config->handle); -               pm_printf(ALPM_LOG_ERROR, _("failed to prepare transaction (%s)\n"), -                       alpm_strerror(err)); +               retval = 1; +                switch(err) {                        case ALPM_ERR_PKG_INVALID_ARCH:                                for(i = data; i; i = alpm_list_next(i)) { @@ -128,12 +128,27 @@ int pacman_remove(alpm_list_t *targets)                                        free(depstring);                                }                                break; +                       case ALPM_ERR_UNSATISFIED_OPTDEPS: +                               for(i = data; i; i = alpm_list_next(i)) { +                                       alpm_depmissing_t *miss = i->data; +                                       char *depstring = alpm_dep_compute_string(miss->depend); +                                       if(miss->description) { +                                               printf(_(":: %s: optionally requires %s (%s)\n"), miss->target, depstring, miss->description); +                                       } else { +                                               printf(_(":: %s: optionally requires %s\n"), miss->target, depstring); +                                       } +                                       free(depstring); +                               } +                               retval = 0; +                               break;                        default:                                break;                }                FREELIST(data); -               retval = 1; -               goto cleanup; +               if(retval) { +                       pm_printf(ALPM_LOG_ERROR, _("failed to prepare transaction (%s)\n"), alpm_strerror(err)); +                       goto cleanup; +               }        }

       /* Search for holdpkg in target list */ diff --git a/src/util/testdb.c b/src/util/testdb.c index b15bbe5..e1401cc 100644 --- a/src/util/testdb.c +++ b/src/util/testdb.c @@ -99,7 +99,7 @@ static int checkdeps(alpm_list_t *pkglist)        alpm_list_t *data, *i;        int ret = 0;        /* check dependencies */ -       data = alpm_checkdeps(handle, pkglist, NULL, pkglist, 0); +       data = alpm_checkdeps(handle, pkglist, NULL, pkglist, 0, 0);        for(i = data; i; i = alpm_list_next(i)) {                alpm_depmissing_t *miss = i->data;                char *depstring = alpm_dep_compute_string(miss->depend); -- 1.7.7.3

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