diff mbox series

[bug#56428] home: Add -I, --list-installed option.

Message ID 20220707150644.2349-1-antero@mailbox.org
State Accepted
Headers show
Series [bug#56428] home: Add -I, --list-installed option. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch success View Laminar job
cbaines/issue success View issue

Commit Message

Antero Mejr July 7, 2022, 3:06 p.m. UTC
* guix/scripts/package.scm (list-installed): New procedure.
* guix/scripts/home.scm: Use it.
---
Remove extra (reverse...) from last patch.

 guix/scripts/home.scm    | 12 ++++++++++++
 guix/scripts/package.scm | 30 +++++++++++++++++-------------
 2 files changed, 29 insertions(+), 13 deletions(-)

Comments

Andrew Tropin July 12, 2022, 10:13 a.m. UTC | #1
On 2022-07-07 15:06, Antero Mejr via Guix-patches via wrote:

Hi Antero,

this is a good addition, thank you very much!

A hint: use reroll-count when generating new revision of the patch and
subject will become [PATCH v2], [PATCH v3] etc.

> * guix/scripts/package.scm (list-installed): New procedure.
> * guix/scripts/home.scm: Use it.
> ---
> Remove extra (reverse...) from last patch.
>
>  guix/scripts/home.scm    | 12 ++++++++++++
>  guix/scripts/package.scm | 30 +++++++++++++++++-------------
>  2 files changed, 29 insertions(+), 13 deletions(-)
>
> diff --git a/guix/scripts/home.scm b/guix/scripts/home.scm
> index 0f5c3388a1..b0b8412d8c 100644
> --- a/guix/scripts/home.scm
> +++ b/guix/scripts/home.scm
> @@ -4,6 +4,7 @@
>  ;;; Copyright © 2021 Pierre Langlois <pierre.langlois@gmx.com>
>  ;;; Copyright © 2021 Oleg Pykhalov <go.wigust@gmail.com>
>  ;;; Copyright © 2022 Ludovic Courtès <ludo@gnu.org>
> +;;; Copyright © 2022 Antero Mejr <antero@mailbox.org>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -143,6 +144,10 @@ (define (show-help)
>                           use BACKEND for 'extension-graph' and 'shepherd-graph'"))
>    (newline)
>    (display (G_ "
> +  -I, --list-installed[=REGEXP]
> +                         list installed packages matching REGEXP"))

1. guix home/system and guix package has slightly different cli:
guix package --list-generations vs guix home/system list-generations

I think that later is more apropriate, because:

`guix package --remove=htop --list-installed` is possible, but doesn't
make too much sense, and `guix home reconfigure ./he.scm list-installed`
is not possible, because only one action at time can be specicified.

Implementing this functionality as --argument makes it possible to type
`guix home reconfigure ./he.scm --list-installed`, which again doesn't
make much sense as in the example above.  I advice either implement
list-installed as a separate action or as an additional --argument to
describe/list-generations action.

2. Would be good to mention it in doc/guix.texi.

3. It would be nice to implement the same for guix system.

> +  (newline)
> +  (display (G_ "
>    -h, --help             display this help and exit"))
>    (display (G_ "
>    -V, --version          display version information and exit"))
> @@ -183,6 +188,13 @@ (define %options
>           (option '("graph-backend") #t #f
>                   (lambda (opt name arg result)
>                     (alist-cons 'graph-backend arg result)))
> +         (option '(#\I "list-installed") #f #t
> +                 (lambda (opt name arg result)
> +                   (pretty-print-table
> +                    (list-installed (or arg "")
> +                                    (list
> +                                     (string-append %guix-home "/profile"))))
> +                   (exit 0)))
>  
>           ;; Container options.  (option '(#\N "network") #f #f diff
> --git a/guix/scripts/package.scm b/guix/scripts/package.scm index
> 99a6cfaa29..02e91a0ee1 100644 --- a/guix/scripts/package.scm +++
> b/guix/scripts/package.scm @@ -11,6 +11,7 @@ ;;; Copyright © 2020
> Simon Tournier <zimon.toutoune@gmail.com> ;;; Copyright © 2018 Steve
> Sprang <scs@stevesprang.com> ;;; Copyright © 2022 Josselin Poiret
> <dev@jpoiret.xyz> +;;; Copyright © 2022 Antero Mejr
> <antero@mailbox.org> ;;; ;;; This file is part of GNU Guix.  ;;; @@
> -773,6 +774,20 @@ (define absolute
>  
>    (add-indirect-root store absolute))
>  
> +(define-public (list-installed regexp profiles)

To make it consistent with the rest of the module, I think it will
better to use define and explicitly export list-installed in module
definition at the top of the file.

> +  (let* ((regexp    (and regexp (make-regexp* regexp regexp/icase)))
> +         (manifest  (concatenate-manifests
> +                     (map profile-manifest profiles)))
> +         (installed (manifest-entries manifest)))
> +    (leave-on-EPIPE
> +     (let ((rows (filter-map
> +                  (match-lambda
> +                    (($ <manifest-entry> name version output path _)
> +                     (and (regexp-exec regexp name)
> +                          (list name (or version "?") output path))))
> +                  installed)))
> +       rows))))
> +
>  
>  ;;;
>  ;;; Queries and actions.
> @@ -824,19 +839,8 @@ (define (diff-profiles profile numbers)
>         #t)
>  
>        (('list-installed regexp)
> -       (let* ((regexp    (and regexp (make-regexp* regexp regexp/icase)))
> -              (manifest  (concatenate-manifests
> -                          (map profile-manifest profiles)))
> -              (installed (manifest-entries manifest)))
> -         (leave-on-EPIPE
> -          (let ((rows (filter-map
> -                       (match-lambda
> -                         (($ <manifest-entry> name version output path _)
> -                          (and (regexp-exec regexp name)
> -                               (list name (or version "?") output path))))
> -                       installed)))
> -            ;; Show most recently installed packages last.
> -            (pretty-print-table (reverse rows)))))
> +       ;; Show most recently installed packages last.
> +       (pretty-print-table (reverse (list-installed regexp profiles)))
>         #t)
>  
>        (('list-available regexp)

Tested the patch, guix package -I, guix home -I filter-regex works good,
but see my comments above.
Antero Mejr July 12, 2022, 10:50 p.m. UTC | #2
Andrew Tropin <andrew@trop.in> writes:

> Implementing this functionality as --argument makes it possible to type
> `guix home reconfigure ./he.scm --list-installed`, which again doesn't
> make much sense as in the example above.  I advice either implement
> list-installed as a separate action or as an additional --argument to
> describe/list-generations action.

I added it to both guix home describe and list-generations. Adding it to
describe seems most intuitive to me, and adding it to list-generations
is useful for tracking when a package was added to the profile.

If --list-installed is specified for an invalid subcommand, the flag is
ignored.
Same behavior as when using --network with a guix home describe, for
example.

> 2. Would be good to mention it in doc/guix.texi.

Done.

> 3. It would be nice to implement the same for guix system.

I will do that in another patch/issue.

> To make it consistent with the rest of the module, I think it will
> better to use define and explicitly export list-installed in module
> definition at the top of the file.

Done.

Thanks for the review, and also thanks for guix home!
diff mbox series

Patch

diff --git a/guix/scripts/home.scm b/guix/scripts/home.scm
index 0f5c3388a1..b0b8412d8c 100644
--- a/guix/scripts/home.scm
+++ b/guix/scripts/home.scm
@@ -4,6 +4,7 @@ 
 ;;; Copyright © 2021 Pierre Langlois <pierre.langlois@gmx.com>
 ;;; Copyright © 2021 Oleg Pykhalov <go.wigust@gmail.com>
 ;;; Copyright © 2022 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2022 Antero Mejr <antero@mailbox.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -143,6 +144,10 @@  (define (show-help)
                          use BACKEND for 'extension-graph' and 'shepherd-graph'"))
   (newline)
   (display (G_ "
+  -I, --list-installed[=REGEXP]
+                         list installed packages matching REGEXP"))
+  (newline)
+  (display (G_ "
   -h, --help             display this help and exit"))
   (display (G_ "
   -V, --version          display version information and exit"))
@@ -183,6 +188,13 @@  (define %options
          (option '("graph-backend") #t #f
                  (lambda (opt name arg result)
                    (alist-cons 'graph-backend arg result)))
+         (option '(#\I "list-installed") #f #t
+                 (lambda (opt name arg result)
+                   (pretty-print-table
+                    (list-installed (or arg "")
+                                    (list
+                                     (string-append %guix-home "/profile"))))
+                   (exit 0)))
 
          ;; Container options.
          (option '(#\N "network") #f #f
diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index 99a6cfaa29..02e91a0ee1 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -11,6 +11,7 @@ 
 ;;; Copyright © 2020 Simon Tournier <zimon.toutoune@gmail.com>
 ;;; Copyright © 2018 Steve Sprang <scs@stevesprang.com>
 ;;; Copyright © 2022 Josselin Poiret <dev@jpoiret.xyz>
+;;; Copyright © 2022 Antero Mejr <antero@mailbox.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -773,6 +774,20 @@  (define absolute
 
   (add-indirect-root store absolute))
 
+(define-public (list-installed regexp profiles)
+  (let* ((regexp    (and regexp (make-regexp* regexp regexp/icase)))
+         (manifest  (concatenate-manifests
+                     (map profile-manifest profiles)))
+         (installed (manifest-entries manifest)))
+    (leave-on-EPIPE
+     (let ((rows (filter-map
+                  (match-lambda
+                    (($ <manifest-entry> name version output path _)
+                     (and (regexp-exec regexp name)
+                          (list name (or version "?") output path))))
+                  installed)))
+       rows))))
+
 
 ;;;
 ;;; Queries and actions.
@@ -824,19 +839,8 @@  (define (diff-profiles profile numbers)
        #t)
 
       (('list-installed regexp)
-       (let* ((regexp    (and regexp (make-regexp* regexp regexp/icase)))
-              (manifest  (concatenate-manifests
-                          (map profile-manifest profiles)))
-              (installed (manifest-entries manifest)))
-         (leave-on-EPIPE
-          (let ((rows (filter-map
-                       (match-lambda
-                         (($ <manifest-entry> name version output path _)
-                          (and (regexp-exec regexp name)
-                               (list name (or version "?") output path))))
-                       installed)))
-            ;; Show most recently installed packages last.
-            (pretty-print-table (reverse rows)))))
+       ;; Show most recently installed packages last.
+       (pretty-print-table (reverse (list-installed regexp profiles)))
        #t)
 
       (('list-available regexp)