[pacman-dev] [PATCH 06/14] optdepends are not orphans unless --optdeps is specified
Dan McGee
dpmcgee at gmail.com
Wed Nov 30 22:42:23 EST 2011
On Wed, Nov 23, 2011 at 9:51 AM, Benedikt Morbach
<benedikt.morbach at googlemail.com> wrote:
> Signed-off-by: Benedikt Morbach <benedikt.morbach at googlemail.com>
> ---
> 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* <file>::
> 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 <file> query the package that owns <file>\n"));
> addlist(_(" -p, --file <package> 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
>
>
More information about the pacman-dev
mailing list