[PATCH 0/1] Support multiple profiles with '--list-installed'.

DoneSubmitted by zimoun.
Details
2 participants
  • Ludovic Courtès
  • zimoun
Owner
unassigned
Severity
normal
Z
Z
zimoun wrote on 14 May 2020 16:13
(address . guix-patches@gnu.org)(name . zimoun)(address . zimon.toutoune@gmail.com)
20200514141350.4867-1-zimon.toutoune@gmail.com
Dear,

As reported [1],

guix package -p profile1 -p profile2 -I
and
guix package -p profile2 -p profile2 -I

return different list; the one of the first profile actually.

This patch fixes this unexpected behaviour.


Last, I have failed to add a test in 'tests/guix-package.sh'. I am not sure
to understand how the store is handled when running the test suite.


All the best,
simon



zimoun (1):
guix package: Support multiple profiles with '--list-installed'.

guix/scripts/package.scm | 31 +++++++++++++++++--------------
1 file changed, 17 insertions(+), 14 deletions(-)

--
2.26.1
Z
Z
zimoun wrote on 14 May 2020 16:17
[PATCH 1/1] guix package: Support multiple profiles with '--list-installed'.
(address . 41260@debbugs.gnu.org)(name . zimoun)(address . zimon.toutoune@gmail.com)
20200514141759.6455-1-zimon.toutoune@gmail.com
* guix/scripts/package.scm (process-query): List installed multiple profiles.
---
guix/scripts/package.scm | 31 +++++++++++++++++--------------
1 file changed, 17 insertions(+), 14 deletions(-)

Toggle diff (44 lines)
diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index a69efa365e..a4a6100a33 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -717,20 +717,23 @@ processed, #f otherwise."
        #t)
 
       (('list-installed regexp)
-       (let* ((regexp    (and regexp (make-regexp* regexp regexp/icase)))
-              (manifest  (profile-manifest profile))
-              (installed (manifest-entries manifest)))
-         (leave-on-EPIPE
-          (for-each (match-lambda
-                      (($ <manifest-entry> name version output path _)
-                       (when (or (not regexp)
-                                 (regexp-exec regexp name))
-                         (format #t "~a\t~a\t~a\t~a~%"
-                                 name (or version "?") output path))))
-
-                    ;; Show most recently installed packages last.
-                    (reverse installed)))
-         #t))
+       (for-each
+        (lambda (profile)
+          (let* ((regexp    (and regexp (make-regexp* regexp regexp/icase)))
+                 (manifest  (profile-manifest profile))
+                 (installed (manifest-entries manifest)))
+            (leave-on-EPIPE
+             (for-each (match-lambda
+                         (($ <manifest-entry> name version output path _)
+                          (when (or (not regexp)
+                                    (regexp-exec regexp name))
+                            (format #t "~a\t~a\t~a\t~a~%"
+                                    name (or version "?") output path))))
+
+                       ;; Show most recently installed packages last.
+                       (reverse installed)))))
+        profiles)
+       #t)
 
       (('list-available regexp)
        (let* ((regexp    (and regexp (make-regexp* regexp regexp/icase)))
-- 
2.26.1
L
L
Ludovic Courtès wrote on 16 May 2020 19:33
(name . zimoun)(address . zimon.toutoune@gmail.com)(address . 41260@debbugs.gnu.org)
87v9kvpywp.fsf@gnu.org
Hi!

zimoun <zimon.toutoune@gmail.com> skribis:

Toggle quote (2 lines)
> * guix/scripts/package.scm (process-query): List installed multiple profiles.

[...]

Toggle quote (17 lines)
> + (for-each
> + (lambda (profile)
> + (let* ((regexp (and regexp (make-regexp* regexp regexp/icase)))
> + (manifest (profile-manifest profile))
> + (installed (manifest-entries manifest)))
> + (leave-on-EPIPE
> + (for-each (match-lambda
> + (($ <manifest-entry> name version output path _)
> + (when (or (not regexp)
> + (regexp-exec regexp name))
> + (format #t "~a\t~a\t~a\t~a~%"
> + name (or version "?") output path))))
> +
> + ;; Show most recently installed packages last.
> + (reverse installed)))))
> + profiles)

How about instead loading all the manifests, merging them with
‘concatenate-manifests’, and operating on that? That would avoid
special-casing.

Bonus point if you can add a test case for that, similar to
‘--search-paths’ with multiple profiles. :-)

Thanks,
Ludo’.
Z
Z
zimoun wrote on 21 May 2020 23:43
[PATCH v2] guix package: Support multiple profiles with '--list-installed'.
(address . 41260@debbugs.gnu.org)(name . zimoun)(address . zimon.toutoune@gmail.com)
20200521214306.6948-1-zimon.toutoune@gmail.com
* guix/scripts/package.scm (process-query): List installed multiple profiles.
* tests/guix-package-net.sh: Test it.
---
guix/scripts/package.scm | 20 +++++++++++---------
tests/guix-package-net.sh | 12 ++++++++++++
2 files changed, 23 insertions(+), 9 deletions(-)

Toggle diff (77 lines)
diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index a69efa365e..1246147798 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -675,12 +675,13 @@ doesn't need it."
 (define (process-query opts)
   "Process any query specified by OPTS.  Return #t when a query was actually
 processed, #f otherwise."
-  (let* ((profiles (match (filter-map (match-lambda
-                                        (('profile . p) p)
-                                        (_              #f))
-                                      opts)
-                     (() (list %current-profile))
-                     (lst (reverse lst))))
+  (let* ((profiles (delete-duplicates
+                    (match (filter-map (match-lambda
+                                         (('profile . p) p)
+                                         (_              #f))
+                                       opts)
+                      (() (list %current-profile))
+                      (lst (reverse lst)))))
          (profile  (match profiles
                      ((head tail ...) head))))
     (match (assoc-ref opts 'query)
@@ -718,7 +719,8 @@ processed, #f otherwise."
 
       (('list-installed regexp)
        (let* ((regexp    (and regexp (make-regexp* regexp regexp/icase)))
-              (manifest  (profile-manifest profile))
+              (manifest  (concatenate-manifests
+                          (map profile-manifest profiles)))
               (installed (manifest-entries manifest)))
          (leave-on-EPIPE
           (for-each (match-lambda
@@ -729,8 +731,8 @@ processed, #f otherwise."
                                  name (or version "?") output path))))
 
                     ;; Show most recently installed packages last.
-                    (reverse installed)))
-         #t))
+                    (reverse installed))))
+       #t)
 
       (('list-available regexp)
        (let* ((regexp    (and regexp (make-regexp* regexp regexp/icase)))
diff --git a/tests/guix-package-net.sh b/tests/guix-package-net.sh
index 48a94865e1..3876701fa2 100644
--- a/tests/guix-package-net.sh
+++ b/tests/guix-package-net.sh
@@ -1,6 +1,7 @@
 # GNU Guix --- Functional package management for GNU
 # Copyright © 2012, 2013, 2014, 2015, 2017, 2019 Ludovic Courtès <ludo@gnu.org>
 # Copyright © 2013 Nikita Karetnikov <nikita@karetnikov.org>
+# Copyright © 2020 Simon Tournier <zimon.toutoune@gmail.com>
 #
 # This file is part of GNU Guix.
 #
@@ -78,6 +79,17 @@ esac
 
 test "`guix package -p "$profile" -I 'g.*e' | cut -f1`" = "guile-bootstrap"
 
+guix package --bootstrap -p "$profile_alt" -i gcc-bootstrap
+installed="`guix package -p "$profile" -p "$profile_alt" -I | cut -f1 | xargs echo | sort`"
+case "x$installed" in
+    "gcc-bootstrap guile-bootstrap make-boot0")
+        true;;
+    "*")
+        false;;
+esac
+test "`guix package -p "$profile_alt" -p "$profile" -I | wc -l`" = "3"
+rm "$profile_alt"
+
 # List generations.
 test "`guix package -p "$profile" -l | cut -f1 | grep guile | head -n1`" \
      = "  guile-bootstrap"
-- 
2.26.2
Z
Z
zimoun wrote on 21 May 2020 23:43
Re: [bug#41260] [PATCH 1/1] guix package: Support multiple profiles with '--list-installed'.
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 41260@debbugs.gnu.org)
CAJ3okZ3pN8oh_11-OFJjnk2L2MKmVJ472d=bX1vYaSubYetddw@mail.gmail.com
Hi Ludo,

Sorry for the delay.

On Sat, 16 May 2020 at 19:34, Ludovic Courtès <ludo@gnu.org> wrote:

Toggle quote (4 lines)
> How about instead loading all the manifests, merging them with
> ‘concatenate-manifests’, and operating on that? That would avoid
> special-casing.

See v2.


Toggle quote (3 lines)
> Bonus point if you can add a test case for that, similar to
> ‘--search-paths’ with multiple profiles. :-)

Done in 'tests/guix-packages-net.sh'. See v2.


BTW, I added a 'delete-duplicates' on profiles. And I was tempted to
also add 'sort' to always process in the same order. And personally,
I prefer the alphanumerical order than the order of installation
because it is more reliable and easier to find what I am looking for
-- otherwise I always do the extra step of pipe: |cut -f1| xargs echo|
sort which is "ugly". The order of installation seems redundant with
--list-generation. WDYT?


Cheers,
simon
L
L
Ludovic Courtès wrote on 22 May 2020 22:31
(name . zimoun)(address . zimon.toutoune@gmail.com)(address . 41260@debbugs.gnu.org)
87pnav90f9.fsf@gnu.org
Hi,

zimoun <zimon.toutoune@gmail.com> skribis:

Toggle quote (8 lines)
> BTW, I added a 'delete-duplicates' on profiles. And I was tempted to
> also add 'sort' to always process in the same order. And personally,
> I prefer the alphanumerical order than the order of installation
> because it is more reliable and easier to find what I am looking for
> -- otherwise I always do the extra step of pipe: |cut -f1| xargs echo|
> sort which is "ugly". The order of installation seems redundant with
> --list-generation. WDYT?

I’m not sure about ‘sort’ because the order (unfortunately) does make a
difference when there are file-level collisions. If we were starting
today, we could forcefully sort things, but I’m tempted to think that
it’d break someone’s workflow if we did it now¹.

Ludo’.

¹ Obligatory reference: https://xkcd.com/1172/.
L
L
Ludovic Courtès wrote on 23 May 2020 15:54
Re: [bug#41260] [PATCH v2] guix package: Support multiple profiles with '--list-installed'.
(name . zimoun)(address . zimon.toutoune@gmail.com)(address . 41260-done@debbugs.gnu.org)
87pnau7o5e.fsf@gnu.org
Hi,

zimoun <zimon.toutoune@gmail.com> skribis:

Toggle quote (3 lines)
> * guix/scripts/package.scm (process-query): List installed multiple profiles.
> * tests/guix-package-net.sh: Test it.

Applied, thanks!

Ludo’.
Closed
Z
Z
zimoun wrote on 25 May 2020 11:32
Re: [bug#41260] [PATCH 1/1] guix package: Support multiple profiles with '--list-installed'.
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 41260@debbugs.gnu.org)
CAJ3okZ21RmLh9S0FFq2ns3-XanXHzgrCCab6sSp_JA=FAc-c7Q@mail.gmail.com
Hi Ludo,

On Fri, 22 May 2020 at 22:31, Ludovic Courtès <ludo@gnu.org> wrote:

Toggle quote (2 lines)
> ¹ Obligatory reference: https://xkcd.com/1172/.

Yeah! Exactly! :-)
But I feel one "formatter" to "pretty" display is lacking. It should
be an elegant solution for the recent discussion, a bit as "git log
--pretty=oneline|full|etc." does.
For example, I remember an old (rejected) patch by Robert Vollmer [1].


Anyway, another topic. :-)


Thank you for the review.
All the best,
simon
?
Your comment

This issue is archived.

To comment on this conversation send email to 41260@debbugs.gnu.org