[pacman-dev] [PATCH] Group optional dependencies by installation status
Not sure how expensive alpm_db_get_pkg is, so it might be worthwhile to use something other than make_optstring to accomplish this if we want to avoid calling it four times per package. Signed-off-by: Drew DeVault <sir@cmpwn.com> --- src/pacman/util.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/pacman/util.c b/src/pacman/util.c index 23c4009..c1d3dab 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1243,10 +1243,19 @@ void display_optdepends(alpm_pkg_t *pkg) optdeps = alpm_pkg_get_optdepends(pkg); - /* turn optdepends list into a text list */ + /* turn optdepends list into a text list, with uninstalled dependencies last */ for(i = optdeps; i; i = alpm_list_next(i)) { alpm_depend_t *optdep = i->data; - optstrings = alpm_list_add(optstrings, make_optstring(optdep)); + if(alpm_db_get_pkg(alpm_get_localdb(config->handle), optdep->name) != 0) { + optstrings = alpm_list_add(optstrings, make_optstring(optdep)); + } + } + + for(i = optdeps; i; i = alpm_list_next(i)) { + alpm_depend_t *optdep = i->data; + if(alpm_db_get_pkg(alpm_get_localdb(config->handle), optdep->name) == 0) { + optstrings = alpm_list_add(optstrings, make_optstring(optdep)); + } } if(optstrings) { -- 1.8.3.2
On 12/07/13 04:43, Drew DeVault wrote:
Not sure how expensive alpm_db_get_pkg is, so it might be worthwhile to use something other than make_optstring to accomplish this if we want to avoid calling it four times per package.
Optional dependencies are (or should be) listed in a PKGBUILD in order of importance. We previously rejected outputting them in alphabetical order because of this. Given this patch just groups the installed/not installed nature of the optdepend, I suppose this is more reasonable, but I am still reluctant. Also, surely uninstalled optdepends are the more important ones and should be listed first. Allan
Signed-off-by: Drew DeVault <sir@cmpwn.com> --- src/pacman/util.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/src/pacman/util.c b/src/pacman/util.c index 23c4009..c1d3dab 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1243,10 +1243,19 @@ void display_optdepends(alpm_pkg_t *pkg)
optdeps = alpm_pkg_get_optdepends(pkg);
- /* turn optdepends list into a text list */ + /* turn optdepends list into a text list, with uninstalled dependencies last */ for(i = optdeps; i; i = alpm_list_next(i)) { alpm_depend_t *optdep = i->data; - optstrings = alpm_list_add(optstrings, make_optstring(optdep)); + if(alpm_db_get_pkg(alpm_get_localdb(config->handle), optdep->name) != 0) { + optstrings = alpm_list_add(optstrings, make_optstring(optdep)); + } + } + + for(i = optdeps; i; i = alpm_list_next(i)) { + alpm_depend_t *optdep = i->data; + if(alpm_db_get_pkg(alpm_get_localdb(config->handle), optdep->name) == 0) { + optstrings = alpm_list_add(optstrings, make_optstring(optdep)); + } }
if(optstrings) {
Optional dependencies are (or should be) listed in a PKGBUILD in order of importance. We previously rejected outputting them in alphabetical order because of this. Given this patch just groups the installed/not installed nature of the optdepend, I suppose this is more reasonable, but I am still reluctant. I can see the concern, but I think the importance of having a package installed disappears once you already have it. Also, surely uninstalled optdepends are the more important ones and should be listed first. The idea was getting them closer to the end of the output, where your caret will be prepared to type them in and install.
Drew DeVault On 07/11/2013 08:43 PM, Allan McRae wrote:
On 12/07/13 04:43, Drew DeVault wrote:
Not sure how expensive alpm_db_get_pkg is, so it might be worthwhile to use something other than make_optstring to accomplish this if we want to avoid calling it four times per package. Optional dependencies are (or should be) listed in a PKGBUILD in order of importance. We previously rejected outputting them in alphabetical order because of this. Given this patch just groups the installed/not installed nature of the optdepend, I suppose this is more reasonable, but I am still reluctant.
Also, surely uninstalled optdepends are the more important ones and should be listed first.
Allan
Signed-off-by: Drew DeVault <sir@cmpwn.com> --- src/pacman/util.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/src/pacman/util.c b/src/pacman/util.c index 23c4009..c1d3dab 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1243,10 +1243,19 @@ void display_optdepends(alpm_pkg_t *pkg)
optdeps = alpm_pkg_get_optdepends(pkg);
- /* turn optdepends list into a text list */ + /* turn optdepends list into a text list, with uninstalled dependencies last */ for(i = optdeps; i; i = alpm_list_next(i)) { alpm_depend_t *optdep = i->data; - optstrings = alpm_list_add(optstrings, make_optstring(optdep)); + if(alpm_db_get_pkg(alpm_get_localdb(config->handle), optdep->name) != 0) { + optstrings = alpm_list_add(optstrings, make_optstring(optdep)); + } + } + + for(i = optdeps; i; i = alpm_list_next(i)) { + alpm_depend_t *optdep = i->data; + if(alpm_db_get_pkg(alpm_get_localdb(config->handle), optdep->name) == 0) { + optstrings = alpm_list_add(optstrings, make_optstring(optdep)); + } }
if(optstrings) {
On 12/07/13 13:03, Drew DeVault wrote:
Optional dependencies are (or should be) listed in a PKGBUILD in order of importance. We previously rejected outputting them in alphabetical order because of this. Given this patch just groups the installed/not installed nature of the optdepend, I suppose this is more reasonable, but I am still reluctant. I can see the concern, but I think the importance of having a package installed disappears once you already have it. Also, surely uninstalled optdepends are the more important ones and should be listed first. The idea was getting them closer to the end of the output, where your caret will be prepared to type them in and install.
But that puts the most important uninstalled optdep in the middle of the output.
But that puts the most important uninstalled optdep in the middle of the output. I suppose that's true. From my testing, however, it looks fairly distinct. Example:
Optional dependencies for cmus alsa-lib: for ALSA output plugin support [installed] libao: for AO output plugin support [installed] libpulse: for PulseAudio output plugin support [installed] faad2: for AAC input plugin support [installed] flac: for flac input plugin support [installed] libmad: for mp3 input plugin support [installed] libmodplug: for modplug input plugin support [installed] libmp4v2: for mp4 input plugin support [installed] libmpcdec: for musepack input plugin support [installed] libvorbis: for vorbis input plugin support [installed] wavpack: for wavpack input plugin support [installed] ffmpeg: for ffmpeg input plugin support opusfile: for opus input plugin support What if we moved [installed] to the start of the line, or added a line between the two groups of packages? Drew DeVault On 07/11/2013 11:34 PM, Allan McRae wrote:
On 12/07/13 13:03, Drew DeVault wrote:
Optional dependencies are (or should be) listed in a PKGBUILD in order of importance. We previously rejected outputting them in alphabetical order because of this. Given this patch just groups the installed/not installed nature of the optdepend, I suppose this is more reasonable, but I am still reluctant. I can see the concern, but I think the importance of having a package installed disappears once you already have it. Also, surely uninstalled optdepends are the more important ones and should be listed first. The idea was getting them closer to the end of the output, where your caret will be prepared to type them in and install.
But that puts the most important uninstalled optdep in the middle of the output.
On 12/07/13 15:40, Drew DeVault wrote:
But that puts the most important uninstalled optdep in the middle of the output. I suppose that's true. From my testing, however, it looks fairly distinct. Example:
Optional dependencies for cmus alsa-lib: for ALSA output plugin support [installed] libao: for AO output plugin support [installed] libpulse: for PulseAudio output plugin support [installed] faad2: for AAC input plugin support [installed] flac: for flac input plugin support [installed] libmad: for mp3 input plugin support [installed] libmodplug: for modplug input plugin support [installed] libmp4v2: for mp4 input plugin support [installed] libmpcdec: for musepack input plugin support [installed] libvorbis: for vorbis input plugin support [installed] wavpack: for wavpack input plugin support [installed] ffmpeg: for ffmpeg input plugin support opusfile: for opus input plugin support
What if we moved [installed] to the start of the line, or added a line between the two groups of packages?
What if we just did not print installed optional dependencies?
What if we just did not print installed optional dependencies? I'm open to that idea, but hesitant to completely remove them. Maybe we can adjust the output of pacman -Si to include this status in the optional dep list? I'd like additional opinions for either of these changes.
Drew DeVault On 07/11/2013 11:57 PM, Allan McRae wrote:
On 12/07/13 15:40, Drew DeVault wrote:
But that puts the most important uninstalled optdep in the middle of the output. I suppose that's true. From my testing, however, it looks fairly distinct. Example:
Optional dependencies for cmus alsa-lib: for ALSA output plugin support [installed] libao: for AO output plugin support [installed] libpulse: for PulseAudio output plugin support [installed] faad2: for AAC input plugin support [installed] flac: for flac input plugin support [installed] libmad: for mp3 input plugin support [installed] libmodplug: for modplug input plugin support [installed] libmp4v2: for mp4 input plugin support [installed] libmpcdec: for musepack input plugin support [installed] libvorbis: for vorbis input plugin support [installed] wavpack: for wavpack input plugin support [installed] ffmpeg: for ffmpeg input plugin support opusfile: for opus input plugin support
What if we moved [installed] to the start of the line, or added a line between the two groups of packages?
What if we just did not print installed optional dependencies?
On 12/07/13 16:06, Drew DeVault wrote:
What if we just did not print installed optional dependencies? I'm open to that idea, but hesitant to completely remove them. Maybe we can adjust the output of pacman -Si to include this status in the optional dep list? I'd like additional opinions for either of these changes.
You mean -Qi? And like we already do?
You mean -Qi? And like we already do?
Heh, whoops. You're quite right. I'd still like a second opinion on removing optional installed packages from the output, though. On 07/12/2013 01:17 AM, Allan McRae wrote:
On 12/07/13 16:06, Drew DeVault wrote:
What if we just did not print installed optional dependencies? I'm open to that idea, but hesitant to completely remove them. Maybe we can adjust the output of pacman -Si to include this status in the optional dep list? I'd like additional opinions for either of these changes.
You mean -Qi? And like we already do?
On Fri, Jul 12, 2013 at 9:20 AM, Drew DeVault <sircmpwn@gmail.com> wrote:
On 07/12/2013 01:17 AM, Allan McRae wrote:
On 12/07/13 16:06, Drew DeVault wrote:
What if we just did not print installed optional dependencies? I'm open to that idea, but hesitant to completely remove them. Maybe we can adjust the output of pacman -Si to include this status in the optional dep list? I'd like additional opinions for either of these changes.
You mean -Qi? And like we already do?
Heh, whoops. You're quite right. I'd still like a second opinion on removing optional installed packages from the output, though.
This sounds like the best idea to me. I don't see the point in listing installed optional dependencies. pacman doesn't list installed dependencies. Why behave any differently for installed optional dependencies? The only reason it should print them is so that the user knows that there may be more packages they need to install. Jason
participants (3)
-
Allan McRae
-
Drew DeVault
-
Jason St. John