[pacman-dev] [PATCH 0/6] Initial patches for better optdep support
Hi, I've send this before, but it seems like it didn't reach the list. I apologize if I'm flooding your inbox and you got this patch series twice. The pacman codebase is completely new to me, so please forgive me the obvious mistakes that I overlooked and feel free to point them out and yell at me. This implements most features from https://wiki.archlinux.org/index.php/User:Allan/Pacman_OptDepends It probably isn't quite ready to be merged yet, but I want to gather some feedback on the stuff I've already done. What is there: - No regressions afaics - Only show uninstalled optdepends during install/upgrade - In package info, show [installed] after installed optdepends - In package info (local or -ii) show packages which optionally depend on the queried package - '-Qt' doesn't consider optdepends to be orphans, unless '--nooptdeps/-n' is given What is missing: - Show which of the packages listed by '-Qtn' is an optdepend and what optdeps on it - In package info display the description alongside the "reverse optdeps" - Recursive removal of unneeded optdeps - Anything listed under "Other Ideas" - Tests (mainly needed for the package removal stuff I think) - Docs with less sucky english ;-) -- Benedikt Benedikt Morbach (6): Split optdep into alpm_depend_t and description Hook new optdepend structures up Only display uninstalled optdepends during install/upgrade Show optdep install status in package info optdepends are not orphans unless --nooptdepends is specified Make package info show optional requirements doc/pacman.8.txt | 6 ++++ lib/libalpm/alpm.h | 14 ++++++++- lib/libalpm/be_local.c | 11 +++++- lib/libalpm/be_package.c | 5 ++- lib/libalpm/be_sync.c | 7 ++++- lib/libalpm/deps.c | 70 ++++++++++++++++++++++++++++++++++++++++++ lib/libalpm/deps.h | 3 ++ lib/libalpm/package.c | 39 ++++++++++++++++------- src/pacman/conf.h | 1 + src/pacman/package.c | 13 ++++++-- src/pacman/pacman.c | 3 ++ src/pacman/query.c | 16 ++++++++-- src/pacman/util.c | 75 +++++++++++++++++++++++++++++++++++++++------- src/pacman/util.h | 1 + src/util/pactree.c | 2 +- 15 files changed, 230 insertions(+), 36 deletions(-) -- 1.7.6
This is done as a preparation to better handle optdepends. This commit should not change pacman's behaviour, as it simply adds new functions and data structures and doesn't yet hook them up anywhere. --- lib/libalpm/alpm.h | 12 +++++++++ lib/libalpm/deps.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++ lib/libalpm/deps.h | 3 ++ 3 files changed, 85 insertions(+), 0 deletions(-) diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 6e1e4bc..21f65a1 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -130,6 +130,12 @@ typedef struct _alpm_depend_t { alpm_depmod_t mod; } alpm_depend_t; +/** Optional dependency */ +typedef struct _alpm_optdepend_t { + alpm_depend_t *depend; + char *description; +} alpm_optdepend_t; + /** Missing dependency */ typedef struct _alpm_depmissing_t { char *target; @@ -1021,6 +1027,12 @@ alpm_list_t *alpm_checkconflicts(alpm_handle_t *handle, alpm_list_t *pkglist); */ char *alpm_dep_compute_string(const alpm_depend_t *dep); +/** Returns a newly allocated string representing the optional dependency information. + * @param dep a optional dependency info structure + * @return a formatted string, e.g. "sqlite: for Database support" + */ +char *alpm_optdep_compute_string(const alpm_optdepend_t *optdep); + /** @} */ /** @} */ diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c index c3681b3..2f4f53a 100644 --- a/lib/libalpm/deps.c +++ b/lib/libalpm/deps.c @@ -44,6 +44,13 @@ void _alpm_dep_free(alpm_depend_t *dep) FREE(dep); } +void _alpm_optdep_free(alpm_optdepend_t *optdep) +{ + _alpm_dep_free(optdep->depend); + FREE(optdep->description); + FREE(optdep); +} + static alpm_depmissing_t *depmiss_new(const char *target, alpm_depend_t *dep, const char *causingpkg) { @@ -466,6 +473,44 @@ alpm_depend_t *_alpm_dep_dup(const alpm_depend_t *dep) return newdep; } +alpm_optdepend_t *_alpm_splitoptdep(const char *optstring) +{ + alpm_optdepend_t *optdep; + char *depstring; + const char *descstring; + + ASSERT(optstring != NULL, return NULL); + + CALLOC(optdep, 1, sizeof(alpm_optdepend_t), return NULL); + + if ((descstring = strstr(optstring, ":")) == NULL) { + descstring = optstring + strlen(optstring); + } + + STRNDUP(depstring, optstring, descstring - optstring, return NULL); + optdep->depend = _alpm_splitdep(depstring); + FREE(depstring); + + if (*descstring == '\0') { + optdep->description = NULL; + } else { + STRDUP(optdep->description, descstring + 1, return NULL); + } + + return optdep; +} + +alpm_optdepend_t *_alpm_optdep_dup(const alpm_optdepend_t *optdep) +{ + alpm_optdepend_t *newdep; + CALLOC(newdep, 1, sizeof(alpm_optdepend_t), return NULL); + + newdep->depend = _alpm_dep_dup(optdep->depend); + STRDUP(newdep->description, optdep->description, return NULL); + + return newdep; +} + /* These parameters are messy. We check if this package, given a list of * targets and a db is safe to remove. We do NOT remove it if it is in the * target list, or if if the package was explictly installed and @@ -820,4 +865,29 @@ char SYMEXPORT *alpm_dep_compute_string(const alpm_depend_t *dep) return str; } + +/** Reverse of splitoptdep; make a optdep string from a alpm_optdepend_t struct. + * The string must be freed! + * @param optdep the optdepend to turn into a string + * @return a string-formatted optional dependency with description + */ +char SYMEXPORT *alpm_optdep_compute_string(const alpm_optdepend_t *optdep) +{ + char *str, *depstring; + size_t len; + + ASSERT(optdep != NULL, return NULL); + + depstring = alpm_dep_compute_string(optdep->depend); + + if(optdep->description != NULL) { + len = strlen(depstring) + strlen(optdep->description) + 2; + MALLOC(str, len, return NULL); + snprintf(str, len, "%s:%s", depstring, optdep->description); + FREE(depstring); + return str; + } else { + return depstring; + } +} /* vim: set ts=2 sw=2 noet: */ diff --git a/lib/libalpm/deps.h b/lib/libalpm/deps.h index 6ef4cbb..acdd25a 100644 --- a/lib/libalpm/deps.h +++ b/lib/libalpm/deps.h @@ -28,7 +28,9 @@ #include "alpm.h" void _alpm_dep_free(alpm_depend_t *dep); +void _alpm_optdep_free(alpm_optdepend_t *optdep); 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); void _alpm_recursedeps(alpm_db_t *db, alpm_list_t *targs, int include_explicit); @@ -36,6 +38,7 @@ int _alpm_resolvedeps(alpm_handle_t *handle, alpm_list_t *localpkgs, alpm_pkg_t alpm_list_t *preferred, alpm_list_t **packages, alpm_list_t *remove, alpm_list_t **data); alpm_depend_t *_alpm_splitdep(const char *depstring); +alpm_optdepend_t *_alpm_splitoptdep(const char *optstring); int _alpm_depcmp(alpm_pkg_t *pkg, alpm_depend_t *dep); #endif /* _ALPM_DEPS_H */ -- 1.7.6
On Thu, Jul 21, 2011 at 1:39 PM, Benedikt Morbach <benedikt.morbach@googlemail.com> wrote:
This is done as a preparation to better handle optdepends. This commit should not change pacman's behaviour, as it simply adds new functions and data structures and doesn't yet hook them up anywhere. --- lib/libalpm/alpm.h | 12 +++++++++ lib/libalpm/deps.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++ lib/libalpm/deps.h | 3 ++ 3 files changed, 85 insertions(+), 0 deletions(-)
diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 6e1e4bc..21f65a1 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -130,6 +130,12 @@ typedef struct _alpm_depend_t { alpm_depmod_t mod; } alpm_depend_t;
+/** Optional dependency */ +typedef struct _alpm_optdepend_t { + alpm_depend_t *depend; Haven't looked through everything yet, but it probably makes more sense to make this not a pointer- that way you can alloc and free the optdep structure in one single memory allocation.
+ char *description; +} alpm_optdepend_t; + /** Missing dependency */ typedef struct _alpm_depmissing_t { char *target; @@ -1021,6 +1027,12 @@ alpm_list_t *alpm_checkconflicts(alpm_handle_t *handle, alpm_list_t *pkglist); */ char *alpm_dep_compute_string(const alpm_depend_t *dep);
+/** Returns a newly allocated string representing the optional dependency information. + * @param dep a optional dependency info structure + * @return a formatted string, e.g. "sqlite: for Database support" + */ +char *alpm_optdep_compute_string(const alpm_optdepend_t *optdep); + /** @} */
/** @} */ diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c index c3681b3..2f4f53a 100644 --- a/lib/libalpm/deps.c +++ b/lib/libalpm/deps.c @@ -44,6 +44,13 @@ void _alpm_dep_free(alpm_depend_t *dep) FREE(dep); }
+void _alpm_optdep_free(alpm_optdepend_t *optdep) +{ + _alpm_dep_free(optdep->depend); Of course if you take my suggestion above, you won't be able to do this anymore- you will just want to do FREE(optdep->dep.name); FREE(optdep->dep.version); here.
+ FREE(optdep->description); + FREE(optdep); +} + static alpm_depmissing_t *depmiss_new(const char *target, alpm_depend_t *dep, const char *causingpkg) { @@ -466,6 +473,44 @@ alpm_depend_t *_alpm_dep_dup(const alpm_depend_t *dep) return newdep; }
+alpm_optdepend_t *_alpm_splitoptdep(const char *optstring) +{ + alpm_optdepend_t *optdep; + char *depstring; + const char *descstring; + + ASSERT(optstring != NULL, return NULL); + + CALLOC(optdep, 1, sizeof(alpm_optdepend_t), return NULL); + + if ((descstring = strstr(optstring, ":")) == NULL) { + descstring = optstring + strlen(optstring); + } I don't like this- it differs from how we do things in splitdep(), which is to leave version NULL if we didn't have one. We should do the same with the description field here.
+ + STRNDUP(depstring, optstring, descstring - optstring, return NULL); + optdep->depend = _alpm_splitdep(depstring); Err...now I see what is going on. It seems to be it would be better in some fashion if splitdep() just handled (and ignored?) the version part, or something along those lines.
I also see another problem here- now that we have epoch and it uses a ':' separator, this simple logic here is going to fall apart.
+ FREE(depstring); + + if (*descstring == '\0') { + optdep->description = NULL; calloc already nulled this out, so you don't need to do it again.
+ } else { + STRDUP(optdep->description, descstring + 1, return NULL); + } + + return optdep; +} + +alpm_optdepend_t *_alpm_optdep_dup(const alpm_optdepend_t *optdep) +{ + alpm_optdepend_t *newdep; + CALLOC(newdep, 1, sizeof(alpm_optdepend_t), return NULL); + + newdep->depend = _alpm_dep_dup(optdep->depend); + STRDUP(newdep->description, optdep->description, return NULL); + + return newdep; +} + /* These parameters are messy. We check if this package, given a list of * targets and a db is safe to remove. We do NOT remove it if it is in the * target list, or if if the package was explictly installed and @@ -820,4 +865,29 @@ char SYMEXPORT *alpm_dep_compute_string(const alpm_depend_t *dep)
return str; } + +/** Reverse of splitoptdep; make a optdep string from a alpm_optdepend_t struct. + * The string must be freed! + * @param optdep the optdepend to turn into a string + * @return a string-formatted optional dependency with description + */ +char SYMEXPORT *alpm_optdep_compute_string(const alpm_optdepend_t *optdep) +{ + char *str, *depstring; + size_t len; + + ASSERT(optdep != NULL, return NULL); What's going on here with all the extra spaces?
+ + depstring = alpm_dep_compute_string(optdep->depend); + + if(optdep->description != NULL) { + len = strlen(depstring) + strlen(optdep->description) + 2; + MALLOC(str, len, return NULL); + snprintf(str, len, "%s:%s", depstring, optdep->description); Don't we usually print a single space after the colon? I thought that was our established format. + FREE(depstring); + return str; + } else { + return depstring; + } This is nitpicking a bit, but a few things here. str can be defined inside the if block since it is only used there. The else is not strictly necessary as you returned from within the if anyway. Finally, you can use free() instead of FREE() (although the complier should be smart enough anyway to optimize away the unnecessary NULL-ing).
And it looks best/easiest to just ignore my struct in struct comment at the start of this- so many of our dep functions are allocating the object already that is isn't worth changing all of them around right now to accommodate a slightly smaller memory footprint.
+} /* vim: set ts=2 sw=2 noet: */ diff --git a/lib/libalpm/deps.h b/lib/libalpm/deps.h index 6ef4cbb..acdd25a 100644 --- a/lib/libalpm/deps.h +++ b/lib/libalpm/deps.h @@ -28,7 +28,9 @@ #include "alpm.h"
void _alpm_dep_free(alpm_depend_t *dep); +void _alpm_optdep_free(alpm_optdepend_t *optdep); 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); void _alpm_recursedeps(alpm_db_t *db, alpm_list_t *targs, int include_explicit); @@ -36,6 +38,7 @@ int _alpm_resolvedeps(alpm_handle_t *handle, alpm_list_t *localpkgs, alpm_pkg_t alpm_list_t *preferred, alpm_list_t **packages, alpm_list_t *remove, alpm_list_t **data); alpm_depend_t *_alpm_splitdep(const char *depstring); +alpm_optdepend_t *_alpm_splitoptdep(const char *optstring); int _alpm_depcmp(alpm_pkg_t *pkg, alpm_depend_t *dep);
#endif /* _ALPM_DEPS_H */ -- 1.7.6
No new behaviour introduced, everything should work exactly as before. --- lib/libalpm/be_local.c | 11 ++++++++- lib/libalpm/be_package.c | 5 ++- lib/libalpm/be_sync.c | 7 +++++- lib/libalpm/package.c | 7 ++++- src/pacman/package.c | 11 ++++++++- src/pacman/util.c | 51 ++++++++++++++++++++++++++++++++++++---------- 6 files changed, 72 insertions(+), 20 deletions(-) diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index 70f242d..7e4812b 100644 --- a/lib/libalpm/be_local.c +++ b/lib/libalpm/be_local.c @@ -610,7 +610,12 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq) info->depends = alpm_list_add(info->depends, _alpm_splitdep(line)); } } else if(strcmp(line, "%OPTDEPENDS%") == 0) { - READ_AND_STORE_ALL(info->optdepends); + /* Different than the rest because of the _alpm_splitoptdep call. */ + while(1) { + READ_NEXT(); + if(strlen(line) == 0) break; + info->optdepends = alpm_list_add(info->optdepends, _alpm_splitoptdep(line)); + } } else if(strcmp(line, "%CONFLICTS%") == 0) { READ_AND_STORE_ALL(info->conflicts); } else if(strcmp(line, "%PROVIDES%") == 0) { @@ -800,7 +805,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 46bdaed..438960c 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -35,7 +35,7 @@ #include "log.h" #include "handle.h" #include "package.h" -#include "deps.h" /* _alpm_splitdep */ +#include "deps.h" /* _alpm_splitdep _alpm_splitoptdep */ /** * Open a package changelog for reading. Similar to fopen in functionality, @@ -192,7 +192,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) { newpkg->conflicts = alpm_list_add(newpkg->conflicts, strdup(ptr)); } else if(strcmp(key, "replaces") == 0) { diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c index daed01f..3a0c2b4 100644 --- a/lib/libalpm/be_sync.c +++ b/lib/libalpm/be_sync.c @@ -555,7 +555,12 @@ static int sync_db_read(alpm_db_t *db, struct archive *archive, pkg->depends = alpm_list_add(pkg->depends, _alpm_splitdep(line)); } } else if(strcmp(line, "%OPTDEPENDS%") == 0) { - READ_AND_STORE_ALL(pkg->optdepends); + /* Different than the rest because of the _alpm_splitoptdep call. */ + while(1) { + READ_NEXT(); + if(strlen(line) == 0) break; + pkg->optdepends = alpm_list_add(pkg->optdepends, _alpm_splitoptdep(line)); + } } else if(strcmp(line, "%CONFLICTS%") == 0) { READ_AND_STORE_ALL(pkg->conflicts); } else if(strcmp(line, "%PROVIDES%") == 0) { diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c index ae9b9a9..a64d613 100644 --- a/lib/libalpm/package.c +++ b/lib/libalpm/package.c @@ -502,7 +502,9 @@ alpm_pkg_t *_alpm_pkg_dup(alpm_pkg_t *pkg) for(i = pkg->depends; i; i = alpm_list_next(i)) { 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 = alpm_list_next(i)) { + newpkg->optdepends = alpm_list_add(newpkg->optdepends, _alpm_optdep_dup(i->data)); + } newpkg->conflicts = alpm_list_strdup(pkg->conflicts); newpkg->provides = alpm_list_strdup(pkg->provides); for(i = pkg->deltas; i; i = alpm_list_next(i)) { @@ -551,7 +553,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); FREELIST(pkg->conflicts); FREELIST(pkg->provides); alpm_list_free_inner(pkg->deltas, (alpm_list_fn_free)_alpm_delta_free); diff --git a/src/pacman/package.c b/src/pacman/package.c index afbac6b..6e6d379 100644 --- a/src/pacman/package.c +++ b/src/pacman/package.c @@ -53,7 +53,7 @@ void dump_pkg_full(alpm_pkg_t *pkg, enum pkg_from from, int extra) const char *label; double size; const alpm_list_t *i; - alpm_list_t *requiredby = NULL, *depstrings = NULL; + alpm_list_t *depstrings = NULL, *optstrings = NULL, *requiredby = NULL; if(pkg == NULL) { return; @@ -87,6 +87,12 @@ void dump_pkg_full(alpm_pkg_t *pkg, enum pkg_from from, int extra) depstrings = alpm_list_add(depstrings, alpm_dep_compute_string(dep)); } + /* turn optdepends list into a text list */ + for(i = alpm_pkg_get_optdepends(pkg); i; i = alpm_list_next(i)) { + alpm_optdepend_t *optdep = (alpm_optdepend_t *)alpm_list_getdata(i); + optstrings = alpm_list_add(optstrings, alpm_optdep_compute_string(optdep)); + } + 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); @@ -104,7 +110,7 @@ void dump_pkg_full(alpm_pkg_t *pkg, enum pkg_from from, int extra) list_display(_("Groups :"), alpm_pkg_get_groups(pkg)); list_display(_("Provides :"), alpm_pkg_get_provides(pkg)); list_display(_("Depends On :"), depstrings); - list_display_linebreak(_("Optional Deps :"), alpm_pkg_get_optdepends(pkg)); + list_display_linebreak(_("Optional Deps :"), optstrings); if(extra || from == PKG_FROM_LOCALDB) { list_display(_("Required By :"), requiredby); } @@ -158,6 +164,7 @@ void dump_pkg_full(alpm_pkg_t *pkg, enum pkg_from from, int extra) printf("\n"); FREELIST(depstrings); + FREELIST(optstrings); FREELIST(requiredby); } diff --git a/src/pacman/util.c b/src/pacman/util.c index 7065abd..14dcf94 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -974,32 +974,61 @@ void print_packages(const alpm_list_t *packages) } } -/* Helper function for comparing strings using the +/* Helper function for comparing optdepends using the * alpm "compare func" signature */ -int str_cmp(const void *s1, const void *s2) +int opt_cmp(const void *o1, const void *o2) { - return strcmp(s1, s2); + char *str1 = alpm_optdep_compute_string((alpm_optdepend_t*)o1); + char *str2 = alpm_optdep_compute_string((alpm_optdepend_t*)o2); + int ret = strcmp(str1, str2); + + free(str1); + free(str2); + + return ret; } 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 = (alpm_optdepend_t *)alpm_list_getdata(i); + 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 = (alpm_optdepend_t *)alpm_list_getdata(i); + 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.6
No new behaviour introduced, everything should work exactly as before. --- lib/libalpm/be_local.c | 11 ++++++++- lib/libalpm/be_package.c | 5 ++- lib/libalpm/be_sync.c | 7 +++++- lib/libalpm/package.c | 7 ++++- src/pacman/package.c | 11 ++++++++- src/pacman/util.c | 51 ++++++++++++++++++++++++++++++++++++---------- 6 files changed, 72 insertions(+), 20 deletions(-)
diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index 70f242d..7e4812b 100644 --- a/lib/libalpm/be_local.c +++ b/lib/libalpm/be_local.c @@ -610,7 +610,12 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq) info->depends = alpm_list_add(info->depends, _alpm_splitdep(line)); } } else if(strcmp(line, "%OPTDEPENDS%") == 0) { - READ_AND_STORE_ALL(info->optdepends); + /* Different than the rest because of the _alpm_splitoptdep call. */ + while(1) { + READ_NEXT(); + if(strlen(line) == 0) break; + info->optdepends = alpm_list_add(info->optdepends, _alpm_splitoptdep(line)); + } } else if(strcmp(line, "%CONFLICTS%") == 0) { READ_AND_STORE_ALL(info->conflicts); } else if(strcmp(line, "%PROVIDES%") == 0) { @@ -800,7 +805,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 46bdaed..438960c 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -35,7 +35,7 @@ #include "log.h" #include "handle.h" #include "package.h" -#include "deps.h" /* _alpm_splitdep */ +#include "deps.h" /* _alpm_splitdep _alpm_splitoptdep */ Use a comma if you update this comment, but really you can just kill
On Thu, Jul 21, 2011 at 1:39 PM, Benedikt Morbach <benedikt.morbach@googlemail.com> wrote: the comment bit completely.
/** * Open a package changelog for reading. Similar to fopen in functionality, @@ -192,7 +192,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) { newpkg->conflicts = alpm_list_add(newpkg->conflicts, strdup(ptr)); } else if(strcmp(key, "replaces") == 0) { diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c index daed01f..3a0c2b4 100644 --- a/lib/libalpm/be_sync.c +++ b/lib/libalpm/be_sync.c @@ -555,7 +555,12 @@ static int sync_db_read(alpm_db_t *db, struct archive *archive, pkg->depends = alpm_list_add(pkg->depends, _alpm_splitdep(line)); } } else if(strcmp(line, "%OPTDEPENDS%") == 0) { - READ_AND_STORE_ALL(pkg->optdepends); + /* Different than the rest because of the _alpm_splitoptdep call. */ + while(1) { + READ_NEXT(); + if(strlen(line) == 0) break; + pkg->optdepends = alpm_list_add(pkg->optdepends, _alpm_splitoptdep(line)); + } } else if(strcmp(line, "%CONFLICTS%") == 0) { READ_AND_STORE_ALL(pkg->conflicts); } else if(strcmp(line, "%PROVIDES%") == 0) { diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c index ae9b9a9..a64d613 100644 --- a/lib/libalpm/package.c +++ b/lib/libalpm/package.c @@ -502,7 +502,9 @@ alpm_pkg_t *_alpm_pkg_dup(alpm_pkg_t *pkg) for(i = pkg->depends; i; i = alpm_list_next(i)) { 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 = alpm_list_next(i)) { + newpkg->optdepends = alpm_list_add(newpkg->optdepends, _alpm_optdep_dup(i->data)); + } newpkg->conflicts = alpm_list_strdup(pkg->conflicts); newpkg->provides = alpm_list_strdup(pkg->provides); for(i = pkg->deltas; i; i = alpm_list_next(i)) { @@ -551,7 +553,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); FREELIST(pkg->conflicts); FREELIST(pkg->provides); alpm_list_free_inner(pkg->deltas, (alpm_list_fn_free)_alpm_delta_free); diff --git a/src/pacman/package.c b/src/pacman/package.c index afbac6b..6e6d379 100644 --- a/src/pacman/package.c +++ b/src/pacman/package.c @@ -53,7 +53,7 @@ void dump_pkg_full(alpm_pkg_t *pkg, enum pkg_from from, int extra) const char *label; double size; const alpm_list_t *i; - alpm_list_t *requiredby = NULL, *depstrings = NULL; + alpm_list_t *depstrings = NULL, *optstrings = NULL, *requiredby = NULL;
if(pkg == NULL) { return; @@ -87,6 +87,12 @@ void dump_pkg_full(alpm_pkg_t *pkg, enum pkg_from from, int extra) depstrings = alpm_list_add(depstrings, alpm_dep_compute_string(dep)); }
+ /* turn optdepends list into a text list */ + for(i = alpm_pkg_get_optdepends(pkg); i; i = alpm_list_next(i)) { + alpm_optdepend_t *optdep = (alpm_optdepend_t *)alpm_list_getdata(i); + optstrings = alpm_list_add(optstrings, alpm_optdep_compute_string(optdep)); + } + 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); @@ -104,7 +110,7 @@ void dump_pkg_full(alpm_pkg_t *pkg, enum pkg_from from, int extra) list_display(_("Groups :"), alpm_pkg_get_groups(pkg)); list_display(_("Provides :"), alpm_pkg_get_provides(pkg)); list_display(_("Depends On :"), depstrings); - list_display_linebreak(_("Optional Deps :"), alpm_pkg_get_optdepends(pkg)); + list_display_linebreak(_("Optional Deps :"), optstrings); if(extra || from == PKG_FROM_LOCALDB) { list_display(_("Required By :"), requiredby); } @@ -158,6 +164,7 @@ void dump_pkg_full(alpm_pkg_t *pkg, enum pkg_from from, int extra) printf("\n");
FREELIST(depstrings); + FREELIST(optstrings); FREELIST(requiredby); }
diff --git a/src/pacman/util.c b/src/pacman/util.c index 7065abd..14dcf94 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -974,32 +974,61 @@ void print_packages(const alpm_list_t *packages) } }
-/* Helper function for comparing strings using the +/* Helper function for comparing optdepends using the * alpm "compare func" signature */ -int str_cmp(const void *s1, const void *s2) +int opt_cmp(const void *o1, const void *o2) { - return strcmp(s1, s2); + char *str1 = alpm_optdep_compute_string((alpm_optdepend_t*)o1); + char *str2 = alpm_optdep_compute_string((alpm_optdepend_t*)o2); + int ret = strcmp(str1, str2); + + free(str1); + free(str2); + + return ret;
Something feels a bit off with this logic. Wouldn't we really just want to compare by the name portion? Even if we want to compare by more, we can just do it with three fallthrough strcmp()s which is a bit better than having to compute and free these strings over and over. e.g.: int ret; alpm od *od1 = o1; alpm_od *od2 = o2; ret = strcmp(od1->name, od2->name); if(!ret && od1->version && od2->version) { ret = strcmp(od1->version, od2->version); } if(!ret && od1->desc && od2->desc) { ret = strcmp(od1->desc, od2->desc); } return ret;
}
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); Since we're changing this line, can you fix the coding style and use spaces after commas please?
+ + /* turn optdepends list into a text list */ + for(i = optdeps; i; i = alpm_list_next(i)) { + alpm_optdepend_t *optdep = (alpm_optdepend_t *)alpm_list_getdata(i); No need for the cast here, it is implied in C.
+ 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 = (alpm_optdepend_t *)alpm_list_getdata(i); Same as above, no need for casting.
+ 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.6
--- src/pacman/util.c | 41 ++++++++++++++++++++++++++++------------- src/pacman/util.h | 1 + 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/src/pacman/util.c b/src/pacman/util.c index 14dcf94..d3a5648 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -990,17 +990,13 @@ int opt_cmp(const void *o1, const void *o2) void display_new_optdepends(alpm_pkg_t *oldpkg, alpm_pkg_t *newpkg) { - alpm_list_t *i, *old, *new, *optdeps, *optstrings = NULL; + alpm_list_t *old, *new, *optdeps, *optstrings; 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 = (alpm_optdepend_t *)alpm_list_getdata(i); - optstrings = alpm_list_add(optstrings, alpm_optdep_compute_string(optdep)); - } + optstrings = optdep_string_list(optdeps); if(optstrings) { printf(_("New optional dependencies for %s\n"), alpm_pkg_get_name(newpkg)); @@ -1013,15 +1009,10 @@ void display_new_optdepends(alpm_pkg_t *oldpkg, alpm_pkg_t *newpkg) void display_optdepends(alpm_pkg_t *pkg) { - alpm_list_t *i, *optdeps, *optstrings = NULL; + alpm_list_t *optdeps, *optstrings; 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 = (alpm_optdepend_t *)alpm_list_getdata(i); - optstrings = alpm_list_add(optstrings, alpm_optdep_compute_string(optdep)); - } + optstrings = optdep_string_list(optdeps); if(optstrings) { printf(_("Optional dependencies for %s\n"), alpm_pkg_get_name(pkg)); @@ -1031,6 +1022,30 @@ void display_optdepends(alpm_pkg_t *pkg) FREELIST(optstrings); } +/* Creates a human readable list from a alpm_list_t of optdepends + * include_installed is false, installed packages are excluded from the list + * the returned list has to be freed! + */ + +alpm_list_t *optdep_string_list(const alpm_list_t *optlist) +{ + alpm_list_t *optstrings = NULL; + alpm_optdepend_t *optdep; + alpm_db_t *db_local; + + db_local = alpm_option_get_localdb(config->handle); + + /* turn optdepends list into a text list */ + for( ; optlist; optlist = alpm_list_next(optlist)) { + optdep = (alpm_optdepend_t *)alpm_list_getdata(optlist); + if(alpm_db_get_pkg(db_local, optdep->depend->name) == NULL) { + optstrings = alpm_list_add(optstrings, alpm_optdep_compute_string(optdep)); + } + } + + return optstrings; +} + static void display_repo_list(const char *dbname, alpm_list_t *list) { const char *prefix= " "; diff --git a/src/pacman/util.h b/src/pacman/util.h index ee3dbd1..ed7c01f 100644 --- a/src/pacman/util.h +++ b/src/pacman/util.h @@ -62,6 +62,7 @@ void display_targets(const alpm_list_t *pkgs, int install); int str_cmp(const void *s1, const void *s2); void display_new_optdepends(alpm_pkg_t *oldpkg, alpm_pkg_t *newpkg); void display_optdepends(alpm_pkg_t *pkg); +alpm_list_t *optdep_string_list(const alpm_list_t *optdeps); void print_packages(const alpm_list_t *packages); void select_display(const alpm_list_t *pkglist); int select_question(int count); -- 1.7.6
On Thu, Jul 21, 2011 at 1:39 PM, Benedikt Morbach <benedikt.morbach@googlemail.com> wrote:
--- src/pacman/util.c | 41 ++++++++++++++++++++++++++++------------- src/pacman/util.h | 1 + 2 files changed, 29 insertions(+), 13 deletions(-)
diff --git a/src/pacman/util.c b/src/pacman/util.c index 14dcf94..d3a5648 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -990,17 +990,13 @@ int opt_cmp(const void *o1, const void *o2)
void display_new_optdepends(alpm_pkg_t *oldpkg, alpm_pkg_t *newpkg) { - alpm_list_t *i, *old, *new, *optdeps, *optstrings = NULL; + alpm_list_t *old, *new, *optdeps, *optstrings;
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 = (alpm_optdepend_t *)alpm_list_getdata(i); - optstrings = alpm_list_add(optstrings, alpm_optdep_compute_string(optdep)); - } + optstrings = optdep_string_list(optdeps);
if(optstrings) { printf(_("New optional dependencies for %s\n"), alpm_pkg_get_name(newpkg)); @@ -1013,15 +1009,10 @@ void display_new_optdepends(alpm_pkg_t *oldpkg, alpm_pkg_t *newpkg)
void display_optdepends(alpm_pkg_t *pkg) { - alpm_list_t *i, *optdeps, *optstrings = NULL; + alpm_list_t *optdeps, *optstrings;
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 = (alpm_optdepend_t *)alpm_list_getdata(i); - optstrings = alpm_list_add(optstrings, alpm_optdep_compute_string(optdep)); - } + optstrings = optdep_string_list(optdeps);
if(optstrings) { printf(_("Optional dependencies for %s\n"), alpm_pkg_get_name(pkg)); @@ -1031,6 +1022,30 @@ void display_optdepends(alpm_pkg_t *pkg) FREELIST(optstrings); }
+/* Creates a human readable list from a alpm_list_t of optdepends + * include_installed is false, installed packages are excluded from the list + * the returned list has to be freed! + */ + No blank line between comment and function signature, please. You're also missing something here- I see no 'include_installed" bits at all. Please also use punctuation (e.g. periods at end of sentences), and even better yet would be making this Doxygen-style and using @param and such. +alpm_list_t *optdep_string_list(const alpm_list_t *optlist) +{ + alpm_list_t *optstrings = NULL; + alpm_optdepend_t *optdep; + alpm_db_t *db_local; + + db_local = alpm_option_get_localdb(config->handle); + + /* turn optdepends list into a text list */ + for( ; optlist; optlist = alpm_list_next(optlist)) { + optdep = (alpm_optdepend_t *)alpm_list_getdata(optlist); No cast needed. + if(alpm_db_get_pkg(db_local, optdep->depend->name) == NULL) { + optstrings = alpm_list_add(optstrings, alpm_optdep_compute_string(optdep)); + } + } + + return optstrings; +} + static void display_repo_list(const char *dbname, alpm_list_t *list) { const char *prefix= " "; diff --git a/src/pacman/util.h b/src/pacman/util.h index ee3dbd1..ed7c01f 100644 --- a/src/pacman/util.h +++ b/src/pacman/util.h @@ -62,6 +62,7 @@ void display_targets(const alpm_list_t *pkgs, int install); int str_cmp(const void *s1, const void *s2); void display_new_optdepends(alpm_pkg_t *oldpkg, alpm_pkg_t *newpkg); void display_optdepends(alpm_pkg_t *pkg); +alpm_list_t *optdep_string_list(const alpm_list_t *optdeps); This is unnecessary. I'd rather the function be static in util.c and defined before any users as it is used nowhere else.
void print_packages(const alpm_list_t *packages); void select_display(const alpm_list_t *pkglist); int select_question(int count); -- 1.7.6
--- src/pacman/package.c | 5 +---- src/pacman/util.c | 15 ++++++++++++--- src/pacman/util.h | 2 +- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/pacman/package.c b/src/pacman/package.c index 6e6d379..3e882ef 100644 --- a/src/pacman/package.c +++ b/src/pacman/package.c @@ -88,10 +88,7 @@ void dump_pkg_full(alpm_pkg_t *pkg, enum pkg_from from, int extra) } /* turn optdepends list into a text list */ - for(i = alpm_pkg_get_optdepends(pkg); i; i = alpm_list_next(i)) { - alpm_optdepend_t *optdep = (alpm_optdepend_t *)alpm_list_getdata(i); - optstrings = alpm_list_add(optstrings, alpm_optdep_compute_string(optdep)); - } + optstrings = optdep_string_list(alpm_pkg_get_optdepends(pkg), 1); if(extra || from == PKG_FROM_LOCALDB) { /* compute this here so we don't get a pause in the middle of output */ diff --git a/src/pacman/util.c b/src/pacman/util.c index d3a5648..88b7872 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -996,7 +996,7 @@ void display_new_optdepends(alpm_pkg_t *oldpkg, alpm_pkg_t *newpkg) new = alpm_pkg_get_optdepends(newpkg); optdeps = alpm_list_diff(new,old,opt_cmp); - optstrings = optdep_string_list(optdeps); + optstrings = optdep_string_list(optdeps, 0); if(optstrings) { printf(_("New optional dependencies for %s\n"), alpm_pkg_get_name(newpkg)); @@ -1012,7 +1012,7 @@ void display_optdepends(alpm_pkg_t *pkg) alpm_list_t *optdeps, *optstrings; optdeps = alpm_pkg_get_optdepends(pkg); - optstrings = optdep_string_list(optdeps); + optstrings = optdep_string_list(optdeps, 0); if(optstrings) { printf(_("Optional dependencies for %s\n"), alpm_pkg_get_name(pkg)); @@ -1027,7 +1027,7 @@ void display_optdepends(alpm_pkg_t *pkg) * the returned list has to be freed! */ -alpm_list_t *optdep_string_list(const alpm_list_t *optlist) +alpm_list_t *optdep_string_list(const alpm_list_t *optlist, const int include_installed) { alpm_list_t *optstrings = NULL; alpm_optdepend_t *optdep; @@ -1040,6 +1040,15 @@ alpm_list_t *optdep_string_list(const alpm_list_t *optlist) optdep = (alpm_optdepend_t *)alpm_list_getdata(optlist); if(alpm_db_get_pkg(db_local, optdep->depend->name) == NULL) { optstrings = alpm_list_add(optstrings, alpm_optdep_compute_string(optdep)); + } else if (include_installed) { + char *str, *tmp; + tmp = alpm_optdep_compute_string(optdep); + if ((str = realloc(tmp, strlen(tmp) + 13)) != NULL) { + strcpy(str + strlen(str), " [installed]"); + optstrings = alpm_list_add(optstrings, str); + } else { + free(tmp); + } } } diff --git a/src/pacman/util.h b/src/pacman/util.h index ed7c01f..f2fca85 100644 --- a/src/pacman/util.h +++ b/src/pacman/util.h @@ -62,7 +62,7 @@ void display_targets(const alpm_list_t *pkgs, int install); int str_cmp(const void *s1, const void *s2); void display_new_optdepends(alpm_pkg_t *oldpkg, alpm_pkg_t *newpkg); void display_optdepends(alpm_pkg_t *pkg); -alpm_list_t *optdep_string_list(const alpm_list_t *optdeps); +alpm_list_t *optdep_string_list(const alpm_list_t *optdeps, const int include_installed); void print_packages(const alpm_list_t *packages); void select_display(const alpm_list_t *pkglist); int select_question(int count); -- 1.7.6
On Thu, Jul 21, 2011 at 1:39 PM, Benedikt Morbach <benedikt.morbach@googlemail.com> wrote:
--- src/pacman/package.c | 5 +---- src/pacman/util.c | 15 ++++++++++++--- src/pacman/util.h | 2 +- 3 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/src/pacman/package.c b/src/pacman/package.c index 6e6d379..3e882ef 100644 --- a/src/pacman/package.c +++ b/src/pacman/package.c @@ -88,10 +88,7 @@ void dump_pkg_full(alpm_pkg_t *pkg, enum pkg_from from, int extra) }
/* turn optdepends list into a text list */ - for(i = alpm_pkg_get_optdepends(pkg); i; i = alpm_list_next(i)) { - alpm_optdepend_t *optdep = (alpm_optdepend_t *)alpm_list_getdata(i); - optstrings = alpm_list_add(optstrings, alpm_optdep_compute_string(optdep)); - } + optstrings = optdep_string_list(alpm_pkg_get_optdepends(pkg), 1); Why wasn't this in the previous patch?
if(extra || from == PKG_FROM_LOCALDB) { /* compute this here so we don't get a pause in the middle of output */ diff --git a/src/pacman/util.c b/src/pacman/util.c index d3a5648..88b7872 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -996,7 +996,7 @@ void display_new_optdepends(alpm_pkg_t *oldpkg, alpm_pkg_t *newpkg) new = alpm_pkg_get_optdepends(newpkg); optdeps = alpm_list_diff(new,old,opt_cmp);
- optstrings = optdep_string_list(optdeps); + optstrings = optdep_string_list(optdeps, 0);
if(optstrings) { printf(_("New optional dependencies for %s\n"), alpm_pkg_get_name(newpkg)); @@ -1012,7 +1012,7 @@ void display_optdepends(alpm_pkg_t *pkg) alpm_list_t *optdeps, *optstrings;
optdeps = alpm_pkg_get_optdepends(pkg); - optstrings = optdep_string_list(optdeps); + optstrings = optdep_string_list(optdeps, 0);
if(optstrings) { printf(_("Optional dependencies for %s\n"), alpm_pkg_get_name(pkg)); @@ -1027,7 +1027,7 @@ void display_optdepends(alpm_pkg_t *pkg) * the returned list has to be freed! */
-alpm_list_t *optdep_string_list(const alpm_list_t *optlist) +alpm_list_t *optdep_string_list(const alpm_list_t *optlist, const int include_installed)
I'd probably prefer just "int" instead of "const int" since we don't tend to use that modifier on anything passed by value.
{ alpm_list_t *optstrings = NULL; alpm_optdepend_t *optdep; @@ -1040,6 +1040,15 @@ alpm_list_t *optdep_string_list(const alpm_list_t *optlist) optdep = (alpm_optdepend_t *)alpm_list_getdata(optlist); if(alpm_db_get_pkg(db_local, optdep->depend->name) == NULL) { optstrings = alpm_list_add(optstrings, alpm_optdep_compute_string(optdep)); + } else if (include_installed) { + char *str, *tmp; + tmp = alpm_optdep_compute_string(optdep); + if ((str = realloc(tmp, strlen(tmp) + 13)) != NULL) { Probably better to do const char * const installed = " [installed]"; and then you can use strlen(installed_ rather than the magic number 13 as the compiler will be smart about it.
+ strcpy(str + strlen(str), " [installed]"); + optstrings = alpm_list_add(optstrings, str); + } else { We should probably make a bit more noise here if this fails. + free(tmp); + } } }
diff --git a/src/pacman/util.h b/src/pacman/util.h index ed7c01f..f2fca85 100644 --- a/src/pacman/util.h +++ b/src/pacman/util.h @@ -62,7 +62,7 @@ void display_targets(const alpm_list_t *pkgs, int install); int str_cmp(const void *s1, const void *s2); void display_new_optdepends(alpm_pkg_t *oldpkg, alpm_pkg_t *newpkg); void display_optdepends(alpm_pkg_t *pkg); -alpm_list_t *optdep_string_list(const alpm_list_t *optdeps); +alpm_list_t *optdep_string_list(const alpm_list_t *optdeps, const int include_installed); void print_packages(const alpm_list_t *packages); void select_display(const alpm_list_t *pkglist); int select_question(int count); -- 1.7.6
--- doc/pacman.8.txt | 6 ++++++ lib/libalpm/alpm.h | 2 +- lib/libalpm/package.c | 32 ++++++++++++++++++++++---------- 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 +- 8 files changed, 48 insertions(+), 16 deletions(-) diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt index 64b1ff3..90fb040 100644 --- a/doc/pacman.8.txt +++ b/doc/pacman.8.txt @@ -269,6 +269,12 @@ Query Options[[QO]] database(s). Typically these are packages that were downloaded manually and installed with '\--upgrade'. +*-n, \--nooptdeps*:: + 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 21f65a1..adba014 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -535,7 +535,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); /** @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 a64d613..d473973 100644 --- a/lib/libalpm/package.c +++ b/lib/libalpm/package.c @@ -376,7 +376,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) { const alpm_list_t *i; pkg->handle->pm_errno = 0; @@ -384,11 +385,22 @@ 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) { + if(_alpm_depcmp(pkg, ((alpm_optdepend_t *)j->data)->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)); + } } } } @@ -396,7 +408,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; @@ -407,17 +419,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->is_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 bce42ab..2330e9a 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_nooptdeps; 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 3e882ef..250f701 100644 --- a/src/pacman/package.c +++ b/src/pacman/package.c @@ -92,7 +92,7 @@ void dump_pkg_full(alpm_pkg_t *pkg, enum pkg_from from, 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 e855203..54ddbc0 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -146,6 +146,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, --nooptdeps 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")); @@ -458,6 +459,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_nooptdeps = 1; break; case 'g': (config->group)++; break; case 'i': (config->op_q_info)++; break; case 'k': config->op_q_check = 1; break; @@ -598,6 +600,7 @@ static int parseargs(int argc, char *argv[]) {"list", no_argument, 0, 'l'}, {"foreign", no_argument, 0, 'm'}, {"nosave", no_argument, 0, 'n'}, + {"nooptdeps", no_argument, 0, 'n'}, {"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 251c4dd..6b6f521 100644 --- a/src/pacman/query.c +++ b/src/pacman/query.c @@ -360,9 +360,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; } @@ -387,7 +393,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_nooptdeps && !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 9b67863..c7f2b20 100644 --- a/src/util/pactree.c +++ b/src/util/pactree.c @@ -342,7 +342,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 = alpm_list_getdata(i); -- 1.7.6
--- src/pacman/package.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/pacman/package.c b/src/pacman/package.c index 250f701..8bb67dc 100644 --- a/src/pacman/package.c +++ b/src/pacman/package.c @@ -53,7 +53,8 @@ void dump_pkg_full(alpm_pkg_t *pkg, enum pkg_from from, int extra) const char *label; double size; const alpm_list_t *i; - alpm_list_t *depstrings = NULL, *optstrings = NULL, *requiredby = NULL; + alpm_list_t *depstrings, *optstrings, *requiredby, *optrequiredby; + depstrings = optstrings = requiredby = optrequiredby = NULL; if(pkg == NULL) { return; @@ -92,7 +93,8 @@ void dump_pkg_full(alpm_pkg_t *pkg, enum pkg_from from, 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, 0); + requiredby = alpm_pkg_compute_requiredby(pkg, 0); + optrequiredby = alpm_pkg_compute_requiredby(pkg, 1); } /* actual output */ @@ -110,6 +112,7 @@ void dump_pkg_full(alpm_pkg_t *pkg, enum pkg_from from, int extra) list_display_linebreak(_("Optional Deps :"), optstrings); if(extra || from == PKG_FROM_LOCALDB) { list_display(_("Required By :"), requiredby); + list_display(_("Optional For :"), optrequiredby); } list_display(_("Conflicts With :"), alpm_pkg_get_conflicts(pkg)); list_display(_("Replaces :"), alpm_pkg_get_replaces(pkg)); -- 1.7.6
On Thu, Jul 21, 2011 at 1:39 PM, Benedikt Morbach <benedikt.morbach@googlemail.com> wrote:
Hi,
I've send this before, but it seems like it didn't reach the list. I apologize if I'm flooding your inbox and you got this patch series twice.
The pacman codebase is completely new to me, so please forgive me the obvious mistakes that I overlooked and feel free to point them out and yell at me.
This implements most features from https://wiki.archlinux.org/index.php/User:Allan/Pacman_OptDepends
It probably isn't quite ready to be merged yet, but I want to gather some feedback on the stuff I've already done. I tried to comment on most of the patches (1-4). The overall work looks really good, just a few minor comments in there.
When you resubmit, can you be sure to include signoffs on each patch? man git-commit, -s option. -Dan
participants (2)
-
Benedikt Morbach
-
Dan McGee