Emacs interface for Guix

Emacs interface for Guix

[Alex Kost]

Hello,

I have questions about installing/deleting packages using guile repl.
I know it is possible to ‘(guix-package "--install" "guile-2.0.11")’,
but is there an easy way to install an output of an exact _package
object_?  I mean not any "guile" but #<package guile-2.0.11 ...>.

I think ‘(guix-package "--install-from-expression" ...)’ is also not
sufficient as it always (?) installs “out”.

If I understand correctly a lot of stuff should be done to perform
such actions properly: at first the new manifest is created from the
current one by adding/removing entries (created from packages) for
installation/deletion, then the derivations are built and symlinks are
updated.

There is a lot of code in “guix/scripts/package.scm” to do all that
stuff and unfortunately not much is exported from this module
(“package->manifest-entry” from “options->installable” would be very
useful for example), and I think trying to include this code in the
helper scheme file for guix.el is not right (besides my scheme foo is
weak and I'm not sure I can do that successfully) that's why I ask
about an easier way.

Thanks, Alex

Re: Emacs interface for Guix

[Ludovic Courtès]

Hi, Alex,

Alex Kost <alezost@gmail.com> skribis:

> I have questions about installing/deleting packages using guile repl.
> I know it is possible to ‘(guix-package "--install" "guile-2.0.11")’,
> but is there an easy way to install an output of an exact _package
> object_?  I mean not any "guile" but #<package guile-2.0.11 ...>.
>
> I think ‘(guix-package "--install-from-expression" ...)’ is also not
> sufficient as it always (?) installs “out”.

This is correct.  (It could be worked around by adding a command-line
option to specify another output, but that just hadn’t seem very useful
so far.)

> There is a lot of code in “guix/scripts/package.scm” to do all that
> stuff and unfortunately not much is exported from this module
> (“package->manifest-entry” from “options->installable” would be very
> useful for example), and I think trying to include this code in the
> helper scheme file for guix.el is not right (besides my scheme foo is
> weak and I'm not sure I can do that successfully) that's why I ask
> about an easier way.

Well, you’ve already understood what needs to be done.  :-)

Namely, (guix scripts package) needs to be made more modular, and the
generic bits must be moved to (guix profiles).  Now that there’s a
second consumer for this API, there’s more of an incentive to do it.

I’m willing to help for that, but I’m happy if you give it a try.
WDYT?

Thanks,
Ludo’.

Re: Emacs interface for Guix

[Alex Kost]

Ludovic Courtès (2014-07-26 00:36 +0400) wrote:

[...]

> Namely, (guix scripts package) needs to be made more modular, and the
> generic bits must be moved to (guix profiles).  Now that there’s a
> second consumer for this API, there’s more of an incentive to do it.
>
> I’m willing to help for that, but I’m happy if you give it a try.
> WDYT?

Sure, I'll do what I can, however the quality of my try may be
unsatisfactory.  Anyway I think I'll send a patch in several days.

--
Alex Kost

Re: Emacs interface for Guix

[Alex Kost]

[-- Attachment #1: Type: text/plain, Size: 1262 bytes --]

Hello, Ludovic and happy vacations :)

Alex Kost (2014-07-26 21:44 +0400) wrote:

> Ludovic Courtès (2014-07-26 00:36 +0400) wrote:
>
> [...]
>
>> Namely, (guix scripts package) needs to be made more modular, and the
>> generic bits must be moved to (guix profiles).  Now that there’s a
>> second consumer for this API, there’s more of an incentive to do it.
>>
>> I’m willing to help for that, but I’m happy if you give it a try.
>> WDYT?
>
> Sure, I'll do what I can, however the quality of my try may be
> unsatisfactory.  Anyway I think I'll send a patch in several days.

You are probably reading this a couple of weeks from now so my changes
may not be actual, but anyway I'm attaching a patch with the changes.

Here is what I've done:

- A part of code for installing/upgrading/removing was extracted from
  ‘guix-package’ function (from ‘process-actions’ more precisely).  So
  the new function (I named it ‘process-package-actions’) can be used in
  "guix.el".

- A bit of code was placed into "profiles.scm" as ‘manifest-add’.

- Also I think you forgot (?) to remove ‘deduplicate’ function in commit
  4ca0b41, so I did it as well.

I hope something of the above is acceptable.  WDYT?


[-- Attachment #2: guix.diff --]
[-- Type: text/x-diff, Size: 20226 bytes --]

diff --git a/guix/profiles.scm b/guix/profiles.scm
index 5e69e01..8533af5 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -47,6 +47,7 @@
             manifest-pattern?
 
             manifest-remove
+            manifest-add
             manifest-installed?
             manifest-matching-entries
 
@@ -196,6 +197,25 @@ must be a manifest-pattern."
                        (manifest-entries manifest)
                        patterns)))
 
+(define (manifest-add manifest entries)
+  "Add ENTRIES to MANIFEST and return new manifest.
+Remove MANIFEST entries that have the same name and output as ENTRIES."
+  (define (same-entry? entry name output)
+    (match entry
+      (($ <manifest-entry> entry-name _ entry-output _ ...)
+       (and (equal? name entry-name)
+            (equal? output entry-output)))))
+
+  (make-manifest
+   (append entries
+           (fold (lambda (entry result)
+                   (match entry
+                     (($ <manifest-entry> name _ out _ ...)
+                      (filter (negate (cut same-entry? <> name out))
+                              result))))
+                 (manifest-entries manifest)
+                 entries))))
+
 (define (manifest-installed? manifest pattern)
   "Return #t if MANIFEST has an entry matching PATTERN (a manifest-pattern),
 #f otherwise."
diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index 31da773..09c1bf1 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -44,6 +44,7 @@
   #:use-module ((gnu packages bootstrap) #:select (%bootstrap-guile))
   #:use-module (guix gnu-maintenance)
   #:export (specification->package+output
+            process-package-actions
             guix-package))
 
 (define %store
@@ -620,112 +621,46 @@ Install, remove, or upgrade PACKAGES in a single transaction.\n"))
 
          %standard-build-options))
 
-(define (options->installable opts manifest)
-  "Given MANIFEST, the current manifest, and OPTS, the result of 'args-fold',
-return the new list of manifest entries."
-  (define (deduplicate deps)
-    ;; Remove duplicate entries from DEPS, a list of propagated inputs, where
-    ;; each input is a name/path tuple.
-    (define (same? d1 d2)
-      (match d1
-        ((_ p1)
-         (match d2
-           ((_ p2) (eq? p1 p2))
-           (_      #f)))
-        ((_ p1 out1)
-         (match d2
-           ((_ p2 out2)
-            (and (string=? out1 out2)
-                 (eq? p1 p2)))
-           (_ #f)))))
-
-    (delete-duplicates deps same?))
-
-  (define (package->manifest-entry* package output)
-    (check-package-freshness package)
-    ;; When given a package via `-e', install the first of its
-    ;; outputs (XXX).
-    (package->manifest-entry package output))
-
+(define (options->installable options manifest)
+  "Given OPTIONS, return a list of patterns for installing/upgrading.
+Returned list is suitable for 'process-package-actions'."
   (define upgrade-regexps
     (filter-map (match-lambda
                  (('upgrade . regexp)
                   (make-regexp (or regexp "")))
                  (_ #f))
-                opts))
+                options))
 
   (define packages-to-upgrade
     (match upgrade-regexps
       (()
        '())
       ((_ ...)
-       (let ((newest (find-newest-available-packages)))
-         (filter-map (match-lambda
-                      (($ <manifest-entry> name version output path _)
-                       (and (any (cut regexp-exec <> name)
-                                 upgrade-regexps)
-                            (upgradeable? name version path)
-                            (let ((output (or output "out")))
-                              (call-with-values
-                                  (lambda ()
-                                    (specification->package+output name output))
-                                list))))
-                      (_ #f))
-                     (manifest-entries manifest))))))
-
-  (define to-upgrade
-    (map (match-lambda
-          ((package output)
-           (package->manifest-entry* package output)))
-         packages-to-upgrade))
+       (filter-map (match-lambda
+                    (($ <manifest-entry> name version output path _)
+                     (and (any (cut regexp-exec <> name)
+                               upgrade-regexps)
+                          (upgradeable? name version path)
+                          (let ((output (or output "out")))
+                            (call-with-values
+                                (lambda ()
+                                  (specification->package+output name output))
+                              list))))
+                    (_ #f))
+                   (manifest-entries manifest)))))
 
   (define packages-to-install
     (filter-map (match-lambda
-                 (('install . (? package? p))
-                  (list p "out"))
-                 (('install . (? string? spec))
-                  (and (not (store-path? spec))
-                       (let-values (((package output)
-                                     (specification->package+output spec)))
-                         (and package (list package output)))))
+                 (('install . package) package)
                  (_ #f))
-                opts))
-
-  (define to-install
-    (append (map (match-lambda
-                  ((package output)
-                   (package->manifest-entry* package output)))
-                 packages-to-install)
-            (filter-map (match-lambda
-                         (('install . (? package?))
-                          #f)
-                         (('install . (? store-path? path))
-                          (let-values (((name version)
-                                        (package-name->name+version
-                                         (store-path-package-name path))))
-                            (manifest-entry
-                             (name name)
-                             (version version)
-                             (output #f)
-                             (item path))))
-                         (_ #f))
-                        opts)))
-
-  (append to-upgrade to-install))
-
-(define (options->removable options manifest)
-  "Given options, return the list of manifest patterns of packages to be
-removed from MANIFEST."
+                options))
+
+  (append packages-to-upgrade packages-to-install))
+
+(define (options->removable options)
+  "Given OPTIONS, return a list of package specifications for deleting."
   (filter-map (match-lambda
-               (('remove . spec)
-                (call-with-values
-                    (lambda ()
-                      (package-specification->name+version+output spec))
-                  (lambda (name version output)
-                    (manifest-pattern
-                      (name name)
-                      (version version)
-                      (output output)))))
+               (('remove . spec) spec)
                (_ #f))
               options))
 
@@ -744,6 +679,150 @@ removed from MANIFEST."
           file
           (apply throw args)))))
 
+(define (ensure-default-profile)
+  "Ensure the default profile symlink and directory exist and are
+writable."
+  (define (rtfm)
+    (format (current-error-port)
+            (_ "Try \"info '(guix) Invoking guix package'\" for \
+more information.~%"))
+    (exit 1))
+
+  ;; Create ~/.guix-profile if it doesn't exist yet.
+  (when (and %user-profile-directory
+             %current-profile
+             (not (false-if-exception
+                   (lstat %user-profile-directory))))
+    (symlink %current-profile %user-profile-directory))
+
+  (let ((s (stat %profile-directory #f)))
+    ;; Attempt to create /…/profiles/per-user/$USER if needed.
+    (unless (and s (eq? 'directory (stat:type s)))
+      (catch 'system-error
+        (lambda ()
+          (mkdir-p %profile-directory))
+        (lambda args
+          ;; Often, we cannot create %PROFILE-DIRECTORY because its
+          ;; parent directory is root-owned and we're running
+          ;; unprivileged.
+          (format (current-error-port)
+                  (_ "error: while creating directory `~a': ~a~%")
+                  %profile-directory
+                  (strerror (system-error-errno args)))
+          (format (current-error-port)
+                  (_ "Please create the `~a' directory, with you as the owner.~%")
+                  %profile-directory)
+          (rtfm))))
+
+    ;; Bail out if it's not owned by the user.
+    (unless (or (not s) (= (stat:uid s) (getuid)))
+      (format (current-error-port)
+              (_ "error: directory `~a' is not owned by you~%")
+              %profile-directory)
+      (format (current-error-port)
+              (_ "Please change the owner of `~a' to user ~s.~%")
+              %profile-directory (or (getenv "USER")
+                                     (getenv "LOGNAME")
+                                     (getuid)))
+      (rtfm))))
+
+(define* (process-package-actions store profile
+                                  #:key (install '()) (remove '())
+                                  dry-run? (use-substitutes? #t))
+  "Install/remove packages.
+
+INSTALL is a list of package patterns for installation.  Each element of
+the list may be a package, a list (PACKAGE OUTPUT), a string with name
+specification or a store path.
+
+REMOVE is a list of name specifications for removing from PROFILE
+manifest."
+  (define (package->manifest-entry* package output)
+    (check-package-freshness package)
+    ;; When given a package via `-e', install the first of its
+    ;; outputs (XXX).
+    (package->manifest-entry package output))
+
+  (define (entries-to-install install)
+    ;; Return a list of manifest entries for installing.
+    (filter-map (match-lambda
+                 ((? package? package)
+                  (package->manifest-entry* package "out"))
+                 (((? package? package) output)
+                  (package->manifest-entry* package output))
+                 ((? string? spec-or-path)
+                  (if (store-path? spec-or-path)
+                      (let-values (((name version)
+                                    (package-name->name+version
+                                     (store-path-package-name spec-or-path))))
+                        (manifest-entry
+                         (name name)
+                         (version version)
+                         (output #f)
+                         (item spec-or-path)))
+                      (let-values (((package output)
+                                    (specification->package+output spec-or-path)))
+                        (and package (package->manifest-entry* package output)))))
+                 (_ #f))
+                install))
+
+  (define (patterns-to-remove remove)
+    ;; Return a list of manifest patterns for removing.
+    (map (lambda (spec)
+           (call-with-values
+               (lambda ()
+                 (package-specification->name+version+output spec))
+             (lambda (name version output)
+               (manifest-pattern
+                (name name)
+                (version version)
+                (output output)))))
+         remove))
+
+  (let* ((manifest (profile-manifest profile))
+         (install  (entries-to-install install))
+         (remove   (patterns-to-remove remove))
+         (new      (manifest-add (manifest-remove manifest remove)
+                                 install))
+         (entries  (manifest-entries new)))
+
+    (unless (and (null? install) (null? remove))
+      (when (equal? profile %current-profile)
+        (ensure-default-profile))
+
+      (let* ((prof-drv (run-with-store store (profile-derivation new)))
+             (prof     (derivation->output-path prof-drv))
+             (remove   (manifest-matching-entries manifest remove)))
+        (show-what-to-remove/install remove install dry-run?)
+        (show-what-to-build store (list prof-drv)
+                            #:use-substitutes? use-substitutes?
+                            #:dry-run? dry-run?)
+
+        (cond
+         (dry-run? #t)
+         ((and (file-exists? profile)
+               (and=> (readlink* profile) (cut string=? prof <>)))
+          (format (current-error-port) (_ "nothing to be done~%")))
+         (else
+          (let* ((number (generation-number profile))
+
+                 ;; Always use NUMBER + 1 for the new profile,
+                 ;; possibly overwriting a "previous future
+                 ;; generation".
+                 (name   (generation-file-name profile
+                                               (+ 1 number))))
+            (and (build-derivations store (list prof-drv))
+                 (let ((count (length entries)))
+                   (switch-symlinks name prof)
+                   (switch-symlinks profile name)
+                   (maybe-register-gc-root store profile)
+                   (format #t (N_ "~a package in profile~%"
+                                  "~a packages in profile~%"
+                                  count)
+                           count)
+                   (display-search-paths entries
+                                         profile))))))))))
+
 \f
 ;;;
 ;;; Entry point.
@@ -767,66 +846,13 @@ removed from MANIFEST."
     (let ((out (derivation->output-path (%guile-for-build))))
       (not (valid-path? (%store) out))))
 
-  (define (ensure-default-profile)
-    ;; Ensure the default profile symlink and directory exist and are
-    ;; writable.
-
-    (define (rtfm)
-      (format (current-error-port)
-              (_ "Try \"info '(guix) Invoking guix package'\" for \
-more information.~%"))
-      (exit 1))
-
-    ;; Create ~/.guix-profile if it doesn't exist yet.
-    (when (and %user-profile-directory
-               %current-profile
-               (not (false-if-exception
-                     (lstat %user-profile-directory))))
-      (symlink %current-profile %user-profile-directory))
-
-    (let ((s (stat %profile-directory #f)))
-      ;; Attempt to create /…/profiles/per-user/$USER if needed.
-      (unless (and s (eq? 'directory (stat:type s)))
-        (catch 'system-error
-          (lambda ()
-            (mkdir-p %profile-directory))
-          (lambda args
-            ;; Often, we cannot create %PROFILE-DIRECTORY because its
-            ;; parent directory is root-owned and we're running
-            ;; unprivileged.
-            (format (current-error-port)
-                    (_ "error: while creating directory `~a': ~a~%")
-                    %profile-directory
-                    (strerror (system-error-errno args)))
-            (format (current-error-port)
-                    (_ "Please create the `~a' directory, with you as the owner.~%")
-                    %profile-directory)
-            (rtfm))))
-
-      ;; Bail out if it's not owned by the user.
-      (unless (or (not s) (= (stat:uid s) (getuid)))
-        (format (current-error-port)
-                (_ "error: directory `~a' is not owned by you~%")
-                %profile-directory)
-        (format (current-error-port)
-                (_ "Please change the owner of `~a' to user ~s.~%")
-                %profile-directory (or (getenv "USER")
-                                       (getenv "LOGNAME")
-                                       (getuid)))
-        (rtfm))))
-
   (define (process-actions opts)
     ;; Process any install/remove/upgrade action from OPTS.
 
-    (define dry-run? (assoc-ref opts 'dry-run?))
-    (define verbose? (assoc-ref opts 'verbose?))
-    (define profile  (assoc-ref opts 'profile))
-
-    (define (same-package? entry name output)
-      (match entry
-        (($ <manifest-entry> entry-name _ entry-output _ ...)
-         (and (equal? name entry-name)
-              (equal? output entry-output)))))
+    (define substitutes? (assoc-ref opts 'substitutes?))
+    (define dry-run?     (assoc-ref opts 'dry-run?))
+    (define verbose?     (assoc-ref opts 'verbose?))
+    (define profile      (assoc-ref opts 'profile))
 
     (define current-generation-number
       (generation-number profile))
@@ -895,61 +921,12 @@ more information.~%"))
              (_ #f))
             opts))
           (else
-           (let* ((manifest (profile-manifest profile))
-                  (install  (options->installable opts manifest))
-                  (remove   (options->removable opts manifest))
-                  (entries
-                   (append install
-                           (fold (lambda (package result)
-                                   (match package
-                                     (($ <manifest-entry> name _ out _ ...)
-                                      (filter (negate
-                                               (cut same-package? <>
-                                                    name out))
-                                              result))))
-                                 (manifest-entries
-                                  (manifest-remove manifest remove))
-                                 install)))
-                  (new      (make-manifest entries)))
-
-             (when (equal? profile %current-profile)
-               (ensure-default-profile))
-
-             (unless (and (null? install) (null? remove))
-               (let* ((prof-drv (run-with-store (%store)
-                                  (profile-derivation new)))
-                      (prof     (derivation->output-path prof-drv))
-                      (remove   (manifest-matching-entries manifest remove)))
-                 (show-what-to-remove/install remove install dry-run?)
-                 (show-what-to-build (%store) (list prof-drv)
-                                     #:use-substitutes?
-                                     (assoc-ref opts 'substitutes?)
-                                     #:dry-run? dry-run?)
-
-                 (cond
-                  (dry-run? #t)
-                  ((and (file-exists? profile)
-                        (and=> (readlink* profile) (cut string=? prof <>)))
-                   (format (current-error-port) (_ "nothing to be done~%")))
-                  (else
-                   (let* ((number (generation-number profile))
-
-                          ;; Always use NUMBER + 1 for the new profile,
-                          ;; possibly overwriting a "previous future
-                          ;; generation".
-                          (name   (generation-file-name profile
-                                                        (+ 1 number))))
-                     (and (build-derivations (%store) (list prof-drv))
-                          (let ((count (length entries)))
-                            (switch-symlinks name prof)
-                            (switch-symlinks profile name)
-                            (maybe-register-gc-root (%store) profile)
-                            (format #t (N_ "~a package in profile~%"
-                                           "~a packages in profile~%"
-                                           count)
-                                    count)
-                            (display-search-paths entries
-                                                  profile))))))))))))
+           (process-package-actions
+            (%store) profile
+            #:install (options->installable opts (profile-manifest profile))
+            #:remove  (options->removable opts)
+            #:use-substitutes? substitutes?
+            #:dry-run? dry-run?))))
 
   (define (process-query opts)
     ;; Process any query specified by OPTS.  Return #t when a query was

Re: Emacs interface for Guix

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> - A part of code for installing/upgrading/removing was extracted from
>   ‘guix-package’ function (from ‘process-actions’ more precisely).  So
>   the new function (I named it ‘process-package-actions’) can be used in
>   "guix.el".

That looks good, but could you make it a separate patch?

In general, it’s better to send atomic changes, with a commit log, in
the format produced by ‘git format-patch’ (see HACKING.)  That
facilitates review and incremental changes.

> - A bit of code was placed into "profiles.scm" as ‘manifest-add’.

Good idea.  Could you send a single patch for this change?  I’ll even
add a couple of test cases in tests/profiles.scm for the new procedure
if you don’t do it yourself.  :-)

> - Also I think you forgot (?) to remove ‘deduplicate’ function in commit
>   4ca0b41, so I did it as well.

Indeed, I’ve just removed it and a couple others.

Somehow I thought we were compiling with -Wunused-toplevel.

Thanks,
Ludo’.

[PATCH] Emacs interface for Guix

[Alex Kost]

[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]

Hello,

Ludovic Courtès (2014-08-12 00:54 +0400) wrote:

> Alex Kost <alezost@gmail.com> skribis:
>
>> - A part of code for installing/upgrading/removing was extracted from
>>   ‘guix-package’ function (from ‘process-actions’ more precisely).  So
>>   the new function (I named it ‘process-package-actions’) can be used in
>>   "guix.el".
>
> That looks good, but could you make it a separate patch?
>
> In general, it’s better to send atomic changes, with a commit log, in
> the format produced by ‘git format-patch’ (see HACKING.)  That
> facilitates review and incremental changes.

Thanks for pointing.  I've never contributed to a real project, so I
don't know the rules actually :)

>> - A bit of code was placed into "profiles.scm" as ‘manifest-add’.
>
> Good idea.  Could you send a single patch for this change?  I’ll even
> add a couple of test cases in tests/profiles.scm for the new procedure
> if you don’t do it yourself.  :-)

Ok, I'm attaching 2 patches with ‘manifest-add’ and
‘process-package-actions’.  What should be changed/improved there?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-profiles-Add-manifest-add.patch --]
[-- Type: text/x-patch, Size: 2954 bytes --]

From af4b8495969d70d59aa9f3f296628daeaf80b0d2 Mon Sep 17 00:00:00 2001
From: Alex Kost <alezost@gmail.com>
Date: Tue, 12 Aug 2014 12:32:16 +0400
Subject: [PATCH 1/2] profiles: Add 'manifest-add'.

* guix/profiles.scm (manifest-add): New procedure.
* tests/profiles.scm (guile-1.8.8): New variable.
  ("manifest-add"): New test.
---
 guix/profiles.scm  | 20 ++++++++++++++++++++
 tests/profiles.scm | 21 +++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/guix/profiles.scm b/guix/profiles.scm
index 5e69e01..c7aec79 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -47,6 +47,7 @@
             manifest-pattern?
 
             manifest-remove
+            manifest-add
             manifest-installed?
             manifest-matching-entries
 
@@ -196,6 +197,25 @@ must be a manifest-pattern."
                        (manifest-entries manifest)
                        patterns)))
 
+(define (manifest-add manifest entries)
+  "Add a list of manifest ENTRIES to MANIFEST and return new manifest.
+Remove MANIFEST entries that have the same name and output as ENTRIES."
+  (define (same-entry? entry name output)
+    (match entry
+      (($ <manifest-entry> entry-name _ entry-output _ ...)
+       (and (equal? name entry-name)
+            (equal? output entry-output)))))
+
+  (make-manifest
+   (append entries
+           (fold (lambda (entry result)
+                   (match entry
+                     (($ <manifest-entry> name _ out _ ...)
+                      (filter (negate (cut same-entry? <> name out))
+                              result))))
+                 (manifest-entries manifest)
+                 entries))))
+
 (define (manifest-installed? manifest pattern)
   "Return #t if MANIFEST has an entry matching PATTERN (a manifest-pattern),
 #f otherwise."
diff --git a/tests/profiles.scm b/tests/profiles.scm
index d405f64..b2919d7 100644
--- a/tests/profiles.scm
+++ b/tests/profiles.scm
@@ -40,6 +40,13 @@
 
 ;; Example manifest entries.
 
+(define guile-1.8.8
+  (manifest-entry
+    (name "guile")
+    (version "1.8.8")
+    (item "/gnu/store/...")
+    (output "out")))
+
 (define guile-2.0.9
   (manifest-entry
     (name "guile")
@@ -101,6 +108,20 @@
             (null? (manifest-entries m3))
             (null? (manifest-entries m4)))))))
 
+(test-assert "manifest-add"
+  (let* ((m0 (manifest '()))
+         (m1 (manifest-add m0 (list guile-1.8.8)))
+         (m2 (manifest-add m1 (list guile-2.0.9)))
+         (m3 (manifest-add m2 (list guile-2.0.9:debug)))
+         (m4 (manifest-add m3 (list guile-2.0.9:debug))))
+    (and (match (manifest-entries m1)
+           ((($ <manifest-entry> "guile" "1.8.8" "out")) #t)
+           (_ #f))
+         (match (manifest-entries m2)
+           ((($ <manifest-entry> "guile" "2.0.9" "out")) #t)
+           (_ #f))
+         (equal? m3 m4))))
+
 (test-assert "profile-derivation"
   (run-with-store %store
     (mlet* %store-monad
-- 
2.0.3


[-- Attachment #3: 0002-guix-package-Add-process-package-actions.patch --]
[-- Type: text/x-patch, Size: 17619 bytes --]

From 5fd45b3f4216921837f522d56b20c4be0a58fe8e Mon Sep 17 00:00:00 2001
From: Alex Kost <alezost@gmail.com>
Date: Tue, 12 Aug 2014 13:54:23 +0400
Subject: [PATCH 2/2] guix package: Add 'process-package-actions'.

* guix/scripts/package.scm (process-package-actions): New procedure.
  (guix-package): Use it.
  [ensure-default-profile]: Move to top-level.
  [substitutes?]: New variable.
  [same-package?]: Remove.
  (options->installable, options->removable): Change according to
  'process-package-actions'.
---
 guix/scripts/package.scm | 336 +++++++++++++++++++++++------------------------
 1 file changed, 166 insertions(+), 170 deletions(-)

diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index 4eb046e..2719b74 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -44,6 +44,7 @@
   #:use-module ((gnu packages bootstrap) #:select (%bootstrap-guile))
   #:use-module (guix gnu-maintenance)
   #:export (specification->package+output
+            process-package-actions
             guix-package))
 
 (define %store
@@ -619,21 +620,15 @@ Install, remove, or upgrade PACKAGES in a single transaction.\n"))
 
          %standard-build-options))
 
-(define (options->installable opts manifest)
-  "Given MANIFEST, the current manifest, and OPTS, the result of 'args-fold',
-return the new list of manifest entries."
-  (define (package->manifest-entry* package output)
-    (check-package-freshness package)
-    ;; When given a package via `-e', install the first of its
-    ;; outputs (XXX).
-    (package->manifest-entry package output))
-
+(define (options->installable options manifest)
+  "Given OPTIONS, return a list of patterns for installing/upgrading.
+Returned list is suitable for 'process-package-actions'."
   (define upgrade-regexps
     (filter-map (match-lambda
                  (('upgrade . regexp)
                   (make-regexp (or regexp "")))
                  (_ #f))
-                opts))
+                options))
 
   (define packages-to-upgrade
     (match upgrade-regexps
@@ -653,59 +648,18 @@ return the new list of manifest entries."
                     (_ #f))
                    (manifest-entries manifest)))))
 
-  (define to-upgrade
-    (map (match-lambda
-          ((package output)
-           (package->manifest-entry* package output)))
-         packages-to-upgrade))
-
   (define packages-to-install
     (filter-map (match-lambda
-                 (('install . (? package? p))
-                  (list p "out"))
-                 (('install . (? string? spec))
-                  (and (not (store-path? spec))
-                       (let-values (((package output)
-                                     (specification->package+output spec)))
-                         (and package (list package output)))))
+                 (('install . package) package)
                  (_ #f))
-                opts))
-
-  (define to-install
-    (append (map (match-lambda
-                  ((package output)
-                   (package->manifest-entry* package output)))
-                 packages-to-install)
-            (filter-map (match-lambda
-                         (('install . (? package?))
-                          #f)
-                         (('install . (? store-path? path))
-                          (let-values (((name version)
-                                        (package-name->name+version
-                                         (store-path-package-name path))))
-                            (manifest-entry
-                             (name name)
-                             (version version)
-                             (output #f)
-                             (item path))))
-                         (_ #f))
-                        opts)))
-
-  (append to-upgrade to-install))
-
-(define (options->removable options manifest)
-  "Given options, return the list of manifest patterns of packages to be
-removed from MANIFEST."
+                options))
+
+  (append packages-to-upgrade packages-to-install))
+
+(define (options->removable options)
+  "Given OPTIONS, return a list of package specifications for deleting."
   (filter-map (match-lambda
-               (('remove . spec)
-                (call-with-values
-                    (lambda ()
-                      (package-specification->name+version+output spec))
-                  (lambda (name version output)
-                    (manifest-pattern
-                      (name name)
-                      (version version)
-                      (output output)))))
+               (('remove . spec) spec)
                (_ #f))
               options))
 
@@ -724,6 +678,150 @@ removed from MANIFEST."
           file
           (apply throw args)))))
 
+(define (ensure-default-profile)
+  "Ensure the default profile symlink and directory exist and are
+writable."
+  (define (rtfm)
+    (format (current-error-port)
+            (_ "Try \"info '(guix) Invoking guix package'\" for \
+more information.~%"))
+    (exit 1))
+
+  ;; Create ~/.guix-profile if it doesn't exist yet.
+  (when (and %user-profile-directory
+             %current-profile
+             (not (false-if-exception
+                   (lstat %user-profile-directory))))
+    (symlink %current-profile %user-profile-directory))
+
+  (let ((s (stat %profile-directory #f)))
+    ;; Attempt to create /…/profiles/per-user/$USER if needed.
+    (unless (and s (eq? 'directory (stat:type s)))
+      (catch 'system-error
+        (lambda ()
+          (mkdir-p %profile-directory))
+        (lambda args
+          ;; Often, we cannot create %PROFILE-DIRECTORY because its
+          ;; parent directory is root-owned and we're running
+          ;; unprivileged.
+          (format (current-error-port)
+                  (_ "error: while creating directory `~a': ~a~%")
+                  %profile-directory
+                  (strerror (system-error-errno args)))
+          (format (current-error-port)
+                  (_ "Please create the `~a' directory, with you as the owner.~%")
+                  %profile-directory)
+          (rtfm))))
+
+    ;; Bail out if it's not owned by the user.
+    (unless (or (not s) (= (stat:uid s) (getuid)))
+      (format (current-error-port)
+              (_ "error: directory `~a' is not owned by you~%")
+              %profile-directory)
+      (format (current-error-port)
+              (_ "Please change the owner of `~a' to user ~s.~%")
+              %profile-directory (or (getenv "USER")
+                                     (getenv "LOGNAME")
+                                     (getuid)))
+      (rtfm))))
+
+(define* (process-package-actions store profile
+                                  #:key (install '()) (remove '())
+                                  dry-run? (use-substitutes? #t))
+  "Install/remove packages.
+
+INSTALL is a list of package patterns for installation.  Each element of
+the list may be a package, a list (PACKAGE OUTPUT), a string with name
+specification or a store path.
+
+REMOVE is a list of name specifications for removing from PROFILE
+manifest."
+  (define (package->manifest-entry* package output)
+    (check-package-freshness package)
+    ;; When given a package via `-e', install the first of its
+    ;; outputs (XXX).
+    (package->manifest-entry package output))
+
+  (define (entries-to-install install)
+    ;; Return a list of manifest entries for installing.
+    (filter-map (match-lambda
+                 ((? package? package)
+                  (package->manifest-entry* package "out"))
+                 (((? package? package) output)
+                  (package->manifest-entry* package output))
+                 ((? string? spec-or-path)
+                  (if (store-path? spec-or-path)
+                      (let-values (((name version)
+                                    (package-name->name+version
+                                     (store-path-package-name spec-or-path))))
+                        (manifest-entry
+                         (name name)
+                         (version version)
+                         (output #f)
+                         (item spec-or-path)))
+                      (let-values (((package output)
+                                    (specification->package+output spec-or-path)))
+                        (and package (package->manifest-entry* package output)))))
+                 (_ #f))
+                install))
+
+  (define (patterns-to-remove remove)
+    ;; Return a list of manifest patterns for removing.
+    (map (lambda (spec)
+           (call-with-values
+               (lambda ()
+                 (package-specification->name+version+output spec))
+             (lambda (name version output)
+               (manifest-pattern
+                (name name)
+                (version version)
+                (output output)))))
+         remove))
+
+  (let* ((manifest (profile-manifest profile))
+         (install  (entries-to-install install))
+         (remove   (patterns-to-remove remove))
+         (new      (manifest-add (manifest-remove manifest remove)
+                                 install))
+         (entries  (manifest-entries new)))
+
+    (unless (and (null? install) (null? remove))
+      (when (equal? profile %current-profile)
+        (ensure-default-profile))
+
+      (let* ((prof-drv (run-with-store store (profile-derivation new)))
+             (prof     (derivation->output-path prof-drv))
+             (remove   (manifest-matching-entries manifest remove)))
+        (show-what-to-remove/install remove install dry-run?)
+        (show-what-to-build store (list prof-drv)
+                            #:use-substitutes? use-substitutes?
+                            #:dry-run? dry-run?)
+
+        (cond
+         (dry-run? #t)
+         ((and (file-exists? profile)
+               (and=> (readlink* profile) (cut string=? prof <>)))
+          (format (current-error-port) (_ "nothing to be done~%")))
+         (else
+          (let* ((number (generation-number profile))
+
+                 ;; Always use NUMBER + 1 for the new profile,
+                 ;; possibly overwriting a "previous future
+                 ;; generation".
+                 (name   (generation-file-name profile
+                                               (+ 1 number))))
+            (and (build-derivations store (list prof-drv))
+                 (let ((count (length entries)))
+                   (switch-symlinks name prof)
+                   (switch-symlinks profile name)
+                   (maybe-register-gc-root store profile)
+                   (format #t (N_ "~a package in profile~%"
+                                  "~a packages in profile~%"
+                                  count)
+                           count)
+                   (display-search-paths entries
+                                         profile))))))))))
+
 \f
 ;;;
 ;;; Entry point.
@@ -742,65 +840,12 @@ removed from MANIFEST."
                 %default-options
                 #f))
 
-  (define (ensure-default-profile)
-    ;; Ensure the default profile symlink and directory exist and are
-    ;; writable.
-
-    (define (rtfm)
-      (format (current-error-port)
-              (_ "Try \"info '(guix) Invoking guix package'\" for \
-more information.~%"))
-      (exit 1))
-
-    ;; Create ~/.guix-profile if it doesn't exist yet.
-    (when (and %user-profile-directory
-               %current-profile
-               (not (false-if-exception
-                     (lstat %user-profile-directory))))
-      (symlink %current-profile %user-profile-directory))
-
-    (let ((s (stat %profile-directory #f)))
-      ;; Attempt to create /…/profiles/per-user/$USER if needed.
-      (unless (and s (eq? 'directory (stat:type s)))
-        (catch 'system-error
-          (lambda ()
-            (mkdir-p %profile-directory))
-          (lambda args
-            ;; Often, we cannot create %PROFILE-DIRECTORY because its
-            ;; parent directory is root-owned and we're running
-            ;; unprivileged.
-            (format (current-error-port)
-                    (_ "error: while creating directory `~a': ~a~%")
-                    %profile-directory
-                    (strerror (system-error-errno args)))
-            (format (current-error-port)
-                    (_ "Please create the `~a' directory, with you as the owner.~%")
-                    %profile-directory)
-            (rtfm))))
-
-      ;; Bail out if it's not owned by the user.
-      (unless (or (not s) (= (stat:uid s) (getuid)))
-        (format (current-error-port)
-                (_ "error: directory `~a' is not owned by you~%")
-                %profile-directory)
-        (format (current-error-port)
-                (_ "Please change the owner of `~a' to user ~s.~%")
-                %profile-directory (or (getenv "USER")
-                                       (getenv "LOGNAME")
-                                       (getuid)))
-        (rtfm))))
-
   (define (process-actions opts)
     ;; Process any install/remove/upgrade action from OPTS.
 
-    (define dry-run? (assoc-ref opts 'dry-run?))
-    (define profile  (assoc-ref opts 'profile))
-
-    (define (same-package? entry name output)
-      (match entry
-        (($ <manifest-entry> entry-name _ entry-output _ ...)
-         (and (equal? name entry-name)
-              (equal? output entry-output)))))
+    (define substitutes? (assoc-ref opts 'substitutes?))
+    (define dry-run?     (assoc-ref opts 'dry-run?))
+    (define profile      (assoc-ref opts 'profile))
 
     (define current-generation-number
       (generation-number profile))
@@ -869,61 +914,12 @@ more information.~%"))
              (_ #f))
             opts))
           (else
-           (let* ((manifest (profile-manifest profile))
-                  (install  (options->installable opts manifest))
-                  (remove   (options->removable opts manifest))
-                  (entries
-                   (append install
-                           (fold (lambda (package result)
-                                   (match package
-                                     (($ <manifest-entry> name _ out _ ...)
-                                      (filter (negate
-                                               (cut same-package? <>
-                                                    name out))
-                                              result))))
-                                 (manifest-entries
-                                  (manifest-remove manifest remove))
-                                 install)))
-                  (new      (make-manifest entries)))
-
-             (when (equal? profile %current-profile)
-               (ensure-default-profile))
-
-             (unless (and (null? install) (null? remove))
-               (let* ((prof-drv (run-with-store (%store)
-                                  (profile-derivation new)))
-                      (prof     (derivation->output-path prof-drv))
-                      (remove   (manifest-matching-entries manifest remove)))
-                 (show-what-to-remove/install remove install dry-run?)
-                 (show-what-to-build (%store) (list prof-drv)
-                                     #:use-substitutes?
-                                     (assoc-ref opts 'substitutes?)
-                                     #:dry-run? dry-run?)
-
-                 (cond
-                  (dry-run? #t)
-                  ((and (file-exists? profile)
-                        (and=> (readlink* profile) (cut string=? prof <>)))
-                   (format (current-error-port) (_ "nothing to be done~%")))
-                  (else
-                   (let* ((number (generation-number profile))
-
-                          ;; Always use NUMBER + 1 for the new profile,
-                          ;; possibly overwriting a "previous future
-                          ;; generation".
-                          (name   (generation-file-name profile
-                                                        (+ 1 number))))
-                     (and (build-derivations (%store) (list prof-drv))
-                          (let ((count (length entries)))
-                            (switch-symlinks name prof)
-                            (switch-symlinks profile name)
-                            (maybe-register-gc-root (%store) profile)
-                            (format #t (N_ "~a package in profile~%"
-                                           "~a packages in profile~%"
-                                           count)
-                                    count)
-                            (display-search-paths entries
-                                                  profile))))))))))))
+           (process-package-actions
+            (%store) profile
+            #:install (options->installable opts (profile-manifest profile))
+            #:remove  (options->removable opts)
+            #:use-substitutes? substitutes?
+            #:dry-run? dry-run?))))
 
   (define (process-query opts)
     ;; Process any query specified by OPTS.  Return #t when a query was
-- 
2.0.3

Re: [PATCH] Emacs interface for Guix

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> Thanks for pointing.  I've never contributed to a real project, so I
> don't know the rules actually :)

No problem.  :-)  There might still be unwritten rules, but we can fix
that as we go.

> From af4b8495969d70d59aa9f3f296628daeaf80b0d2 Mon Sep 17 00:00:00 2001
> From: Alex Kost <alezost@gmail.com>
> Date: Tue, 12 Aug 2014 12:32:16 +0400
> Subject: [PATCH 1/2] profiles: Add 'manifest-add'.
>
> * guix/profiles.scm (manifest-add): New procedure.
> * tests/profiles.scm (guile-1.8.8): New variable.
>   ("manifest-add"): New test.

Perfect.  I’ve pushed it, followed by a patch that changes
guix/scripts/package.scm to use ‘manifest-add’ (comments welcome.)

> From 5fd45b3f4216921837f522d56b20c4be0a58fe8e Mon Sep 17 00:00:00 2001
> From: Alex Kost <alezost@gmail.com>
> Date: Tue, 12 Aug 2014 13:54:23 +0400
> Subject: [PATCH 2/2] guix package: Add 'process-package-actions'.
>
> * guix/scripts/package.scm (process-package-actions): New procedure.
>   (guix-package): Use it.
>   [ensure-default-profile]: Move to top-level.
>   [substitutes?]: New variable.
>   [same-package?]: Remove.
>   (options->installable, options->removable): Change according to
>   'process-package-actions'.

This patch would need to be rebased on top of f48624f.

Were you planning on using ‘process-package-actions’ in the Emacs
interface?

That seems like a coarse-grain and clumsy interface.  Perhaps there are
tinier parts of it that could be moved to (guix profiles)?  For
instance, there’s no ‘manifest-upgrade’ at the moment.

What about introducing a <manifest-transaction> type that would contain
a list of packages to install, to remove, and to upgrade, and we could do:

  ;; Show what will/would be installed, removed, etc.
  (show-transaction manifest transaction #:dry-run? bool)

  ;; Do the installation/removal/upgrades listed in TRANSACTION, and
  ;; return the new manifest.
  (manifest-perform-transaction manifest transaction)

WDYT?

Thanks,
Ludo’.

Re: [PATCH] Emacs interface for Guix

[Alex Kost]

Ludovic Courtès (2014-08-12 18:19 +0400) wrote:

> Alex Kost <alezost@gmail.com> skribis:
>
>> Thanks for pointing.  I've never contributed to a real project, so I
>> don't know the rules actually :)
>
> No problem.  :-)  There might still be unwritten rules, but we can fix
> that as we go.
>
>> From af4b8495969d70d59aa9f3f296628daeaf80b0d2 Mon Sep 17 00:00:00 2001
>> From: Alex Kost <alezost@gmail.com>
>> Date: Tue, 12 Aug 2014 12:32:16 +0400
>> Subject: [PATCH 1/2] profiles: Add 'manifest-add'.
>>
>> * guix/profiles.scm (manifest-add): New procedure.
>> * tests/profiles.scm (guile-1.8.8): New variable.
>>   ("manifest-add"): New test.
>
> Perfect.  I’ve pushed it, followed by a patch that changes
> guix/scripts/package.scm to use ‘manifest-add’ (comments welcome.)

Thanks, you forgot to delete ‘same-package?’ from ‘guix-package’
[‘process-actions’] in your commit – it is a part of ‘manifest-add’ now.

>> From 5fd45b3f4216921837f522d56b20c4be0a58fe8e Mon Sep 17 00:00:00 2001
>> From: Alex Kost <alezost@gmail.com>
>> Date: Tue, 12 Aug 2014 13:54:23 +0400
>> Subject: [PATCH 2/2] guix package: Add 'process-package-actions'.
>>
>> * guix/scripts/package.scm (process-package-actions): New procedure.
>>   (guix-package): Use it.
>>   [ensure-default-profile]: Move to top-level.
>>   [substitutes?]: New variable.
>>   [same-package?]: Remove.
>>   (options->installable, options->removable): Change according to
>>   'process-package-actions'.
>
> This patch would need to be rebased on top of f48624f.
>
> Were you planning on using ‘process-package-actions’ in the Emacs
> interface?

Yes, actually I've been using that function for a couple of weeks, I
just didn't update "guix.el" repo for that.  But your suggestion below
is definitely better.

> That seems like a coarse-grain and clumsy interface.  Perhaps there are
> tinier parts of it that could be moved to (guix profiles)?  For
> instance, there’s no ‘manifest-upgrade’ at the moment.

I think ‘manifest-add’ already does what ‘manifest-upgrade’ would do: it
replaces entries with the same name.  So if there is installed
“guile-2.0.11:out” and a user installs “guile-1.8.8:out”, the previously
installed guile is removed from manifest.  I thought it's intended
behaviour and that's why ‘options->installable’ combines “to-install”
and “to-upgrade” options.  Could you explain what ‘manifest-upgrade’
should do?

> What about introducing a <manifest-transaction> type that would contain
> a list of packages to install, to remove, and to upgrade, and we could do:

I think only “install” part should contain a list of packages (or
(PACKAGE OUTPUT) things).  Upgrading and removing can be performed on
obsolete packages, so only a package specification of an installed
package is known in such cases.  Perhaps any pattern (package (with
"out" output), (package output), name specification) should be accepted.

So there will be ‘make-manifest-transaction’ function with #:install,
#:upgrade, #:remove keys.  Do I understand it right?

>   ;; Show what will/would be installed, removed, etc.
>   (show-transaction manifest transaction #:dry-run? bool)
>
>   ;; Do the installation/removal/upgrades listed in TRANSACTION, and
>   ;; return the new manifest.
>   (manifest-perform-transaction manifest transaction)

So ‘manifest-perform-transaction’ will open connection?  If so,
shouldn't it accept '#:dry-run' and '#:use-substitutes?' keys?

--
Alex

Re: [PATCH] Emacs interface for Guix

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> Ludovic Courtès (2014-08-12 18:19 +0400) wrote:

[...]

>> Perfect.  I’ve pushed it, followed by a patch that changes
>> guix/scripts/package.scm to use ‘manifest-add’ (comments welcome.)
>
> Thanks, you forgot to delete ‘same-package?’ from ‘guix-package’
> [‘process-actions’] in your commit – it is a part of ‘manifest-add’ now.

Oops, fixed now.

>>> From 5fd45b3f4216921837f522d56b20c4be0a58fe8e Mon Sep 17 00:00:00 2001
>>> From: Alex Kost <alezost@gmail.com>
>>> Date: Tue, 12 Aug 2014 13:54:23 +0400
>>> Subject: [PATCH 2/2] guix package: Add 'process-package-actions'.
>>>
>>> * guix/scripts/package.scm (process-package-actions): New procedure.
>>>   (guix-package): Use it.
>>>   [ensure-default-profile]: Move to top-level.
>>>   [substitutes?]: New variable.
>>>   [same-package?]: Remove.
>>>   (options->installable, options->removable): Change according to
>>>   'process-package-actions'.
>>
>> This patch would need to be rebased on top of f48624f.
>>
>> Were you planning on using ‘process-package-actions’ in the Emacs
>> interface?
>
> Yes, actually I've been using that function for a couple of weeks, I
> just didn't update "guix.el" repo for that.  But your suggestion below
> is definitely better.

Cool.

>> That seems like a coarse-grain and clumsy interface.  Perhaps there are
>> tinier parts of it that could be moved to (guix profiles)?  For
>> instance, there’s no ‘manifest-upgrade’ at the moment.
>
> I think ‘manifest-add’ already does what ‘manifest-upgrade’ would do: it
> replaces entries with the same name.  So if there is installed
> “guile-2.0.11:out” and a user installs “guile-1.8.8:out”, the previously
> installed guile is removed from manifest.  I thought it's intended
> behaviour and that's why ‘options->installable’ combines “to-install”
> and “to-upgrade” options.  Could you explain what ‘manifest-upgrade’
> should do?

Oh you’re right, currently upgrade and install are basically the same
thing.

>> What about introducing a <manifest-transaction> type that would contain
>> a list of packages to install, to remove, and to upgrade, and we could do:
>
> I think only “install” part should contain a list of packages (or
> (PACKAGE OUTPUT) things).  Upgrading and removing can be performed on
> obsolete packages, so only a package specification of an installed
> package is known in such cases.  Perhaps any pattern (package (with
> "out" output), (package output), name specification) should be accepted.

The arguments should be the same as (or compatible) for ‘manifest-add’
and ‘manifest-remove’.

So the list of packages could be installed could be a list of (PACKAGE
OUTPUT) as you note.

The list of packages to upgrade could a list of (PACKAGE OUTPUT) as
well, computed by ‘guix package’ or guix.el.  (The difficulty here is
that (guix profiles) should not depend on (gnu packages).)

The list of packages to remove should be a list of <manifest-pattern>.

> So there will be ‘make-manifest-transaction’ function with #:install,
> #:upgrade, #:remove keys.  Do I understand it right?

Rather, use (define-record-type* <manifest-transaction> ...), so we can
then write:

  (manifest-transaction
    (install lst1)
    (remove lst2)
    ...)

>>   ;; Show what will/would be installed, removed, etc.
>>   (show-transaction manifest transaction #:dry-run? bool)
>>
>>   ;; Do the installation/removal/upgrades listed in TRANSACTION, and
>>   ;; return the new manifest.
>>   (manifest-perform-transaction manifest transaction)
>
> So ‘manifest-perform-transaction’ will open connection?  If so,
> shouldn't it accept '#:dry-run' and '#:use-substitutes?' keys?

No, it would just return the new manifest, built by successive calls to
‘manifest-add’ and ‘manifest-remove’.  Very simple.

The actual profile is still built with ‘profile-derivation’.

Ludo’.

Re: [PATCH] Emacs interface for Guix

[Alex Kost]

[-- Attachment #1: Type: text/plain, Size: 2454 bytes --]

Ludovic Courtès (2014-08-12 23:50 +0400) wrote:

[...]

>>> What about introducing a <manifest-transaction> type that would contain
>>> a list of packages to install, to remove, and to upgrade, and we could do:
>>
>> I think only “install” part should contain a list of packages (or
>> (PACKAGE OUTPUT) things).  Upgrading and removing can be performed on
>> obsolete packages, so only a package specification of an installed
>> package is known in such cases.  Perhaps any pattern (package (with
>> "out" output), (package output), name specification) should be accepted.
>
> The arguments should be the same as (or compatible) for ‘manifest-add’
> and ‘manifest-remove’.
>
> So the list of packages could be installed could be a list of (PACKAGE
> OUTPUT) as you note.
>
> The list of packages to upgrade could a list of (PACKAGE OUTPUT) as
> well, computed by ‘guix package’ or guix.el.  (The difficulty here is
> that (guix profiles) should not depend on (gnu packages).)
>
> The list of packages to remove should be a list of <manifest-pattern>.
>
>> So there will be ‘make-manifest-transaction’ function with #:install,
>> #:upgrade, #:remove keys.  Do I understand it right?
>
> Rather, use (define-record-type* <manifest-transaction> ...), so we can
> then write:
>
>   (manifest-transaction
>     (install lst1)
>     (remove lst2)
>     ...)
>
>>>   ;; Show what will/would be installed, removed, etc.
>>>   (show-transaction manifest transaction #:dry-run? bool)
>>>
>>>   ;; Do the installation/removal/upgrades listed in TRANSACTION, and
>>>   ;; return the new manifest.
>>>   (manifest-perform-transaction manifest transaction)
>>
>> So ‘manifest-perform-transaction’ will open connection?  If so,
>> shouldn't it accept '#:dry-run' and '#:use-substitutes?' keys?
>
> No, it would just return the new manifest, built by successive calls to
> ‘manifest-add’ and ‘manifest-remove’.  Very simple.
>
> The actual profile is still built with ‘profile-derivation’.

I realized there could be a problem with (PACKAGE OUTPUT) elements.
They should be transformed into manifest entries, but
"guix/scripts/package.scm" uses ‘package->manifest-entry*’ for that, so
this cannot be performed in (guix profiles) module.  Perhaps “install”
should just contain a list of manifest entries.  WDYT?

And manifest-transaction stuff could look like this:


[-- Attachment #2: Type: text/plain, Size: 2584 bytes --]

(define-record-type* <manifest-transaction> manifest-transaction
  make-manifest-transaction
  manifest-transaction?
  (install manifest-transaction-install ; list of <manifest-entry>
           (default '()))
  (remove  manifest-transaction-remove  ; list of <manifest-pattern>
           (default '())))

(define (manifest-perform-transaction manifest transaction)
  "Perform TRANSACTION on MANIFEST and return new manifest."
  (let ((install (manifest-transaction-install transaction))
        (remove  (manifest-transaction-remove transaction)))
    (manifest-add (manifest-remove manifest remove)
                  install)))

(define* (show-transaction manifest transaction #:key dry-run?)
  "Display what will/would be installed/removed from MANIFEST by TRANSACTION."
  (let ((install (manifest-transaction-install transaction))
        (remove  (manifest-matching-entries
                  manifest
                  (manifest-transaction-remove transaction))))
    (match remove
      ((($ <manifest-entry> name version output path _) ..1)
       (let ((len    (length name))
             (remove (map (cut format #f "   ~a-~a\t~a\t~a" <> <> <> <>)
                          name version output path)))
         (if dry-run?
             (format (current-error-port)
                     (N_ "The following package would be removed:~%~{~a~%~}~%"
                         "The following packages would be removed:~%~{~a~%~}~%"
                         len)
                     remove)
             (format (current-error-port)
                     (N_ "The following package will be removed:~%~{~a~%~}~%"
                         "The following packages will be removed:~%~{~a~%~}~%"
                         len)
                     remove))))
      (_ #f))
    (match install
      ((($ <manifest-entry> name version output path _) ..1)
       (let ((len     (length name))
             (install (map (cut format #f "   ~a-~a\t~a\t~a" <> <> <> <>)
                           name version output path)))
         (if dry-run?
             (format (current-error-port)
                     (N_ "The following package would be installed:~%~{~a~%~}~%"
                         "The following packages would be installed:~%~{~a~%~}~%"
                         len)
                     install)
             (format (current-error-port)
                     (N_ "The following package will be installed:~%~{~a~%~}~%"
                         "The following packages will be installed:~%~{~a~%~}~%"
                         len)
                     install))))
      (_ #f))))

[-- Attachment #3: Type: text/plain, Size: 298 bytes --]


(I excluded “upgrade” part as it's the same as “install”, and
‘show-transaction’ is almost the same as ‘show-what-to-remove/install’
from "package.scm".)

Also I think "guix.el" should check for freshness too, so
‘check-package-freshness’ should probably be exported.

Re: [PATCH] Emacs interface for Guix

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> I realized there could be a problem with (PACKAGE OUTPUT) elements.
> They should be transformed into manifest entries, but
> "guix/scripts/package.scm" uses ‘package->manifest-entry*’ for that, so
> this cannot be performed in (guix profiles) module.  Perhaps “install”
> should just contain a list of manifest entries.  WDYT?

Yes, that’s fine too.

> And manifest-transaction stuff could look like this:
>
> (define-record-type* <manifest-transaction> manifest-transaction
>   make-manifest-transaction
>   manifest-transaction?
>   (install manifest-transaction-install ; list of <manifest-entry>
>            (default '()))
>   (remove  manifest-transaction-remove  ; list of <manifest-pattern>
>            (default '())))
>
> (define (manifest-perform-transaction manifest transaction)
>   "Perform TRANSACTION on MANIFEST and return new manifest."
>   (let ((install (manifest-transaction-install transaction))
>         (remove  (manifest-transaction-remove transaction)))
>     (manifest-add (manifest-remove manifest remove)
>                   install)))
>
> (define* (show-transaction manifest transaction #:key dry-run?)
>   "Display what will/would be installed/removed from MANIFEST by TRANSACTION."
>   (let ((install (manifest-transaction-install transaction))
>         (remove  (manifest-matching-entries
>                   manifest
>                   (manifest-transaction-remove transaction))))
>     (match remove
>       ((($ <manifest-entry> name version output path _) ..1)
>        (let ((len    (length name))
>              (remove (map (cut format #f "   ~a-~a\t~a\t~a" <> <> <> <>)
>                           name version output path)))
>          (if dry-run?
>              (format (current-error-port)
>                      (N_ "The following package would be removed:~%~{~a~%~}~%"
>                          "The following packages would be removed:~%~{~a~%~}~%"
>                          len)
>                      remove)
>              (format (current-error-port)
>                      (N_ "The following package will be removed:~%~{~a~%~}~%"
>                          "The following packages will be removed:~%~{~a~%~}~%"
>                          len)
>                      remove))))
>       (_ #f))
>     (match install
>       ((($ <manifest-entry> name version output path _) ..1)
>        (let ((len     (length name))
>              (install (map (cut format #f "   ~a-~a\t~a\t~a" <> <> <> <>)
>                            name version output path)))
>          (if dry-run?
>              (format (current-error-port)
>                      (N_ "The following package would be installed:~%~{~a~%~}~%"
>                          "The following packages would be installed:~%~{~a~%~}~%"
>                          len)
>                      install)
>              (format (current-error-port)
>                      (N_ "The following package will be installed:~%~{~a~%~}~%"
>                          "The following packages will be installed:~%~{~a~%~}~%"
>                          len)
>                      install))))
>       (_ #f))))

Looks good!

> (I excluded “upgrade” part as it's the same as “install”, and
> ‘show-transaction’ is almost the same as ‘show-what-to-remove/install’
> from "package.scm".)

Yes.

Could you turn the above thing into a patch with a commit log?  Bonus
points for ‘manifest-perform-transaction’ unit tests.  Make sure to add
a copyright line for yourself in profiles.scm.

And then a second patch to actually use it in (guix scripts package)
would be wonderful.  :-)

In the next iteration, ‘show-what-to-remove/install’ should report
packages that are going to be upgraded (by checking among ‘install’
those are already in the manifest.)

> Also I think "guix.el" should check for freshness too, so
> ‘check-package-freshness’ should probably be exported.

Yes, probably in the (gnu packages) module?

Thanks,
Ludo’.

Re: [PATCH] Emacs interface for Guix

[Alex Kost]

[-- Attachment #1: Type: text/plain, Size: 1625 bytes --]

Ludovic Courtès (2014-08-13 20:03 +0400) wrote:

> Alex Kost <alezost@gmail.com> skribis:
>
> [...]
>
>> (I excluded “upgrade” part as it's the same as “install”, and
>> ‘show-transaction’ is almost the same as ‘show-what-to-remove/install’
>> from "package.scm".)
>
> Yes.
>
> Could you turn the above thing into a patch with a commit log?  Bonus
> points for ‘manifest-perform-transaction’ unit tests.  Make sure to add
> a copyright line for yourself in profiles.scm.
>
> And then a second patch to actually use it in (guix scripts package)
> would be wonderful.  :-)

Ok, I'm attaching these patches.  But there are several issues there:

- I fixed a typo in "tests/profiles.scm" (“profile” -> “profiles”) – Is
  it ok to do this in that commit or should there be a separate commit?

- I added a copyright line to the test file as well.  Is it ok?

- The main thing: look at ‘manifest-show-transaction’ – unlike
  ‘show-what-to-remove/install’ it doesn't display an output path of a
  package item, because a store should be used for that.  So is it
  acceptable or should something be changed there?

> In the next iteration, ‘show-what-to-remove/install’ should report
> packages that are going to be upgraded (by checking among ‘install’
> those are already in the manifest.)

I'll try to do this.

>> Also I think "guix.el" should check for freshness too, so
>> ‘check-package-freshness’ should probably be exported.
>
> Yes, probably in the (gnu packages) module?

Probably, but I think I'm not competent to decide :)


[-- Attachment #2: 0001-profiles-Add-manifest-transaction.patch --]
[-- Type: text/x-patch, Size: 6630 bytes --]

From d2d3f9d296c26ad1d4a1e17d56ae3e3361ca02d7 Mon Sep 17 00:00:00 2001
From: Alex Kost <alezost@gmail.com>
Date: Thu, 14 Aug 2014 00:03:53 +0400
Subject: [PATCH 1/2] profiles: Add 'manifest-transaction'.

* guix/profiles.scm (<manifest-transaction>): New record-type.
  (manifest-perform-transaction): New procedure.
  (manifest-show-transaction): New procedure.
* tests/profiles.scm ("manifest-perform-transaction"): New test.
---
 guix/profiles.scm  | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/profiles.scm | 22 +++++++++++++++-
 2 files changed, 96 insertions(+), 1 deletion(-)

diff --git a/guix/profiles.scm b/guix/profiles.scm
index e921566..55a3348 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013, 2014 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2013 Nikita Karetnikov <nikita@karetnikov.org>
+;;; Copyright © 2014 Alex Kost <alezost@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -18,6 +19,7 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (guix profiles)
+  #:use-module (guix ui)
   #:use-module (guix utils)
   #:use-module (guix records)
   #:use-module (guix derivations)
@@ -51,6 +53,13 @@
             manifest-installed?
             manifest-matching-entries
 
+            manifest-transaction
+            manifest-transaction?
+            manifest-transaction-install
+            manifest-transaction-remove
+            manifest-perform-transaction
+            manifest-show-transaction
+
             profile-manifest
             package->manifest-entry
             profile-derivation
@@ -244,6 +253,72 @@ Remove MANIFEST entries that have the same name and output as ENTRIES."
 
 \f
 ;;;
+;;; Manifest transactions.
+;;;
+
+(define-record-type* <manifest-transaction> manifest-transaction
+  make-manifest-transaction
+  manifest-transaction?
+  (install manifest-transaction-install ; list of <manifest-entry>
+           (default '()))
+  (remove  manifest-transaction-remove  ; list of <manifest-pattern>
+           (default '())))
+
+(define (manifest-perform-transaction manifest transaction)
+  "Perform TRANSACTION on MANIFEST and return new manifest."
+  (let ((install (manifest-transaction-install transaction))
+        (remove  (manifest-transaction-remove transaction)))
+    (manifest-add (manifest-remove manifest remove)
+                  install)))
+
+(define* (manifest-show-transaction manifest transaction #:key dry-run?)
+  "Display what will/would be installed/removed from MANIFEST by TRANSACTION."
+  ;; TODO: Report upgrades more clearly.
+  (let ((install (manifest-transaction-install transaction))
+        (remove  (manifest-matching-entries
+                  manifest (manifest-transaction-remove transaction))))
+    (match remove
+      ((($ <manifest-entry> name version output path _) ..1)
+       (let ((len    (length name))
+             (remove (map (cut format #f "   ~a-~a\t~a\t~a" <> <> <> <>)
+                          name version output path)))
+         (if dry-run?
+             (format (current-error-port)
+                     (N_ "The following package would be removed:~%~{~a~%~}~%"
+                         "The following packages would be removed:~%~{~a~%~}~%"
+                         len)
+                     remove)
+             (format (current-error-port)
+                     (N_ "The following package will be removed:~%~{~a~%~}~%"
+                         "The following packages will be removed:~%~{~a~%~}~%"
+                         len)
+                     remove))))
+      (_ #f))
+    (match install
+      ((($ <manifest-entry> name version output item _) ..1)
+       (let ((len     (length name))
+             (install (map (lambda (name version output item)
+                             (if (package? item)
+                                 (format #f "   ~a-~a\t~a"
+                                         name version output)
+                                 (format #f "   ~a-~a\t~a\t~a"
+                                         name version output item)))
+                           name version output item)))
+         (if dry-run?
+             (format (current-error-port)
+                     (N_ "The following package would be installed:~%~{~a~%~}~%"
+                         "The following packages would be installed:~%~{~a~%~}~%"
+                         len)
+                     install)
+             (format (current-error-port)
+                     (N_ "The following package will be installed:~%~{~a~%~}~%"
+                         "The following packages will be installed:~%~{~a~%~}~%"
+                         len)
+                     install))))
+      (_ #f))))
+
+\f
+;;;
 ;;; Profiles.
 ;;;
 
diff --git a/tests/profiles.scm b/tests/profiles.scm
index b2919d7..e1f1eef 100644
--- a/tests/profiles.scm
+++ b/tests/profiles.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013, 2014 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2014 Alex Kost <alezost@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -26,7 +27,7 @@
   #:use-module (ice-9 match)
   #:use-module (srfi srfi-64))
 
-;; Test the (guix profile) module.
+;; Test the (guix profiles) module.
 
 (define %store
   (open-connection))
@@ -122,6 +123,25 @@
            (_ #f))
          (equal? m3 m4))))
 
+(test-assert "manifest-perform-transaction"
+  (let* ((m0 (manifest (list guile-2.0.9 guile-2.0.9:debug)))
+         (t1 (manifest-transaction
+              (install (list guile-1.8.8))
+              (remove (list (manifest-pattern (name "guile")
+                                              (output "debug"))))))
+         (t2 (manifest-transaction
+              (remove (list (manifest-pattern (name "guile")
+                                              (version "2.0.9")
+                                              (output #f))))))
+         (m1 (manifest-perform-transaction m0 t1))
+         (m2 (manifest-perform-transaction m1 t2))
+         (m3 (manifest-perform-transaction m0 t2)))
+    (and (match (manifest-entries m1)
+           ((($ <manifest-entry> "guile" "1.8.8" "out")) #t)
+           (_ #f))
+         (equal? m1 m2)
+         (null? (manifest-entries m3)))))
+
 (test-assert "profile-derivation"
   (run-with-store %store
     (mlet* %store-monad
-- 
2.0.3


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-guix-package-Use-manifest-transaction.patch --]
[-- Type: text/x-patch, Size: 4669 bytes --]

From 5358263f259ea099bbcb62a6bc548c6c9fdb1567 Mon Sep 17 00:00:00 2001
From: Alex Kost <alezost@gmail.com>
Date: Thu, 14 Aug 2014 00:15:48 +0400
Subject: [PATCH 2/2] guix package: Use 'manifest-transaction'.

* guix/scripts/package.scm (guix-package)[process-actions]: Use
  'manifest-transaction' instead of the equivalent code.
  (show-what-to-remove/install): Remove.
---
 guix/scripts/package.scm | 60 ++++++++----------------------------------------
 1 file changed, 10 insertions(+), 50 deletions(-)

diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index 3bfef4f..b7bdadc 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -184,49 +184,6 @@ DURATION-RELATION with the current time."
          filter-by-duration)
         (else #f)))
 
-(define (show-what-to-remove/install remove install dry-run?)
-  "Given the manifest entries listed in REMOVE and INSTALL, display the
-packages that will/would be installed and removed."
-  ;; TODO: Report upgrades more clearly.
-  (match remove
-    ((($ <manifest-entry> name version output path _) ..1)
-     (let ((len    (length name))
-           (remove (map (cut format #f "   ~a-~a\t~a\t~a" <> <> <> <>)
-                        name version output path)))
-       (if dry-run?
-           (format (current-error-port)
-                   (N_ "The following package would be removed:~%~{~a~%~}~%"
-                       "The following packages would be removed:~%~{~a~%~}~%"
-                       len)
-                   remove)
-           (format (current-error-port)
-                   (N_ "The following package will be removed:~%~{~a~%~}~%"
-                       "The following packages will be removed:~%~{~a~%~}~%"
-                       len)
-                   remove))))
-    (_ #f))
-  (match install
-    ((($ <manifest-entry> name version output item _) ..1)
-     (let ((len     (length name))
-           (install (map (lambda (name version output item)
-                           (format #f "   ~a-~a\t~a\t~a" name version output
-                                   (if (package? item)
-                                       (package-output (%store) item output)
-                                       item)))
-                         name version output item)))
-       (if dry-run?
-           (format (current-error-port)
-                   (N_ "The following package would be installed:~%~{~a~%~}~%"
-                       "The following packages would be installed:~%~{~a~%~}~%"
-                       len)
-                   install)
-           (format (current-error-port)
-                   (N_ "The following package will be installed:~%~{~a~%~}~%"
-                       "The following packages will be installed:~%~{~a~%~}~%"
-                       len)
-                   install))))
-    (_ #f)))
-
 \f
 ;;;
 ;;; Package specifications.
@@ -863,21 +820,24 @@ more information.~%"))
              (_ #f))
             opts))
           (else
-           (let* ((manifest (profile-manifest profile))
-                  (install  (options->installable opts manifest))
-                  (remove   (options->removable opts manifest))
-                  (new      (manifest-add (manifest-remove manifest remove)
-                                          install)))
+           (let* ((manifest    (profile-manifest profile))
+                  (install     (options->installable opts manifest))
+                  (remove      (options->removable opts manifest))
+                  (transaction (manifest-transaction (install install)
+                                                     (remove remove)))
+                  (new         (manifest-perform-transaction
+                                manifest transaction)))
 
              (when (equal? profile %current-profile)
                (ensure-default-profile))
 
              (unless (and (null? install) (null? remove))
                (let* ((prof-drv (run-with-store (%store)
-                                  (profile-derivation new)))
+                                                (profile-derivation new)))
                       (prof     (derivation->output-path prof-drv))
                       (remove   (manifest-matching-entries manifest remove)))
-                 (show-what-to-remove/install remove install dry-run?)
+                 (manifest-show-transaction manifest transaction
+                                            #:dry-run? dry-run?)
                  (show-what-to-build (%store) (list prof-drv)
                                      #:use-substitutes?
                                      (assoc-ref opts 'substitutes?)
-- 
2.0.3


[-- Attachment #4: Type: text/plain, Size: 14 bytes --]


--
Alex Kost

Re: [PATCH] Emacs interface for Guix

[Alex Kost]

[-- Attachment #1: Type: text/plain, Size: 1405 bytes --]

Alex Kost (2014-08-14 00:58 +0400) wrote:

> Ludovic Courtès (2014-08-13 20:03 +0400) wrote:
>
> [...]
>
>> Could you turn the above thing into a patch with a commit log?  Bonus
>> points for ‘manifest-perform-transaction’ unit tests.  Make sure to add
>> a copyright line for yourself in profiles.scm.
>>
>> And then a second patch to actually use it in (guix scripts package)
>> would be wonderful.  :-)
>
> Ok, I'm attaching these patches.  But there are several issues there:
>
> - I fixed a typo in "tests/profiles.scm" (“profile” -> “profiles”) – Is
>   it ok to do this in that commit or should there be a separate commit?
>
> - I added a copyright line to the test file as well.  Is it ok?
>
> - The main thing: look at ‘manifest-show-transaction’ – unlike
>   ‘show-what-to-remove/install’ it doesn't display an output path of a
>   package item, because a store should be used for that.  So is it
>   acceptable or should something be changed there?
>
>> In the next iteration, ‘show-what-to-remove/install’ should report
>> packages that are going to be upgraded (by checking among ‘install’
>> those are already in the manifest.)
>
> I'll try to do this.

Hello and pardon for replying to my own letter.

If the displaying an ouput path is not an issue, what about the
following variant of ‘manifest-show-transaction’:


[-- Attachment #2: manifest-show-transaction.scm --]
[-- Type: text/plain, Size: 1613 bytes --]

(define* (manifest-show-transaction manifest transaction #:key dry-run?)
  "Display what will/would be installed/removed from MANIFEST by TRANSACTION."
  (define (display-entries entries action-string)
    (match entries
      ((($ <manifest-entry> name version output item _) ..1)
       (let ((len (length name))
             (package-strings
              (map (lambda (name version output item)
                     (if (package? item)
                         (format #f "   ~a-~a\t~a" name version output)
                         (format #f "   ~a-~a\t~a\t~a" name version output item)))
                   name version output item)))
         (format (current-error-port)
                 (N_ "The following package ~:[will~;would~] be ~a:~%~{~a~%~}~%"
                     "The following packages ~:[will~;would~] be ~a:~%~{~a~%~}~%"
                     len)
                 dry-run? action-string package-strings)))
      (_ #f)))

  (let* ((remove  (manifest-matching-entries
                   manifest (manifest-transaction-remove transaction)))
         (install (manifest-transaction-install transaction))
         (upgrade (append-map
                   (lambda (entry)
                     (manifest-matching-entries
                      manifest
                      (list (manifest-pattern
                             (name (manifest-entry-name entry))
                             (output (manifest-entry-output entry))))))
                   install)))
    (display-entries upgrade "upgraded (removed)")
    (display-entries install "installed")
    (display-entries remove "removed")))

[-- Attachment #3: Type: text/plain, Size: 588 bytes --]


I tried to avoid the code duplicating, so it became more compact and
perhaps less readable.  Also I added reporting about the packages to
upgrade: I thought as they are going to be replaced by the packages to
install, it is ok to add “(removed)” there.  So an output should look
like this (assuming "file-5.17" and "guile-2.0.9" are installed and are
being upgraded):

The following packages will be upgraded (removed):
   file-5.17	out	/gnu/store/...
   guile-2.0.9	out	/gnu/store/...

The following packages will be installed:
   file-5.18	out
   guile-2.0.11	out

Re: [PATCH] Emacs interface for Guix

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> (define* (manifest-show-transaction manifest transaction #:key dry-run?)
>   "Display what will/would be installed/removed from MANIFEST by TRANSACTION."

[...]

>          (format (current-error-port)
>                  (N_ "The following package ~:[will~;would~] be ~a:~%~{~a~%~}~%"
>                      "The following packages ~:[will~;would~] be ~a:~%~{~a~%~}~%"
>                      len)
>                  dry-run? action-string package-strings)))

[...]

>     (display-entries upgrade "upgraded (removed)")
>     (display-entries install "installed")
>     (display-entries remove "removed")))

Computed strings like impede correct internationalization.  The whole
sentences must be kept intact, to make sure people can translate them
correctly.  So that means repeating things a bit, but that’s
unavoidable.

> I tried to avoid the code duplicating, so it became more compact and
> perhaps less readable.  Also I added reporting about the packages to
> upgrade: I thought as they are going to be replaced by the packages to
> install, it is ok to add “(removed)” there.  So an output should look
> like this (assuming "file-5.17" and "guile-2.0.9" are installed and are
> being upgraded):
>
> The following packages will be upgraded (removed):
>    file-5.17	out	/gnu/store/...
>    guile-2.0.9	out	/gnu/store/...
>
> The following packages will be installed:
>    file-5.18	out
>    guile-2.0.11	out

Ideally, I would just like to see:

 The following packages will be upgraded:
    file-5.17	out	/gnu/store/...
    guile-2.0.9	out	/gnu/store/...

and not see them listed under “will be installed.”

I would just keep the current messages for this patch series, and come
up with an improved message format in a separate patch.

WDYT?

Thanks,
Ludo’.

[PATCH] manifest-transaction

[Alex Kost]

[-- Attachment #1: Type: text/plain, Size: 1836 bytes --]

Ludovic Courtès (2014-08-16 13:27 +0400) wrote:

[...]

> Computed strings like impede correct internationalization.  The whole
> sentences must be kept intact, to make sure people can translate them
> correctly.  So that means repeating things a bit, but that’s
> unavoidable.

Ah, indeed, I didn't think about internationalization.

>> I tried to avoid the code duplicating, so it became more compact and
>> perhaps less readable.  Also I added reporting about the packages to
>> upgrade: I thought as they are going to be replaced by the packages to
>> install, it is ok to add “(removed)” there.  So an output should look
>> like this (assuming "file-5.17" and "guile-2.0.9" are installed and are
>> being upgraded):
>>
>> The following packages will be upgraded (removed):
>>    file-5.17	out	/gnu/store/...
>>    guile-2.0.9	out	/gnu/store/...
>>
>> The following packages will be installed:
>>    file-5.18	out
>>    guile-2.0.11	out
>
> Ideally, I would just like to see:
>
>  The following packages will be upgraded:
>     file-5.17	out	/gnu/store/...
>     guile-2.0.9	out	/gnu/store/...
>
> and not see them listed under “will be installed.”

As you wish (although I would prefer to see what is upgraded and what is
installed in that manner).

> I would just keep the current messages for this patch series, and come
> up with an improved message format in a separate patch.
>
> WDYT?

No problem, so here are the patches again (the second one is modified: I
forgot to delete one unused line last time).  And just in case I'm
mentioning an issue with ‘manifest-show-transaction’ again: unlike
‘show-what-to-remove/install’, it doesn't display an output path of a
package item, because a store should be used for that.  So should
something be done with it?


[-- Attachment #2: 0001-profiles-Add-manifest-transaction.patch --]
[-- Type: text/x-patch, Size: 6630 bytes --]

From d2d3f9d296c26ad1d4a1e17d56ae3e3361ca02d7 Mon Sep 17 00:00:00 2001
From: Alex Kost <alezost@gmail.com>
Date: Thu, 14 Aug 2014 00:03:53 +0400
Subject: [PATCH 1/2] profiles: Add 'manifest-transaction'.

* guix/profiles.scm (<manifest-transaction>): New record-type.
  (manifest-perform-transaction): New procedure.
  (manifest-show-transaction): New procedure.
* tests/profiles.scm ("manifest-perform-transaction"): New test.
---
 guix/profiles.scm  | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/profiles.scm | 22 +++++++++++++++-
 2 files changed, 96 insertions(+), 1 deletion(-)

diff --git a/guix/profiles.scm b/guix/profiles.scm
index e921566..55a3348 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013, 2014 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2013 Nikita Karetnikov <nikita@karetnikov.org>
+;;; Copyright © 2014 Alex Kost <alezost@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -18,6 +19,7 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (guix profiles)
+  #:use-module (guix ui)
   #:use-module (guix utils)
   #:use-module (guix records)
   #:use-module (guix derivations)
@@ -51,6 +53,13 @@
             manifest-installed?
             manifest-matching-entries
 
+            manifest-transaction
+            manifest-transaction?
+            manifest-transaction-install
+            manifest-transaction-remove
+            manifest-perform-transaction
+            manifest-show-transaction
+
             profile-manifest
             package->manifest-entry
             profile-derivation
@@ -244,6 +253,72 @@ Remove MANIFEST entries that have the same name and output as ENTRIES."
 
 \f
 ;;;
+;;; Manifest transactions.
+;;;
+
+(define-record-type* <manifest-transaction> manifest-transaction
+  make-manifest-transaction
+  manifest-transaction?
+  (install manifest-transaction-install ; list of <manifest-entry>
+           (default '()))
+  (remove  manifest-transaction-remove  ; list of <manifest-pattern>
+           (default '())))
+
+(define (manifest-perform-transaction manifest transaction)
+  "Perform TRANSACTION on MANIFEST and return new manifest."
+  (let ((install (manifest-transaction-install transaction))
+        (remove  (manifest-transaction-remove transaction)))
+    (manifest-add (manifest-remove manifest remove)
+                  install)))
+
+(define* (manifest-show-transaction manifest transaction #:key dry-run?)
+  "Display what will/would be installed/removed from MANIFEST by TRANSACTION."
+  ;; TODO: Report upgrades more clearly.
+  (let ((install (manifest-transaction-install transaction))
+        (remove  (manifest-matching-entries
+                  manifest (manifest-transaction-remove transaction))))
+    (match remove
+      ((($ <manifest-entry> name version output path _) ..1)
+       (let ((len    (length name))
+             (remove (map (cut format #f "   ~a-~a\t~a\t~a" <> <> <> <>)
+                          name version output path)))
+         (if dry-run?
+             (format (current-error-port)
+                     (N_ "The following package would be removed:~%~{~a~%~}~%"
+                         "The following packages would be removed:~%~{~a~%~}~%"
+                         len)
+                     remove)
+             (format (current-error-port)
+                     (N_ "The following package will be removed:~%~{~a~%~}~%"
+                         "The following packages will be removed:~%~{~a~%~}~%"
+                         len)
+                     remove))))
+      (_ #f))
+    (match install
+      ((($ <manifest-entry> name version output item _) ..1)
+       (let ((len     (length name))
+             (install (map (lambda (name version output item)
+                             (if (package? item)
+                                 (format #f "   ~a-~a\t~a"
+                                         name version output)
+                                 (format #f "   ~a-~a\t~a\t~a"
+                                         name version output item)))
+                           name version output item)))
+         (if dry-run?
+             (format (current-error-port)
+                     (N_ "The following package would be installed:~%~{~a~%~}~%"
+                         "The following packages would be installed:~%~{~a~%~}~%"
+                         len)
+                     install)
+             (format (current-error-port)
+                     (N_ "The following package will be installed:~%~{~a~%~}~%"
+                         "The following packages will be installed:~%~{~a~%~}~%"
+                         len)
+                     install))))
+      (_ #f))))
+
+\f
+;;;
 ;;; Profiles.
 ;;;
 
diff --git a/tests/profiles.scm b/tests/profiles.scm
index b2919d7..e1f1eef 100644
--- a/tests/profiles.scm
+++ b/tests/profiles.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013, 2014 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2014 Alex Kost <alezost@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -26,7 +27,7 @@
   #:use-module (ice-9 match)
   #:use-module (srfi srfi-64))
 
-;; Test the (guix profile) module.
+;; Test the (guix profiles) module.
 
 (define %store
   (open-connection))
@@ -122,6 +123,25 @@
            (_ #f))
          (equal? m3 m4))))
 
+(test-assert "manifest-perform-transaction"
+  (let* ((m0 (manifest (list guile-2.0.9 guile-2.0.9:debug)))
+         (t1 (manifest-transaction
+              (install (list guile-1.8.8))
+              (remove (list (manifest-pattern (name "guile")
+                                              (output "debug"))))))
+         (t2 (manifest-transaction
+              (remove (list (manifest-pattern (name "guile")
+                                              (version "2.0.9")
+                                              (output #f))))))
+         (m1 (manifest-perform-transaction m0 t1))
+         (m2 (manifest-perform-transaction m1 t2))
+         (m3 (manifest-perform-transaction m0 t2)))
+    (and (match (manifest-entries m1)
+           ((($ <manifest-entry> "guile" "1.8.8" "out")) #t)
+           (_ #f))
+         (equal? m1 m2)
+         (null? (manifest-entries m3)))))
+
 (test-assert "profile-derivation"
   (run-with-store %store
     (mlet* %store-monad
-- 
2.0.3


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-guix-package-Use-manifest-transaction.patch --]
[-- Type: text/x-patch, Size: 4739 bytes --]

From 65511b43843742f2e9bea9bfd611418cf399e524 Mon Sep 17 00:00:00 2001
From: Alex Kost <alezost@gmail.com>
Date: Thu, 14 Aug 2014 00:15:48 +0400
Subject: [PATCH 2/2] guix package: Use 'manifest-transaction'.

* guix/scripts/package.scm (guix-package)[process-actions]: Use
  'manifest-transaction' instead of the equivalent code.
  (show-what-to-remove/install): Remove.
---
 guix/scripts/package.scm | 63 +++++++++---------------------------------------
 1 file changed, 11 insertions(+), 52 deletions(-)

diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index 3bfef4f..6f920d3 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -184,49 +184,6 @@ DURATION-RELATION with the current time."
          filter-by-duration)
         (else #f)))
 
-(define (show-what-to-remove/install remove install dry-run?)
-  "Given the manifest entries listed in REMOVE and INSTALL, display the
-packages that will/would be installed and removed."
-  ;; TODO: Report upgrades more clearly.
-  (match remove
-    ((($ <manifest-entry> name version output path _) ..1)
-     (let ((len    (length name))
-           (remove (map (cut format #f "   ~a-~a\t~a\t~a" <> <> <> <>)
-                        name version output path)))
-       (if dry-run?
-           (format (current-error-port)
-                   (N_ "The following package would be removed:~%~{~a~%~}~%"
-                       "The following packages would be removed:~%~{~a~%~}~%"
-                       len)
-                   remove)
-           (format (current-error-port)
-                   (N_ "The following package will be removed:~%~{~a~%~}~%"
-                       "The following packages will be removed:~%~{~a~%~}~%"
-                       len)
-                   remove))))
-    (_ #f))
-  (match install
-    ((($ <manifest-entry> name version output item _) ..1)
-     (let ((len     (length name))
-           (install (map (lambda (name version output item)
-                           (format #f "   ~a-~a\t~a\t~a" name version output
-                                   (if (package? item)
-                                       (package-output (%store) item output)
-                                       item)))
-                         name version output item)))
-       (if dry-run?
-           (format (current-error-port)
-                   (N_ "The following package would be installed:~%~{~a~%~}~%"
-                       "The following packages would be installed:~%~{~a~%~}~%"
-                       len)
-                   install)
-           (format (current-error-port)
-                   (N_ "The following package will be installed:~%~{~a~%~}~%"
-                       "The following packages will be installed:~%~{~a~%~}~%"
-                       len)
-                   install))))
-    (_ #f)))
-
 \f
 ;;;
 ;;; Package specifications.
@@ -863,21 +820,23 @@ more information.~%"))
              (_ #f))
             opts))
           (else
-           (let* ((manifest (profile-manifest profile))
-                  (install  (options->installable opts manifest))
-                  (remove   (options->removable opts manifest))
-                  (new      (manifest-add (manifest-remove manifest remove)
-                                          install)))
+           (let* ((manifest    (profile-manifest profile))
+                  (install     (options->installable opts manifest))
+                  (remove      (options->removable opts manifest))
+                  (transaction (manifest-transaction (install install)
+                                                     (remove remove)))
+                  (new         (manifest-perform-transaction
+                                manifest transaction)))
 
              (when (equal? profile %current-profile)
                (ensure-default-profile))
 
              (unless (and (null? install) (null? remove))
                (let* ((prof-drv (run-with-store (%store)
-                                  (profile-derivation new)))
-                      (prof     (derivation->output-path prof-drv))
-                      (remove   (manifest-matching-entries manifest remove)))
-                 (show-what-to-remove/install remove install dry-run?)
+                                                (profile-derivation new)))
+                      (prof     (derivation->output-path prof-drv)))
+                 (manifest-show-transaction manifest transaction
+                                            #:dry-run? dry-run?)
                  (show-what-to-build (%store) (list prof-drv)
                                      #:use-substitutes?
                                      (assoc-ref opts 'substitutes?)
-- 
2.0.3

[PATCH] profiles: Report about upgrades.

[Alex Kost]

[-- Attachment #1: Type: text/plain, Size: 2029 bytes --]

Ludovic Courtès (2014-08-16 13:27 +0400) wrote:

> Alex Kost <alezost@gmail.com> skribis:
>
>> (define* (manifest-show-transaction manifest transaction #:key dry-run?)
>>   "Display what will/would be installed/removed from MANIFEST by TRANSACTION."
>
> [...]
>
>>          (format (current-error-port)
>>                  (N_ "The following package ~:[will~;would~] be ~a:~%~{~a~%~}~%"
>>                      "The following packages ~:[will~;would~] be ~a:~%~{~a~%~}~%"
>>                      len)
>>                  dry-run? action-string package-strings)))
>
> [...]
>
>>     (display-entries upgrade "upgraded (removed)")
>>     (display-entries install "installed")
>>     (display-entries remove "removed")))
>
> Computed strings like impede correct internationalization.  The whole
> sentences must be kept intact, to make sure people can translate them
> correctly.  So that means repeating things a bit, but that’s
> unavoidable.
>
>> I tried to avoid the code duplicating, so it became more compact and
>> perhaps less readable.  Also I added reporting about the packages to
>> upgrade: I thought as they are going to be replaced by the packages to
>> install, it is ok to add “(removed)” there.  So an output should look
>> like this (assuming "file-5.17" and "guile-2.0.9" are installed and are
>> being upgraded):
>>
>> The following packages will be upgraded (removed):
>>    file-5.17	out	/gnu/store/...
>>    guile-2.0.9	out	/gnu/store/...
>>
>> The following packages will be installed:
>>    file-5.18	out
>>    guile-2.0.11	out
>
> Ideally, I would just like to see:
>
>  The following packages will be upgraded:
>     file-5.17	out	/gnu/store/...
>     guile-2.0.9	out	/gnu/store/...
>
> and not see them listed under “will be installed.”
>
> I would just keep the current messages for this patch series, and come
> up with an improved message format in a separate patch.

Here is my try to add messages about upgraded packages.  Is it OK?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-profiles-Report-about-upgrades.patch --]
[-- Type: text/x-diff, Size: 4371 bytes --]

From 54c027a227d91acccee68202c24e6ebc27c828fb Mon Sep 17 00:00:00 2001
From: Alex Kost <alezost@gmail.com>
Date: Wed, 20 Aug 2014 15:52:36 +0400
Subject: [PATCH] profiles: Report about upgrades.

* guix/profiles.scm (manifest-show-transaction): Report about upgrades.
---
 guix/profiles.scm | 56 ++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 43 insertions(+), 13 deletions(-)

diff --git a/guix/profiles.scm b/guix/profiles.scm
index 7fff25a..d2d9b9e 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -275,15 +275,34 @@ Remove MANIFEST entries that have the same name and output as ENTRIES."
 (define* (manifest-show-transaction store manifest transaction
                                     #:key dry-run?)
   "Display what will/would be installed/removed from MANIFEST by TRANSACTION."
-  ;; TODO: Report upgrades more clearly.
-  (let ((install (manifest-transaction-install transaction))
-        (remove  (manifest-matching-entries
-                  manifest (manifest-transaction-remove transaction))))
+  (define (package-strings name version output item)
+    (map (lambda (name version output item)
+           (format #f "   ~a-~a\t~a\t~a" name version output
+                   (if (package? item)
+                       (package-output store item output)
+                       item)))
+         name version output item))
+
+  (let* ((remove (manifest-matching-entries
+                  manifest (manifest-transaction-remove transaction)))
+         (install/upgrade (manifest-transaction-install transaction))
+         (install '())
+         (upgrade (append-map
+                   (lambda (entry)
+                     (let ((matching
+                            (manifest-matching-entries
+                             manifest
+                             (list (manifest-pattern
+                                    (name   (manifest-entry-name entry))
+                                    (output (manifest-entry-output entry)))))))
+                       (when (null? matching)
+                         (set! install (cons entry install)))
+                       matching))
+                   install/upgrade)))
     (match remove
-      ((($ <manifest-entry> name version output path _) ..1)
+      ((($ <manifest-entry> name version output item _) ..1)
        (let ((len    (length name))
-             (remove (map (cut format #f "   ~a-~a\t~a\t~a" <> <> <> <>)
-                          name version output path)))
+             (remove (package-strings name version output item)))
          (if dry-run?
              (format (current-error-port)
                      (N_ "The following package would be removed:~%~{~a~%~}~%"
@@ -296,15 +315,26 @@ Remove MANIFEST entries that have the same name and output as ENTRIES."
                          len)
                      remove))))
       (_ #f))
+    (match upgrade
+      ((($ <manifest-entry> name version output item _) ..1)
+       (let ((len     (length name))
+             (upgrade (package-strings name version output item)))
+         (if dry-run?
+             (format (current-error-port)
+                     (N_ "The following package would be upgraded:~%~{~a~%~}~%"
+                         "The following packages would be upgraded:~%~{~a~%~}~%"
+                         len)
+                     upgrade)
+             (format (current-error-port)
+                     (N_ "The following package will be upgraded:~%~{~a~%~}~%"
+                         "The following packages will be upgraded:~%~{~a~%~}~%"
+                         len)
+                     upgrade))))
+      (_ #f))
     (match install
       ((($ <manifest-entry> name version output item _) ..1)
        (let ((len     (length name))
-             (install (map (lambda (name version output item)
-                             (format #f "   ~a-~a\t~a\t~a" name version output
-                                     (if (package? item)
-                                         (package-output store item output)
-                                         item)))
-                           name version output item)))
+             (install (package-strings name version output item)))
          (if dry-run?
              (format (current-error-port)
                      (N_ "The following package would be installed:~%~{~a~%~}~%"
-- 
2.0.3

Re: [PATCH] profiles: Report about upgrades.

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> From 54c027a227d91acccee68202c24e6ebc27c828fb Mon Sep 17 00:00:00 2001
> From: Alex Kost <alezost@gmail.com>
> Date: Wed, 20 Aug 2014 15:52:36 +0400
> Subject: [PATCH] profiles: Report about upgrades.
>
> * guix/profiles.scm (manifest-show-transaction): Report about upgrades.

Perfect.  Applied, thanks!

Ludo’.

Re: [PATCH] profiles: Report about upgrades.

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> +  (let* ((remove (manifest-matching-entries
> +                  manifest (manifest-transaction-remove transaction)))
> +         (install/upgrade (manifest-transaction-install transaction))
> +         (install '())
> +         (upgrade (append-map
> +                   (lambda (entry)
> +                     (let ((matching
> +                            (manifest-matching-entries
> +                             manifest
> +                             (list (manifest-pattern
> +                                    (name   (manifest-entry-name entry))
> +                                    (output (manifest-entry-output entry)))))))
> +                       (when (null? matching)
> +                         (set! install (cons entry install)))
> +                       matching))
> +                   install/upgrade)))

Somehow I had overlooked the ‘set!’ here.  ;-)  I’ve just added an
auxiliary procedure, ‘manifest-transaction-effects’, which does that in
a functional way.  Let me know if there’s anything wrong.

Ludo’.

Re: [PATCH] profiles: Report about upgrades.

[Alex Kost]

Ludovic Courtès (2014-08-30 23:56 +0400) wrote:

> Alex Kost <alezost@gmail.com> skribis:
>
>> +  (let* ((remove (manifest-matching-entries
>> +                  manifest (manifest-transaction-remove transaction)))
>> +         (install/upgrade (manifest-transaction-install transaction))
>> +         (install '())
>> +         (upgrade (append-map
>> +                   (lambda (entry)
>> +                     (let ((matching
>> +                            (manifest-matching-entries
>> +                             manifest
>> +                             (list (manifest-pattern
>> +                                    (name   (manifest-entry-name entry))
>> +                                    (output (manifest-entry-output entry)))))))
>> +                       (when (null? matching)
>> +                         (set! install (cons entry install)))
>> +                       matching))
>> +                   install/upgrade)))
>
> Somehow I had overlooked the ‘set!’ here.  ;-)  I’ve just added an
> auxiliary procedure, ‘manifest-transaction-effects’, which does that in
> a functional way.  Let me know if there’s anything wrong.

Sorry, I didn't know how to avoid ‘set!’ there.

But is it correct to report it like that?  I mean if a user has
“guile-1.8.8” and installs “guile-2.0.9” then (with your variant) he
gets:

--8<---------------cut here---------------start------------->8---
The following package will be upgraded:
   guile-2.0.9	out	/gnu/store/...
--8<---------------cut here---------------end--------------->8---

I thought it should be:

--8<---------------cut here---------------start------------->8---
The following package will be upgraded:
   guile-1.8.8	out	/gnu/store/...
--8<---------------cut here---------------end--------------->8---

Actually that's why (to avoid possible confusion) I initially suggested:

--8<---------------cut here---------------start------------->8---
The following package will be upgraded (removed):
   guile-1.8.8	out	/gnu/store/...

The following package will be installed:
   guile-2.0.9	out	/gnu/store/...
--8<---------------cut here---------------end--------------->8---

--
Alex

Re: [PATCH] profiles: Report about upgrades.

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> But is it correct to report it like that?  I mean if a user has
> “guile-1.8.8” and installs “guile-2.0.9” then (with your variant) he
> gets:
>
> The following package will be upgraded:
>    guile-2.0.9	out	/gnu/store/...
>
> I thought it should be:
>
> The following package will be upgraded:
>    guile-1.8.8	out	/gnu/store/...
>
> Actually that's why (to avoid possible confusion) I initially suggested:
>
> The following package will be upgraded (removed):
>    guile-1.8.8	out	/gnu/store/...
>
> The following package will be installed:
>    guile-2.0.9	out	/gnu/store/...

Oh I see, I had misunderstood the intent.

Well, I don’t know what’s best.  I’m happy with the way things are now,
though I can see why it could be confusing.

Perhaps even a different format, like, say:

  The following package will be upgraded:
    guile 1.8.8 → 2.0.9        out     /gnu/store/...

Thoughts?

Ludo’.

Re: [PATCH] profiles: Report about upgrades.

[Jason Self]

[-- Attachment #1: Type: text/plain, Size: 208 bytes --]

Ludovic Courtès:
> Perhaps even a different format, like, say:
>
>   The following package will be upgraded:
>     guile 1.8.8 → 2.0.9        out     /gnu/store/...
>
> Thoughts?

I like that. :)

Re: [PATCH] profiles: Report about upgrades.

[Alex Kost]

Ludovic Courtès (2014-08-31 23:57 +0400) wrote:

[...]

> Perhaps even a different format, like, say:
>
>   The following package will be upgraded:
>     guile 1.8.8 → 2.0.9        out     /gnu/store/...
>
> Thoughts?

I think, it would be perfect!

Re: [PATCH] profiles: Report about upgrades.

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> Ludovic Courtès (2014-08-31 23:57 +0400) wrote:
>
> [...]
>
>> Perhaps even a different format, like, say:
>>
>>   The following package will be upgraded:
>>     guile 1.8.8 → 2.0.9        out     /gnu/store/...
>>
>> Thoughts?
>
> I think, it would be perfect!

Commit ef8993e does that.

While I was at it, I took the freedom to adjust the format of these
lines (commit 9a91476) so that the version number would always appear in
the second column, while the output name no longer has a separate
column:

--8<---------------cut here---------------start------------->8---
$ ./pre-inst-env guix package -r idutils -u libtasn1 -i emacs libgc libgc:debug -n -p foo
The following package would be removed:
   idutils	4.6	/gnu/store/40nbigb0bghwqvnchjdxsrafbzxmybh8-idutils-4.6

The following package would be upgraded:
   libtasn1	3.6 → 4.1	/gnu/store/52kdi2gmrl2ms92as0nsxbbkndqx07s4-libtasn1-4.1

The following packages would be installed:
   libgc:debug	7.4.0	/gnu/store/98r76vrmvv3fvg26n3dzq3i72l25whl1-libgc-7.4.0-debug
   libgc	7.4.0	/gnu/store/cqhp23ak0kaa4kv1jdvzbsrkw41krczh-libgc-7.4.0
   emacs	24.3	/gnu/store/majxvz4dxcw75m1ni2hph3kbws400y94-emacs-24.3
--8<---------------cut here---------------end--------------->8---

It’s still time to complain if you don’t like it.  :-)

Ludo’.

Re: [PATCH] profiles: Report about upgrades.

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> Great!  I really like it and that ‘right-arrow’ is cool :)

:-)

> There is just one thing (that shouldn't be mentioned):
>
> +        (lambda (key . args)
> +          ">")))))
>
> I would make it "->".  IMHO it is more "understandable" as such
> combination is used in output of some shell commands (like “mv -v” or
> “cp -v”).

Makes sense.  Done.

> But that's a terrible nitpick and it should be ignored.

There’s no such thing as terrible nitpicking.  :-)

Thanks,
Ludo’.

Re: [PATCH] Emacs interface for Guix

[Ludovic Courtès]

(Sorry for replying to messages in the wrong order.  :-))

Alex Kost <alezost@gmail.com> skribis:

> Ok, I'm attaching these patches.  But there are several issues there:
>
> - I fixed a typo in "tests/profiles.scm" (“profile” -> “profiles”) – Is
>   it ok to do this in that commit or should there be a separate commit?

No that’s OK.

> - I added a copyright line to the test file as well.  Is it ok?

Sure!

> - The main thing: look at ‘manifest-show-transaction’ – unlike
>   ‘show-what-to-remove/install’ it doesn't display an output path of a
>   package item, because a store should be used for that.  So is it
>   acceptable or should something be changed there?

I think it should be changed to display the same thing as before.  What
about adding just a ‘store’ parameter to ‘manifest-show-transaction’,
and then just use the same code as ‘show-what-to-remove/install’?

Other than that the two patches look good, so if you make that change,
we can go ahead.

>>> Also I think "guix.el" should check for freshness too, so
>>> ‘check-package-freshness’ should probably be exported.
>>
>> Yes, probably in the (gnu packages) module?
>
> Probably, but I think I'm not competent to decide :)

Well, take it as a suggestion then.  :-)

Thanks,
Ludo’.

Re: [PATCH] Emacs interface for Guix

[Alex Kost]

[-- Attachment #1: Type: text/plain, Size: 1706 bytes --]

Ludovic Courtès (2014-08-16 16:24 +0400) wrote:

> (Sorry for replying to messages in the wrong order.  :-))

Sorry, I had sent old patches before you sent this message :)  Ignore
my previous message (with the subject "[PATCH] manifest-transaction")
please.

> Alex Kost <alezost@gmail.com> skribis:
>
>> Ok, I'm attaching these patches.  But there are several issues there:
>>
>> - I fixed a typo in "tests/profiles.scm" (“profile” -> “profiles”) – Is
>>   it ok to do this in that commit or should there be a separate commit?
>
> No that’s OK.
>
>> - I added a copyright line to the test file as well.  Is it ok?
>
> Sure!
>
>> - The main thing: look at ‘manifest-show-transaction’ – unlike
>>   ‘show-what-to-remove/install’ it doesn't display an output path of a
>>   package item, because a store should be used for that.  So is it
>>   acceptable or should something be changed there?
>
> I think it should be changed to display the same thing as before.  What
> about adding just a ‘store’ parameter to ‘manifest-show-transaction’,
> and then just use the same code as ‘show-what-to-remove/install’?
>
> Other than that the two patches look good, so if you make that change,
> we can go ahead.

I've made the change (the ‘store’ argument is added now).  The patches
are attached.

>>>> Also I think "guix.el" should check for freshness too, so
>>>> ‘check-package-freshness’ should probably be exported.
>>>
>>> Yes, probably in the (gnu packages) module?
>>
>> Probably, but I think I'm not competent to decide :)
>
> Well, take it as a suggestion then.  :-)

OK, I'll send a patch for that later.  Thanks.


[-- Attachment #2: 0001-profiles-Add-manifest-transaction.patch --]
[-- Type: text/x-patch, Size: 6631 bytes --]

From 7641752189cfc4ad3c85a042ea9eeea2b39435b4 Mon Sep 17 00:00:00 2001
From: Alex Kost <alezost@gmail.com>
Date: Thu, 14 Aug 2014 00:03:53 +0400
Subject: [PATCH 1/2] profiles: Add 'manifest-transaction'.

* guix/profiles.scm (<manifest-transaction>): New record-type.
  (manifest-perform-transaction): New procedure.
  (manifest-show-transaction): New procedure.
* tests/profiles.scm ("manifest-perform-transaction"): New test.
---
 guix/profiles.scm  | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/profiles.scm | 22 +++++++++++++++-
 2 files changed, 96 insertions(+), 1 deletion(-)

diff --git a/guix/profiles.scm b/guix/profiles.scm
index e921566..2398b89 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013, 2014 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2013 Nikita Karetnikov <nikita@karetnikov.org>
+;;; Copyright © 2014 Alex Kost <alezost@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -18,6 +19,7 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (guix profiles)
+  #:use-module (guix ui)
   #:use-module (guix utils)
   #:use-module (guix records)
   #:use-module (guix derivations)
@@ -51,6 +53,13 @@
             manifest-installed?
             manifest-matching-entries
 
+            manifest-transaction
+            manifest-transaction?
+            manifest-transaction-install
+            manifest-transaction-remove
+            manifest-perform-transaction
+            manifest-show-transaction
+
             profile-manifest
             package->manifest-entry
             profile-derivation
@@ -244,6 +253,72 @@ Remove MANIFEST entries that have the same name and output as ENTRIES."
 
 \f
 ;;;
+;;; Manifest transactions.
+;;;
+
+(define-record-type* <manifest-transaction> manifest-transaction
+  make-manifest-transaction
+  manifest-transaction?
+  (install manifest-transaction-install ; list of <manifest-entry>
+           (default '()))
+  (remove  manifest-transaction-remove  ; list of <manifest-pattern>
+           (default '())))
+
+(define (manifest-perform-transaction manifest transaction)
+  "Perform TRANSACTION on MANIFEST and return new manifest."
+  (let ((install (manifest-transaction-install transaction))
+        (remove  (manifest-transaction-remove transaction)))
+    (manifest-add (manifest-remove manifest remove)
+                  install)))
+
+(define* (manifest-show-transaction manifest transaction store
+                                    #:key dry-run?)
+  "Display what will/would be installed/removed from MANIFEST by TRANSACTION."
+  ;; TODO: Report upgrades more clearly.
+  (let ((install (manifest-transaction-install transaction))
+        (remove  (manifest-matching-entries
+                  manifest (manifest-transaction-remove transaction))))
+    (match remove
+      ((($ <manifest-entry> name version output path _) ..1)
+       (let ((len    (length name))
+             (remove (map (cut format #f "   ~a-~a\t~a\t~a" <> <> <> <>)
+                          name version output path)))
+         (if dry-run?
+             (format (current-error-port)
+                     (N_ "The following package would be removed:~%~{~a~%~}~%"
+                         "The following packages would be removed:~%~{~a~%~}~%"
+                         len)
+                     remove)
+             (format (current-error-port)
+                     (N_ "The following package will be removed:~%~{~a~%~}~%"
+                         "The following packages will be removed:~%~{~a~%~}~%"
+                         len)
+                     remove))))
+      (_ #f))
+    (match install
+      ((($ <manifest-entry> name version output item _) ..1)
+       (let ((len     (length name))
+             (install (map (lambda (name version output item)
+                             (format #f "   ~a-~a\t~a\t~a" name version output
+                                     (if (package? item)
+                                         (package-output store item output)
+                                         item)))
+                           name version output item)))
+         (if dry-run?
+             (format (current-error-port)
+                     (N_ "The following package would be installed:~%~{~a~%~}~%"
+                         "The following packages would be installed:~%~{~a~%~}~%"
+                         len)
+                     install)
+             (format (current-error-port)
+                     (N_ "The following package will be installed:~%~{~a~%~}~%"
+                         "The following packages will be installed:~%~{~a~%~}~%"
+                         len)
+                     install))))
+      (_ #f))))
+
+\f
+;;;
 ;;; Profiles.
 ;;;
 
diff --git a/tests/profiles.scm b/tests/profiles.scm
index b2919d7..e1f1eef 100644
--- a/tests/profiles.scm
+++ b/tests/profiles.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013, 2014 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2014 Alex Kost <alezost@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -26,7 +27,7 @@
   #:use-module (ice-9 match)
   #:use-module (srfi srfi-64))
 
-;; Test the (guix profile) module.
+;; Test the (guix profiles) module.
 
 (define %store
   (open-connection))
@@ -122,6 +123,25 @@
            (_ #f))
          (equal? m3 m4))))
 
+(test-assert "manifest-perform-transaction"
+  (let* ((m0 (manifest (list guile-2.0.9 guile-2.0.9:debug)))
+         (t1 (manifest-transaction
+              (install (list guile-1.8.8))
+              (remove (list (manifest-pattern (name "guile")
+                                              (output "debug"))))))
+         (t2 (manifest-transaction
+              (remove (list (manifest-pattern (name "guile")
+                                              (version "2.0.9")
+                                              (output #f))))))
+         (m1 (manifest-perform-transaction m0 t1))
+         (m2 (manifest-perform-transaction m1 t2))
+         (m3 (manifest-perform-transaction m0 t2)))
+    (and (match (manifest-entries m1)
+           ((($ <manifest-entry> "guile" "1.8.8" "out")) #t)
+           (_ #f))
+         (equal? m1 m2)
+         (null? (manifest-entries m3)))))
+
 (test-assert "profile-derivation"
   (run-with-store %store
     (mlet* %store-monad
-- 
2.0.3


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-guix-package-Use-manifest-transaction.patch --]
[-- Type: text/x-patch, Size: 4748 bytes --]

From 9bc3426a4550fe7e28a4c9ff807e3650f0ab1b92 Mon Sep 17 00:00:00 2001
From: Alex Kost <alezost@gmail.com>
Date: Thu, 14 Aug 2014 00:15:48 +0400
Subject: [PATCH 2/2] guix package: Use 'manifest-transaction'.

* guix/scripts/package.scm (guix-package)[process-actions]: Use
  'manifest-transaction' instead of the equivalent code.
  (show-what-to-remove/install): Remove.
---
 guix/scripts/package.scm | 63 +++++++++---------------------------------------
 1 file changed, 11 insertions(+), 52 deletions(-)

diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index 3bfef4f..e17ae18 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -184,49 +184,6 @@ DURATION-RELATION with the current time."
          filter-by-duration)
         (else #f)))
 
-(define (show-what-to-remove/install remove install dry-run?)
-  "Given the manifest entries listed in REMOVE and INSTALL, display the
-packages that will/would be installed and removed."
-  ;; TODO: Report upgrades more clearly.
-  (match remove
-    ((($ <manifest-entry> name version output path _) ..1)
-     (let ((len    (length name))
-           (remove (map (cut format #f "   ~a-~a\t~a\t~a" <> <> <> <>)
-                        name version output path)))
-       (if dry-run?
-           (format (current-error-port)
-                   (N_ "The following package would be removed:~%~{~a~%~}~%"
-                       "The following packages would be removed:~%~{~a~%~}~%"
-                       len)
-                   remove)
-           (format (current-error-port)
-                   (N_ "The following package will be removed:~%~{~a~%~}~%"
-                       "The following packages will be removed:~%~{~a~%~}~%"
-                       len)
-                   remove))))
-    (_ #f))
-  (match install
-    ((($ <manifest-entry> name version output item _) ..1)
-     (let ((len     (length name))
-           (install (map (lambda (name version output item)
-                           (format #f "   ~a-~a\t~a\t~a" name version output
-                                   (if (package? item)
-                                       (package-output (%store) item output)
-                                       item)))
-                         name version output item)))
-       (if dry-run?
-           (format (current-error-port)
-                   (N_ "The following package would be installed:~%~{~a~%~}~%"
-                       "The following packages would be installed:~%~{~a~%~}~%"
-                       len)
-                   install)
-           (format (current-error-port)
-                   (N_ "The following package will be installed:~%~{~a~%~}~%"
-                       "The following packages will be installed:~%~{~a~%~}~%"
-                       len)
-                   install))))
-    (_ #f)))
-
 \f
 ;;;
 ;;; Package specifications.
@@ -863,21 +820,23 @@ more information.~%"))
              (_ #f))
             opts))
           (else
-           (let* ((manifest (profile-manifest profile))
-                  (install  (options->installable opts manifest))
-                  (remove   (options->removable opts manifest))
-                  (new      (manifest-add (manifest-remove manifest remove)
-                                          install)))
+           (let* ((manifest    (profile-manifest profile))
+                  (install     (options->installable opts manifest))
+                  (remove      (options->removable opts manifest))
+                  (transaction (manifest-transaction (install install)
+                                                     (remove remove)))
+                  (new         (manifest-perform-transaction
+                                manifest transaction)))
 
              (when (equal? profile %current-profile)
                (ensure-default-profile))
 
              (unless (and (null? install) (null? remove))
                (let* ((prof-drv (run-with-store (%store)
-                                  (profile-derivation new)))
-                      (prof     (derivation->output-path prof-drv))
-                      (remove   (manifest-matching-entries manifest remove)))
-                 (show-what-to-remove/install remove install dry-run?)
+                                                (profile-derivation new)))
+                      (prof     (derivation->output-path prof-drv)))
+                 (manifest-show-transaction manifest transaction (%store)
+                                            #:dry-run? dry-run?)
                  (show-what-to-build (%store) (list prof-drv)
                                      #:use-substitutes?
                                      (assoc-ref opts 'substitutes?)
-- 
2.0.3


[-- Attachment #4: Type: text/plain, Size: 9 bytes --]


--
Alex

Re: [PATCH] Emacs interface for Guix

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> From 7641752189cfc4ad3c85a042ea9eeea2b39435b4 Mon Sep 17 00:00:00 2001
> From: Alex Kost <alezost@gmail.com>
> Date: Thu, 14 Aug 2014 00:03:53 +0400
> Subject: [PATCH 1/2] profiles: Add 'manifest-transaction'.
>
> * guix/profiles.scm (<manifest-transaction>): New record-type.
>   (manifest-perform-transaction): New procedure.
>   (manifest-show-transaction): New procedure.
> * tests/profiles.scm ("manifest-perform-transaction"): New test.

Applied with two minor changes: use (ice-9 format), as reported by
-Wformat, and move the ‘store’ argument first for consistency.

> From 9bc3426a4550fe7e28a4c9ff807e3650f0ab1b92 Mon Sep 17 00:00:00 2001
> From: Alex Kost <alezost@gmail.com>
> Date: Thu, 14 Aug 2014 00:15:48 +0400
> Subject: [PATCH 2/2] guix package: Use 'manifest-transaction'.
>
> * guix/scripts/package.scm (guix-package)[process-actions]: Use
>   'manifest-transaction' instead of the equivalent code.
>   (show-what-to-remove/install): Remove.

Applied, with the store argument first.

Thanks!

Ludo’.

Re: Emacs interface for Guix

[Alex Kost]

Ludovic Courtès (2014-08-20 01:00 +0400) wrote:

> Alex Kost <alezost@gmail.com> skribis:
>
>> From 7641752189cfc4ad3c85a042ea9eeea2b39435b4 Mon Sep 17 00:00:00 2001
>> From: Alex Kost <alezost@gmail.com>
>> Date: Thu, 14 Aug 2014 00:03:53 +0400
>> Subject: [PATCH 1/2] profiles: Add 'manifest-transaction'.
>>
>> * guix/profiles.scm (<manifest-transaction>): New record-type.
>>   (manifest-perform-transaction): New procedure.
>>   (manifest-show-transaction): New procedure.
>> * tests/profiles.scm ("manifest-perform-transaction"): New test.
>
> Applied with two minor changes: use (ice-9 format), as reported by
> -Wformat, and move the ‘store’ argument first for consistency.
>
>> From 9bc3426a4550fe7e28a4c9ff807e3650f0ab1b92 Mon Sep 17 00:00:00 2001
>> From: Alex Kost <alezost@gmail.com>
>> Date: Thu, 14 Aug 2014 00:15:48 +0400
>> Subject: [PATCH 2/2] guix package: Use 'manifest-transaction'.
>>
>> * guix/scripts/package.scm (guix-package)[process-actions]: Use
>>   'manifest-transaction' instead of the equivalent code.
>>   (show-what-to-remove/install): Remove.
>
> Applied, with the store argument first.

Thanks, now (with the latest “guix pull”), installing/upgrading/removing
should work in "guix.el".  If you (or someone else) wish to try it, you
may use:

  (setq guix-dry-run t)

(It has the same meaning as “--dry-run” option).

Also I would like to add support for deleting generations (to
"guix.el"), so I think it would be good to export ‘delete-generation’
from "scripts/package.scm".  WDYT?

--
Alex

Re: Emacs interface for Guix

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> Thanks, now (with the latest “guix pull”), installing/upgrading/removing
> should work in "guix.el".  If you (or someone else) wish to try it, you
> may use:
>
>   (setq guix-dry-run t)
>
> (It has the same meaning as “--dry-run” option).

I gave it a try, but AFAICS, when the REPL is started as “internal”,
guix-main.scm isn’t loaded, and thus “Install” fails:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> (process-package-actions #:install '((58935712 "out")) #:upgrade '() #:remove '() #:use-substitutes? #t #:dry-run? #f)
;;; <stdin>:11:0: warning: possibly unbound variable `process-package-actions'
<unnamed port>:11:0: In procedure #<procedure 42ba0c0 at <current input>:11:0 ()>:
<unnamed port>:11:0: In procedure module-lookup: Unbound variable: process-package-actions
--8<---------------cut here---------------end--------------->8---

Did I miss something?

Besides, I like M-x guix-generations, pretty cool.  :-)

> Also I would like to add support for deleting generations (to
> "guix.el"), so I think it would be good to export ‘delete-generation’
> from "scripts/package.scm".  WDYT?

Yes, that makes sense, one could use it from the *Guix Generation List*
buffer.

Regarding package installation/removal/upgrade, I think it would be
great UI-wise to support transactions that perform multiple operations.

I was initially thinking of something similar to what package.el does:
marks packages from installation/removal, and then hit ‘x’ to execute
the transaction.  So the current “Install” and “Delete” buttons could be
changed to just mark things for installation/removal.

WDYT?

Thanks,
Ludo’.

Re: Emacs interface for Guix

[Alex Kost]

[-- Attachment #1: Type: text/plain, Size: 1204 bytes --]

Ludovic Courtès (2014-08-22 12:56 +0400) wrote:

> Alex Kost <alezost@gmail.com> skribis:
>
>> Thanks, now (with the latest “guix pull”), installing/upgrading/removing
>> should work in "guix.el".  If you (or someone else) wish to try it, you
>> may use:
>>
>>   (setq guix-dry-run t)
>>
>> (It has the same meaning as “--dry-run” option).
>
> I gave it a try, but AFAICS, when the REPL is started as “internal”,
> guix-main.scm isn’t loaded, and thus “Install” fails:
>
> scheme@(guile-user)> (process-package-actions #:install '((58935712 "out")) #:upgrade '() #:remove '() #:use-substitutes? #t #:dry-run? #f)
> ;;; <stdin>:11:0: warning: possibly unbound variable `process-package-actions'
> <unnamed port>:11:0: In procedure #<procedure 42ba0c0 at <current input>:11:0 ()>:
> <unnamed port>:11:0: In procedure module-lookup: Unbound variable: process-package-actions
>
> Did I miss something?

I don't understand why you get this, it works for me and I can't
reproduce it with "emacs -Q".  Does the following recipe works for you?:

1. emacs -Q

2. Evaluate the following code (for example, paste it into *scratch*
   buffer and "M-x eval-buffer"):


[-- Attachment #2: test.el --]
[-- Type: application/emacs-lisp, Size: 467 bytes --]

[-- Attachment #3: Type: text/plain, Size: 231 bytes --]


3. M-x guix-all-available-packages

4. RET on any package that is not installed and press "Install" button
   in “*Guix Package Info*” buffer.  For me “*Guix REPL*” is shown and
   the following output is printed:


[-- Attachment #4: Type: text/plain, Size: 792 bytes --]

scheme@(guile-user)> (process-package-actions #:install '((151687520 "out")) #:upgrade '() #:remove '() #:use-substitutes? #t #:dry-run? #t)
The process begins ...
The following package would be installed:
   a2ps-4.14	out	/gnu/store/1akh02xh4z7i2idfnxbd28zzm87ghav1-a2ps-4.14

substitute-binary: ;;; note: source file /usr/share/guile/site/2.0/guix/config.scm
substitute-binary: ;;;       newer than compiled /usr/share/guile/site/2.0/guix/config.go
The following derivation would be built:
   /gnu/store/868vmg9s02jf4pxm3nvfmk168qbx44ld-profile.drv
The following files would be downloaded:
   /gnu/store/1akh02xh4z7i2idfnxbd28zzm87ghav1-a2ps-4.14
   /gnu/store/43bg01fshdm6hnacb9b8mha681cll0nw-coreutils-8.22
   /gnu/store/sv8ijfgc0d4ll8wzlxa6s2rkcvcjgrwx-perl-5.16.1
scheme@(guile-user)> 

[-- Attachment #5: Type: text/plain, Size: 1487 bytes --]


I tried with emacs 24.3.1 and the latest Geiser (from
<https://github.com/jaor/geiser>).

> Besides, I like M-x guix-generations, pretty cool.  :-)
>
>> Also I would like to add support for deleting generations (to
>> "guix.el"), so I think it would be good to export ‘delete-generation’
>> from "scripts/package.scm".  WDYT?
>
> Yes, that makes sense, one could use it from the *Guix Generation List*
> buffer.

Yes, that's what I had in mind: mark generations with "d" and execute a
deletion operation with "x".

> Regarding package installation/removal/upgrade, I think it would be
> great UI-wise to support transactions that perform multiple operations.
>
> I was initially thinking of something similar to what package.el does:
> marks packages from installation/removal, and then hit ‘x’ to execute
> the transaction.  So the current “Install” and “Delete” buttons could be
> changed to just mark things for installation/removal.

It is already available in a “*Guix Package List*” buffer: you can mark
packages with "d"/"i"/"^" ("u" is for unmarking) and execute an
operation with "x".  As for me, I prefer to keep buttons in a “*Guix
Package Info*” buffer as they are now.  It's just an alternative way to
install/delete a package: if you want to make a transaction of several
actions – use a "list" buffer, if you just want to install some package,
you can do it by pressing a button in "info" buffer.

--
Alex Kost

Re: Emacs interface for Guix

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> Ludovic Courtès (2014-08-22 12:56 +0400) wrote:
>
>> Alex Kost <alezost@gmail.com> skribis:
>>
>>> Thanks, now (with the latest “guix pull”), installing/upgrading/removing
>>> should work in "guix.el".  If you (or someone else) wish to try it, you
>>> may use:
>>>
>>>   (setq guix-dry-run t)
>>>
>>> (It has the same meaning as “--dry-run” option).
>>
>> I gave it a try, but AFAICS, when the REPL is started as “internal”,
>> guix-main.scm isn’t loaded, and thus “Install” fails:
>>
>> scheme@(guile-user)> (process-package-actions #:install '((58935712 "out")) #:upgrade '() #:remove '() #:use-substitutes? #t #:dry-run? #f)
>> ;;; <stdin>:11:0: warning: possibly unbound variable `process-package-actions'
>> <unnamed port>:11:0: In procedure #<procedure 42ba0c0 at <current input>:11:0 ()>:
>> <unnamed port>:11:0: In procedure module-lookup: Unbound variable: process-package-actions
>>
>> Did I miss something?
>
> I don't understand why you get this, it works for me and I can't
> reproduce it with "emacs -Q".  Does the following recipe works for you?:

I can no longer reproduce the issue, but my ~/.config/guix/latest was
pointing to an older version, so I think this may have led to a silent
failure about unbound variables, which then prevented guix-helper.scm to
be successfully loaded.

Thanks,
Ludo’.

[PATCH] guix package: Export generation procedures.

[Alex Kost]

[-- Attachment #1: Type: text/plain, Size: 1523 bytes --]

Ludovic Courtès (2014-08-22 12:56 +0400) wrote:

> Alex Kost <alezost@gmail.com> skribis:
>
>> Also I would like to add support for deleting generations (to
>> "guix.el"), so I think it would be good to export ‘delete-generation’
>> from "scripts/package.scm".  WDYT?
>
> Yes, that makes sense, one could use it from the *Guix Generation List*
> buffer.

What about the attached patch?  Some comments and questions:

- I added 'store' argument to the exported procedures, however it is
  used only in one particular case: when we need to create an empty
  profile (i.e. to call ‘link-to-empty-profile’).  Is there a way to
  avoid using 'store' argument there or is it fine to leave it like
  this?

- I actually need only ‘delete-generations’ procedure for Emacs UI, but
  I think other procedures are also worth to be exported or not?

- Perhaps there is a better place for those functions than
  (guix scripts package)?

- (Not related to this patch, but still …)  Currently with “roll-back”,
  we can only switch to the previous generation.  What about adding a
  possibility to switch to any generation?  So that we could use
  something like this:

    guix package --switch-generation=7

  Also such functionality can be added to Emacs UI: for example pressing
  "C" on a generation in *Guix Generation List* will make this
  generation the current one.

  So ‘roll-back’ procedure may become a special case of the
  ‘switch-generation’ one.  WDYT?


[-- Attachment #2: 0001-guix-package-Export-generation-procedures.patch --]
[-- Type: text/x-diff, Size: 7406 bytes --]

From c50d1674d3be699198afb649a2a9932ca44c89bc Mon Sep 17 00:00:00 2001
From: Alex Kost <alezost@gmail.com>
Date: Sat, 4 Oct 2014 20:45:35 +0400
Subject: [PATCH] guix package: Export generation procedures.

* guix/scripts/package.scm: Export 'roll-back', 'delete-generation',
  'delete-generations'.
  (link-to-empty-profile, roll-back): Add 'store' argument.
  (delete-generations): New procedure.
  (guix-package): Adjust accordingly.
  [delete-generation]: Move to the top level.  Add 'store' and 'profile'
  arguments.
  [display-and-delete]: Move to 'delete-generation'.
---
 guix/scripts/package.scm | 75 +++++++++++++++++++++++++++---------------------
 1 file changed, 43 insertions(+), 32 deletions(-)

diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index 7cd9516..fc9c37b 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2012, 2013, 2014 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2013 Nikita Karetnikov <nikita@karetnikov.org>
 ;;; Copyright © 2013 Mark H Weaver <mhw@netris.org>
+;;; Copyright © 2014 Alex Kost <alezost@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -43,6 +44,9 @@
   #:use-module (gnu packages guile)
   #:use-module ((gnu packages bootstrap) #:select (%bootstrap-guile))
   #:export (specification->package+output
+            roll-back
+            delete-generation
+            delete-generations
             guix-package))
 
 (define %store
@@ -80,12 +84,12 @@ return PROFILE unchanged.  The goal is to treat '-p ~/.guix-profile' as if
       %current-profile
       profile))
 
-(define (link-to-empty-profile generation)
+(define (link-to-empty-profile store generation)
   "Link GENERATION, a string, to the empty profile."
-  (let* ((drv  (run-with-store (%store)
+  (let* ((drv  (run-with-store store
                  (profile-derivation (manifest '()))))
          (prof (derivation->output-path drv "out")))
-    (when (not (build-derivations (%store) (list drv)))
+    (when (not (build-derivations store (list drv)))
           (leave (_ "failed to build the empty profile~%")))
 
     (switch-symlinks generation prof)))
@@ -99,7 +103,7 @@ return PROFILE unchanged.  The goal is to treat '-p ~/.guix-profile' as if
             number previous-number)
     (switch-symlinks profile previous-generation)))
 
-(define (roll-back profile)
+(define (roll-back store profile)
   "Roll back to the previous generation of PROFILE."
   (let* ((number              (generation-number profile))
          (previous-number     (previous-generation-number profile number))
@@ -112,11 +116,39 @@ return PROFILE unchanged.  The goal is to treat '-p ~/.guix-profile' as if
                    (_ "nothing to do: already at the empty profile~%")))
           ((or (zero? previous-number)                  ; going to emptiness
                (not (file-exists? previous-generation)))
-           (link-to-empty-profile previous-generation)
+           (link-to-empty-profile store previous-generation)
            (switch-to-previous-generation profile))
           (else
            (switch-to-previous-generation profile)))))  ; anything else
 
+(define (delete-generation store profile number)
+  "Delete generation with NUMBER from PROFILE."
+  (define (display-and-delete)
+    (let ((generation (generation-file-name profile number)))
+      (format #t (_ "deleting ~a~%") generation)
+      (delete-file generation)))
+
+  (let* ((current-number      (generation-number profile))
+         (previous-number     (previous-generation-number profile number))
+         (previous-generation (generation-file-name profile previous-number)))
+    (cond ((zero? number))              ; do not delete generation 0
+          ((and (= number current-number)
+                (not (file-exists? previous-generation)))
+           (link-to-empty-profile store previous-generation)
+           (switch-to-previous-generation profile)
+           (display-and-delete))
+          ((= number current-number)
+           (roll-back store profile)
+           (display-and-delete))
+          (else
+           (display-and-delete)))))
+
+(define (delete-generations store profile generations)
+  "Delete GENERATIONS from PROFILE.
+GENERATIONS is a list of generation numbers."
+  (for-each (cut delete-generation store profile <>)
+            generations))
+
 (define* (matching-generations str #:optional (profile %current-profile)
                                #:key (duration-relation <=))
   "Return the list of available generations matching a pattern in STR.  See
@@ -680,32 +712,10 @@ more information.~%"))
     (define current-generation-number
       (generation-number profile))
 
-    (define (display-and-delete number)
-      (let ((generation (generation-file-name profile number)))
-        (unless (zero? number)
-          (format #t (_ "deleting ~a~%") generation)
-          (delete-file generation))))
-
-    (define (delete-generation number)
-      (let* ((previous-number (previous-generation-number profile number))
-             (previous-generation
-              (generation-file-name profile previous-number)))
-        (cond ((zero? number))  ; do not delete generation 0
-              ((and (= number current-generation-number)
-                    (not (file-exists? previous-generation)))
-               (link-to-empty-profile previous-generation)
-               (switch-to-previous-generation profile)
-               (display-and-delete number))
-              ((= number current-generation-number)
-               (roll-back profile)
-               (display-and-delete number))
-              (else
-               (display-and-delete number)))))
-
     ;; First roll back if asked to.
     (cond ((and (assoc-ref opts 'roll-back?) (not dry-run?))
            (begin
-             (roll-back profile)
+             (roll-back (%store) profile)
              (process-actions (alist-delete 'roll-back? opts))))
           ((and (assoc-ref opts 'delete-generations)
                 (not dry-run?))
@@ -716,9 +726,10 @@ more information.~%"))
                      (leave (_ "profile '~a' does not exist~%")
                             profile))
                     ((string-null? pattern)
-                     (for-each display-and-delete
-                               (delete current-generation-number
-                                       (profile-generations profile))))
+                     (delete-generations
+                      (%store) profile
+                      (delete current-generation-number
+                              (profile-generations profile))))
                     ;; Do not delete the zeroth generation.
                     ((equal? 0 (string->number pattern))
                      (exit 0))
@@ -731,7 +742,7 @@ more information.~%"))
                      (lambda (numbers)
                        (if (null-list? numbers)
                            (exit 1)
-                           (for-each delete-generation numbers))))
+                           (delete-generations (%store) profile numbers))))
                     (else
                      (leave (_ "invalid syntax: ~a~%")
                             pattern)))
-- 
2.1.2

Re: [PATCH] guix package: Export generation procedures.

[Ludovic Courtès]

Hi!

Alex Kost <alezost@gmail.com> skribis:

> What about the attached patch?  Some comments and questions:
>
> - I added 'store' argument to the exported procedures, however it is
>   used only in one particular case: when we need to create an empty
>   profile (i.e. to call ‘link-to-empty-profile’).  Is there a way to
>   avoid using 'store' argument there or is it fine to leave it like
>   this?

For now it’s fine to leave it like this, with the ‘store’ argument.
Eventually it should be changed to use the monadic style, though.

> - I actually need only ‘delete-generations’ procedure for Emacs UI, but
>   I think other procedures are also worth to be exported or not?

Yes, sure.

> - Perhaps there is a better place for those functions than
>   (guix scripts package)?

Yes, (guix profiles) would be a better place IMO.

> - (Not related to this patch, but still …)  Currently with “roll-back”,
>   we can only switch to the previous generation.  What about adding a
>   possibility to switch to any generation?  So that we could use
>   something like this:
>
>     guix package --switch-generation=7
>
>   Also such functionality can be added to Emacs UI: for example pressing
>   "C" on a generation in *Guix Generation List* will make this
>   generation the current one.
>
>   So ‘roll-back’ procedure may become a special case of the
>   ‘switch-generation’ one.  WDYT?

I think it’s a good idea!  (I think it was suggested in earlier
discussions, but never implemented.)

> From c50d1674d3be699198afb649a2a9932ca44c89bc Mon Sep 17 00:00:00 2001
> From: Alex Kost <alezost@gmail.com>
> Date: Sat, 4 Oct 2014 20:45:35 +0400
> Subject: [PATCH] guix package: Export generation procedures.
>
> * guix/scripts/package.scm: Export 'roll-back', 'delete-generation',
>   'delete-generations'.
>   (link-to-empty-profile, roll-back): Add 'store' argument.
>   (delete-generations): New procedure.
>   (guix-package): Adjust accordingly.
>   [delete-generation]: Move to the top level.  Add 'store' and 'profile'
>   arguments.
>   [display-and-delete]: Move to 'delete-generation'.

OK to commit.

To sum up, I would imagine two followups to this:

  1. Move these procedures to (guix profiles).
  2. Convert them to monadic style.

WDYT?

Thanks,
Ludo’.

[PATCH] emacs: Add support for deleting generations.

[Alex Kost]

[-- Attachment #1: Type: text/plain, Size: 3116 bytes --]

Ludovic Courtès (2014-10-05 00:23 +0400) wrote:

> Hi!
>
> Alex Kost <alezost@gmail.com> skribis:
>
>> What about the attached patch?  Some comments and questions:
>>
>> - I added 'store' argument to the exported procedures, however it is
>>   used only in one particular case: when we need to create an empty
>>   profile (i.e. to call ‘link-to-empty-profile’).  Is there a way to
>>   avoid using 'store' argument there or is it fine to leave it like
>>   this?
>
> For now it’s fine to leave it like this, with the ‘store’ argument.
> Eventually it should be changed to use the monadic style, though.

I read the info manual about gexps and monads.  However I don't really
understand how to use monadic style here, sorry.

>> - I actually need only ‘delete-generations’ procedure for Emacs UI, but
>>   I think other procedures are also worth to be exported or not?
>
> Yes, sure.

Thanks.

>> - Perhaps there is a better place for those functions than
>>   (guix scripts package)?
>
> Yes, (guix profiles) would be a better place IMO.

OK.

>> - (Not related to this patch, but still …)  Currently with “roll-back”,
>>   we can only switch to the previous generation.  What about adding a
>>   possibility to switch to any generation?  So that we could use
>>   something like this:
>>
>>     guix package --switch-generation=7
>>
>>   Also such functionality can be added to Emacs UI: for example pressing
>>   "C" on a generation in *Guix Generation List* will make this
>>   generation the current one.
>>
>>   So ‘roll-back’ procedure may become a special case of the
>>   ‘switch-generation’ one.  WDYT?
>
> I think it’s a good idea!  (I think it was suggested in earlier
> discussions, but never implemented.)

Great, I think I can make it, but without monads :-(

>> From c50d1674d3be699198afb649a2a9932ca44c89bc Mon Sep 17 00:00:00 2001
>> From: Alex Kost <alezost@gmail.com>
>> Date: Sat, 4 Oct 2014 20:45:35 +0400
>> Subject: [PATCH] guix package: Export generation procedures.
>>
>> * guix/scripts/package.scm: Export 'roll-back', 'delete-generation',
>>   'delete-generations'.
>>   (link-to-empty-profile, roll-back): Add 'store' argument.
>>   (delete-generations): New procedure.
>>   (guix-package): Adjust accordingly.
>>   [delete-generation]: Move to the top level.  Add 'store' and 'profile'
>>   arguments.
>>   [display-and-delete]: Move to 'delete-generation'.
>
> OK to commit.
>
> To sum up, I would imagine two followups to this:
>
>   1. Move these procedures to (guix profiles).
>   2. Convert them to monadic style.
>
> WDYT?

I like the idea of using monads there, but as I said I'm too week (I
mean "month" (I mean "weak")) for writing that.

Also those followups would make my commit totally redundant, no?
Wouldn't it be better to make a commit for adding the monadic functions
to (guix profiles) directly?

However if you still allow me to push this commit, I think I can also
push the attached one with the changes for Emacs UI now (if it looks OK
for you).


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-emacs-Add-support-for-deleting-generations.patch --]
[-- Type: text/x-diff, Size: 4811 bytes --]

From c335cdf17a97d07cc3d4149fa7dc13882d16cc87 Mon Sep 17 00:00:00 2001
From: Alex Kost <alezost@gmail.com>
Date: Sun, 5 Oct 2014 12:31:23 +0400
Subject: [PATCH] emacs: Add support for deleting generations.

* doc/emacs.texi (emacs List buffer): Mention new key bindings.
* emacs/guix-base.el (guix-delete-generations): New procedure.
* emacs/guix-info.el (guix-generation-info-insert-number): Use it.
* emacs/guix-list.el (guix-generation-list-mark-delete,
  guix-generation-list-execute): New procedures.
* emacs/guix-main.scm (delete-generations*): New procedure.
---
 doc/emacs.texi      |  5 +++++
 emacs/guix-base.el  | 14 ++++++++++++++
 emacs/guix-info.el  |  6 ++++--
 emacs/guix-list.el  | 19 ++++++++++++++++++-
 emacs/guix-main.scm |  6 ++++++
 5 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/doc/emacs.texi b/doc/emacs.texi
index 3c5698f..4176f2e 100644
--- a/doc/emacs.texi
+++ b/doc/emacs.texi
@@ -205,6 +205,11 @@ List packages installed in the current generation.
 @item i
 Describe marked generations (display available information in a
 ``generation-info'' buffer).
+@item d
+Mark the current generation for deletion (with prefix, mark all
+generations).
+@item x
+Execute actions on the marked generations (i.e.@: delete generations).
 @end table
 
 @node emacs Info buffer
diff --git a/emacs/guix-base.el b/emacs/guix-base.el
index 8da7835..d31fb79 100644
--- a/emacs/guix-base.el
+++ b/emacs/guix-base.el
@@ -801,6 +801,20 @@ Return non-nil, if the operation should be continued; nil otherwise."
                  guix-operation-option-separator)))
   (force-mode-line-update))
 
+(defun guix-delete-generations (&rest generations)
+  "Delete GENERATIONS.
+Each element from GENERATIONS is a generation number."
+  (when (or (not guix-operation-confirm)
+              (y-or-n-p
+               (let ((count (length generations)))
+                 (if (> count 1)
+                     (format "Delete %d generations? " count)
+                   (format "Delete generation number %d? "
+                           (car generations))))))
+    (guix-eval-in-repl
+     (guix-make-guile-expression
+      'delete-generations* guix-current-profile generations))))
+
 (provide 'guix-base)
 
 ;;; guix-base.el ends here
diff --git a/emacs/guix-info.el b/emacs/guix-info.el
index d0a320f..d5226b1 100644
--- a/emacs/guix-info.el
+++ b/emacs/guix-info.el
@@ -627,8 +627,10 @@ ENTRY is an alist with package info."
   (guix-info-insert-indent)
   (guix-info-insert-action-button
    "Delete"
-   (lambda (btn) (error "Sorry, not implemented yet"))
-   "Delete this generation"))
+   (lambda (btn)
+     (guix-delete-generations (button-get btn 'number)))
+   "Delete this generation"
+   'number number))
 
 (provide 'guix-info)
 
diff --git a/emacs/guix-list.el b/emacs/guix-list.el
index 6a4cdfc..4b4b9c5 100644
--- a/emacs/guix-list.el
+++ b/emacs/guix-list.el
@@ -728,8 +728,9 @@ Also see `guix-package-info-type'."
 
 (let ((map guix-generation-list-mode-map))
   (define-key map (kbd "RET") 'guix-generation-list-show-packages)
+  (define-key map (kbd "x")   'guix-generation-list-execute)
   (define-key map (kbd "i")   'guix-list-describe)
-  (define-key map (kbd "d")   'guix-generation-list-mark-delete-simple))
+  (define-key map (kbd "d")   'guix-generation-list-mark-delete))
 
 (defun guix-generation-list-show-packages ()
   "List installed packages for the generation at point."
@@ -737,6 +738,22 @@ Also see `guix-package-info-type'."
   (guix-get-show-entries 'list guix-package-list-type 'generation
                          (guix-list-current-id)))
 
+(defun guix-generation-list-mark-delete (&optional arg)
+  "Mark the current generation for deletion and move to the next line.
+With ARG, mark all generations for deletion."
+  (interactive "P")
+  (if arg
+      (guix-list-mark-all 'delete)
+    (guix-list-mark 'delete t)))
+
+(defun guix-generation-list-execute ()
+  "Delete marked generations."
+  (interactive)
+  (let ((marked (guix-list-get-marked-id-list 'delete)))
+    (or marked
+        (user-error "No generations marked for deletion"))
+    (apply #'guix-delete-generations marked)))
+
 (provide 'guix-list)
 
 ;;; guix-list.el ends here
diff --git a/emacs/guix-main.scm b/emacs/guix-main.scm
index 026a9e9..24ca4d1 100644
--- a/emacs/guix-main.scm
+++ b/emacs/guix-main.scm
@@ -815,3 +815,9 @@ OUTPUTS is a list of package outputs (may be an empty list)."
                                   "~a packages in profile~%"
                                   count)
                            count)))))))))
+
+(define (delete-generations* profile generations)
+  "Delete GENERATIONS from PROFILE.
+GENERATIONS is a list of generation numbers."
+  (let ((store (open-connection)))
+    (delete-generations store profile generations)))
-- 
2.1.2

Re: [PATCH] emacs: Add support for deleting generations.

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> Ludovic Courtès (2014-10-05 00:23 +0400) wrote:

[...]

>> To sum up, I would imagine two followups to this:
>>
>>   1. Move these procedures to (guix profiles).
>>   2. Convert them to monadic style.
>>
>> WDYT?
>
> I like the idea of using monads there, but as I said I'm too week (I
> mean "month" (I mean "weak")) for writing that.

Heh.  :-)  No problem, that can come later.

> Also those followups would make my commit totally redundant, no?
> Wouldn't it be better to make a commit for adding the monadic functions
> to (guix profiles) directly?

I think it’s fine to make changes incrementally.

> However if you still allow me to push this commit,

Sure!

> I think I can also push the attached one with the changes for Emacs UI
> now (if it looks OK for you).
>
>
> From c335cdf17a97d07cc3d4149fa7dc13882d16cc87 Mon Sep 17 00:00:00 2001
> From: Alex Kost <alezost@gmail.com>
> Date: Sun, 5 Oct 2014 12:31:23 +0400
> Subject: [PATCH] emacs: Add support for deleting generations.
>
> * doc/emacs.texi (emacs List buffer): Mention new key bindings.
> * emacs/guix-base.el (guix-delete-generations): New procedure.
> * emacs/guix-info.el (guix-generation-info-insert-number): Use it.
> * emacs/guix-list.el (guix-generation-list-mark-delete,
>   guix-generation-list-execute): New procedures.
> * emacs/guix-main.scm (delete-generations*): New procedure.

Looks good!  Nitpicks:

> +@item x
> +Execute actions on the marked generations (i.e.@: delete generations).

I would make it:

  Execute actions on the marked generations---i.e., delete generations.

Or possibly a comma before “i.e.”.  Certainly a comma after it.

> --- a/emacs/guix-main.scm
> +++ b/emacs/guix-main.scm
> @@ -815,3 +815,9 @@ OUTPUTS is a list of package outputs (may be an empty list)."
>                                    "~a packages in profile~%"
>                                    count)
>                             count)))))))))
> +
> +(define (delete-generations* profile generations)
> +  "Delete GENERATIONS from PROFILE.
> +GENERATIONS is a list of generation numbers."
> +  (let ((store (open-connection)))
> +    (delete-generations store profile generations)))

Rather:

  (with-store store
    (delete-generations store profile generations))

That will take care of closing ‘store’ when the dynamic extent of the
body is left.

Thanks,
Ludo’.

Re: [PATCH] emacs: Add support for deleting generations.

[Alex Kost]

[-- Attachment #1: Type: text/plain, Size: 2204 bytes --]

Ludovic Courtès (2014-10-05 17:14 +0400) wrote:

> Alex Kost <alezost@gmail.com> skribis:
>
>> Ludovic Courtès (2014-10-05 00:23 +0400) wrote:
>
> [...]
>
>>> To sum up, I would imagine two followups to this:
>>>
>>>   1. Move these procedures to (guix profiles).
>>>   2. Convert them to monadic style.
>>>
>>> WDYT?
>>
>> I like the idea of using monads there, but as I said I'm too week (I
>> mean "month" (I mean "weak")) for writing that.
>
> Heh.  :-)  No problem, that can come later.
>
>> Also those followups would make my commit totally redundant, no?
>> Wouldn't it be better to make a commit for adding the monadic functions
>> to (guix profiles) directly?
>
> I think it’s fine to make changes incrementally.

OK.

>> However if you still allow me to push this commit,
>
> Sure!

Done, thanks.

[...]

>> +@item x
>> +Execute actions on the marked generations (i.e.@: delete generations).
>
> I would make it:
>
>   Execute actions on the marked generations---i.e., delete generations.
>
> Or possibly a comma before “i.e.”.  Certainly a comma after it.
>
>> --- a/emacs/guix-main.scm
>> +++ b/emacs/guix-main.scm
>> @@ -815,3 +815,9 @@ OUTPUTS is a list of package outputs (may be an empty list)."
>>                                    "~a packages in profile~%"
>>                                    count)
>>                             count)))))))))
>> +
>> +(define (delete-generations* profile generations)
>> +  "Delete GENERATIONS from PROFILE.
>> +GENERATIONS is a list of generation numbers."
>> +  (let ((store (open-connection)))
>> +    (delete-generations store profile generations)))
>
> Rather:
>
>   (with-store store
>     (delete-generations store profile generations))
>
> That will take care of closing ‘store’ when the dynamic extent of the
> body is left.

Thanks for the comments, the updated patch is attached.  I also fixed
another “i.e.” thing there and used ‘with-store’ in
‘process-package-actions’ procedure as well.  Is it OK to make these
changes in this commit (the ‘with-store’ change is small but many lines
were changed because of the new indentation)?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-emacs-Add-support-for-deleting-generations.patch --]
[-- Type: text/x-diff, Size: 7915 bytes --]

From 1334eb7272068dfd0b17d07e00c87ef0068d68a0 Mon Sep 17 00:00:00 2001
From: Alex Kost <alezost@gmail.com>
Date: Sun, 5 Oct 2014 12:31:23 +0400
Subject: [PATCH] emacs: Add support for deleting generations.

* doc/emacs.texi (emacs List buffer): Mention new key bindings.
* emacs/guix-base.el (guix-delete-generations): New procedure.
* emacs/guix-info.el (guix-generation-info-insert-number): Use it.
* emacs/guix-list.el (guix-generation-list-mark-delete,
  guix-generation-list-execute): New procedures.
* emacs/guix-main.scm (delete-generations*): New procedure.
---
 doc/emacs.texi      |  9 +++++++--
 emacs/guix-base.el  | 14 ++++++++++++++
 emacs/guix-info.el  |  6 ++++--
 emacs/guix-list.el  | 19 +++++++++++++++++-
 emacs/guix-main.scm | 56 +++++++++++++++++++++++++++++------------------------
 5 files changed, 74 insertions(+), 30 deletions(-)

diff --git a/doc/emacs.texi b/doc/emacs.texi
index 3c5698f..2e6b60e 100644
--- a/doc/emacs.texi
+++ b/doc/emacs.texi
@@ -105,8 +105,8 @@ many last generations.
 @end table
 
 By default commands for displaying packages display each output on a
-separate line.  If you prefer to see a list of packages (i.e.@: a list
-with a package per line), use the following setting:
+separate line.  If you prefer to see a list of packages---i.e., a list
+with a package per line, use the following setting:
 
 @example
 (setq guix-package-list-type 'package)
@@ -205,6 +205,11 @@ List packages installed in the current generation.
 @item i
 Describe marked generations (display available information in a
 ``generation-info'' buffer).
+@item d
+Mark the current generation for deletion (with prefix, mark all
+generations).
+@item x
+Execute actions on the marked generations---i.e., delete generations.
 @end table
 
 @node emacs Info buffer
diff --git a/emacs/guix-base.el b/emacs/guix-base.el
index 8da7835..d31fb79 100644
--- a/emacs/guix-base.el
+++ b/emacs/guix-base.el
@@ -801,6 +801,20 @@ Return non-nil, if the operation should be continued; nil otherwise."
                  guix-operation-option-separator)))
   (force-mode-line-update))
 
+(defun guix-delete-generations (&rest generations)
+  "Delete GENERATIONS.
+Each element from GENERATIONS is a generation number."
+  (when (or (not guix-operation-confirm)
+              (y-or-n-p
+               (let ((count (length generations)))
+                 (if (> count 1)
+                     (format "Delete %d generations? " count)
+                   (format "Delete generation number %d? "
+                           (car generations))))))
+    (guix-eval-in-repl
+     (guix-make-guile-expression
+      'delete-generations* guix-current-profile generations))))
+
 (provide 'guix-base)
 
 ;;; guix-base.el ends here
diff --git a/emacs/guix-info.el b/emacs/guix-info.el
index d0a320f..d5226b1 100644
--- a/emacs/guix-info.el
+++ b/emacs/guix-info.el
@@ -627,8 +627,10 @@ ENTRY is an alist with package info."
   (guix-info-insert-indent)
   (guix-info-insert-action-button
    "Delete"
-   (lambda (btn) (error "Sorry, not implemented yet"))
-   "Delete this generation"))
+   (lambda (btn)
+     (guix-delete-generations (button-get btn 'number)))
+   "Delete this generation"
+   'number number))
 
 (provide 'guix-info)
 
diff --git a/emacs/guix-list.el b/emacs/guix-list.el
index 6a4cdfc..4b4b9c5 100644
--- a/emacs/guix-list.el
+++ b/emacs/guix-list.el
@@ -728,8 +728,9 @@ Also see `guix-package-info-type'."
 
 (let ((map guix-generation-list-mode-map))
   (define-key map (kbd "RET") 'guix-generation-list-show-packages)
+  (define-key map (kbd "x")   'guix-generation-list-execute)
   (define-key map (kbd "i")   'guix-list-describe)
-  (define-key map (kbd "d")   'guix-generation-list-mark-delete-simple))
+  (define-key map (kbd "d")   'guix-generation-list-mark-delete))
 
 (defun guix-generation-list-show-packages ()
   "List installed packages for the generation at point."
@@ -737,6 +738,22 @@ Also see `guix-package-info-type'."
   (guix-get-show-entries 'list guix-package-list-type 'generation
                          (guix-list-current-id)))
 
+(defun guix-generation-list-mark-delete (&optional arg)
+  "Mark the current generation for deletion and move to the next line.
+With ARG, mark all generations for deletion."
+  (interactive "P")
+  (if arg
+      (guix-list-mark-all 'delete)
+    (guix-list-mark 'delete t)))
+
+(defun guix-generation-list-execute ()
+  "Delete marked generations."
+  (interactive)
+  (let ((marked (guix-list-get-marked-id-list 'delete)))
+    (or marked
+        (user-error "No generations marked for deletion"))
+    (apply #'guix-delete-generations marked)))
+
 (provide 'guix-list)
 
 ;;; guix-list.el ends here
diff --git a/emacs/guix-main.scm b/emacs/guix-main.scm
index 026a9e9..b85bb5c 100644
--- a/emacs/guix-main.scm
+++ b/emacs/guix-main.scm
@@ -790,28 +790,34 @@ OUTPUTS is a list of package outputs (may be an empty list)."
          (new-manifest (manifest-perform-transaction
                         manifest transaction)))
     (unless (and (null? install) (null? remove))
-      (let* ((store (open-connection))
-             (derivation (run-with-store
-                          store (profile-derivation new-manifest)))
-             (derivations (list derivation))
-             (new-profile (derivation->output-path derivation)))
-        (set-build-options store
-                           #:use-substitutes? use-substitutes?)
-        (manifest-show-transaction store manifest transaction
-                                   #:dry-run? dry-run?)
-        (show-what-to-build store derivations
-                            #:use-substitutes? use-substitutes?
-                            #:dry-run? dry-run?)
-        (unless dry-run?
-          (let ((name (generation-file-name
-                       profile
-                       (+ 1 (generation-number profile)))))
-            (and (build-derivations store derivations)
-                 (let* ((entries (manifest-entries new-manifest))
-                        (count   (length entries)))
-                   (switch-symlinks name new-profile)
-                   (switch-symlinks profile name)
-                   (format #t (N_ "~a package in profile~%"
-                                  "~a packages in profile~%"
-                                  count)
-                           count)))))))))
+      (with-store store
+        (let* ((derivation (run-with-store store
+                             (profile-derivation new-manifest)))
+               (derivations (list derivation))
+               (new-profile (derivation->output-path derivation)))
+          (set-build-options store
+                             #:use-substitutes? use-substitutes?)
+          (manifest-show-transaction store manifest transaction
+                                     #:dry-run? dry-run?)
+          (show-what-to-build store derivations
+                              #:use-substitutes? use-substitutes?
+                              #:dry-run? dry-run?)
+          (unless dry-run?
+            (let ((name (generation-file-name
+                         profile
+                         (+ 1 (generation-number profile)))))
+              (and (build-derivations store derivations)
+                   (let* ((entries (manifest-entries new-manifest))
+                          (count   (length entries)))
+                     (switch-symlinks name new-profile)
+                     (switch-symlinks profile name)
+                     (format #t (N_ "~a package in profile~%"
+                                    "~a packages in profile~%"
+                                    count)
+                             count))))))))))
+
+(define (delete-generations* profile generations)
+  "Delete GENERATIONS from PROFILE.
+GENERATIONS is a list of generation numbers."
+  (with-store store
+    (delete-generations store profile generations)))
-- 
2.1.2

Re: [PATCH] emacs: Add support for deleting generations.

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> Thanks for the comments, the updated patch is attached.  I also fixed
> another “i.e.” thing there and used ‘with-store’ in
> ‘process-package-actions’ procedure as well.  Is it OK to make these
> changes in this commit (the ‘with-store’ change is small but many lines
> were changed because of the new indentation)?

The ‘with-store’ change in ‘process-package-actions’ would be better in
a separate patch, because it fixes an unrelated file descriptor leak.

Other than that the patch looks good to me, thank you!

Ludo’.

Re: [PATCH] emacs: Add support for deleting generations.

[Alex Kost]

[-- Attachment #1: Type: text/plain, Size: 748 bytes --]

Ludovic Courtès (2014-10-05 23:20 +0400) wrote:

> Alex Kost <alezost@gmail.com> skribis:
>
>> Thanks for the comments, the updated patch is attached.  I also fixed
>> another “i.e.” thing there and used ‘with-store’ in
>> ‘process-package-actions’ procedure as well.  Is it OK to make these
>> changes in this commit (the ‘with-store’ change is small but many lines
>> were changed because of the new indentation)?
>
> The ‘with-store’ change in ‘process-package-actions’ would be better in
> a separate patch, because it fixes an unrelated file descriptor leak.
>
> Other than that the patch looks good to me, thank you!

Pushed, thanks.  Here is another patch for fixing ‘process-package-actions’.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-emacs-Use-with-store-in-process-package-actions.patch --]
[-- Type: text/x-diff, Size: 3522 bytes --]

From 6ef2a8b88007840ffa6648563356021770bbb6e6 Mon Sep 17 00:00:00 2001
From: Alex Kost <alezost@gmail.com>
Date: Sun, 5 Oct 2014 23:52:52 +0400
Subject: [PATCH] emacs: Use 'with-store' in 'process-package-actions'.

* emacs/guix-main.scm (process-package-actions): Use 'with-store'.
---
 emacs/guix-main.scm | 50 +++++++++++++++++++++++++-------------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/emacs/guix-main.scm b/emacs/guix-main.scm
index 7dbfa61..b85bb5c 100644
--- a/emacs/guix-main.scm
+++ b/emacs/guix-main.scm
@@ -790,31 +790,31 @@ OUTPUTS is a list of package outputs (may be an empty list)."
          (new-manifest (manifest-perform-transaction
                         manifest transaction)))
     (unless (and (null? install) (null? remove))
-      (let* ((store (open-connection))
-             (derivation (run-with-store
-                          store (profile-derivation new-manifest)))
-             (derivations (list derivation))
-             (new-profile (derivation->output-path derivation)))
-        (set-build-options store
-                           #:use-substitutes? use-substitutes?)
-        (manifest-show-transaction store manifest transaction
-                                   #:dry-run? dry-run?)
-        (show-what-to-build store derivations
-                            #:use-substitutes? use-substitutes?
-                            #:dry-run? dry-run?)
-        (unless dry-run?
-          (let ((name (generation-file-name
-                       profile
-                       (+ 1 (generation-number profile)))))
-            (and (build-derivations store derivations)
-                 (let* ((entries (manifest-entries new-manifest))
-                        (count   (length entries)))
-                   (switch-symlinks name new-profile)
-                   (switch-symlinks profile name)
-                   (format #t (N_ "~a package in profile~%"
-                                  "~a packages in profile~%"
-                                  count)
-                           count)))))))))
+      (with-store store
+        (let* ((derivation (run-with-store store
+                             (profile-derivation new-manifest)))
+               (derivations (list derivation))
+               (new-profile (derivation->output-path derivation)))
+          (set-build-options store
+                             #:use-substitutes? use-substitutes?)
+          (manifest-show-transaction store manifest transaction
+                                     #:dry-run? dry-run?)
+          (show-what-to-build store derivations
+                              #:use-substitutes? use-substitutes?
+                              #:dry-run? dry-run?)
+          (unless dry-run?
+            (let ((name (generation-file-name
+                         profile
+                         (+ 1 (generation-number profile)))))
+              (and (build-derivations store derivations)
+                   (let* ((entries (manifest-entries new-manifest))
+                          (count   (length entries)))
+                     (switch-symlinks name new-profile)
+                     (switch-symlinks profile name)
+                     (format #t (N_ "~a package in profile~%"
+                                    "~a packages in profile~%"
+                                    count)
+                             count))))))))))
 
 (define (delete-generations* profile generations)
   "Delete GENERATIONS from PROFILE.
-- 
2.1.2

Re: [PATCH] emacs: Add support for deleting generations.

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> Pushed, thanks.  Here is another patch for fixing ‘process-package-actions’.
>
>
> From 6ef2a8b88007840ffa6648563356021770bbb6e6 Mon Sep 17 00:00:00 2001
> From: Alex Kost <alezost@gmail.com>
> Date: Sun, 5 Oct 2014 23:52:52 +0400
> Subject: [PATCH] emacs: Use 'with-store' in 'process-package-actions'.
>
> * emacs/guix-main.scm (process-package-actions): Use 'with-store'.

LGTM!

Ludo’.

[PATCH] guix package: Add '--switch-generation' option.

[Alex Kost]

[-- Attachment #1: Type: text/plain, Size: 1682 bytes --]

Ludovic Courtès (2014-10-05 00:23 +0400) wrote:

> Alex Kost <alezost@gmail.com> skribis:>

[...]

>> - (Not related to this patch, but still …)  Currently with “roll-back”,
>>   we can only switch to the previous generation.  What about adding a
>>   possibility to switch to any generation?  So that we could use
>>   something like this:
>>
>>     guix package --switch-generation=7
>>
>>   Also such functionality can be added to Emacs UI: for example pressing
>>   "C" on a generation in *Guix Generation List* will make this
>>   generation the current one.
>>
>>   So ‘roll-back’ procedure may become a special case of the
>>   ‘switch-generation’ one.  WDYT?
>
> I think it’s a good idea!  (I think it was suggested in earlier
> discussions, but never implemented.)

Andreas Enge (2014-10-05 18:44 +0400) wrote:

[...]

> Actually, sometimes I would like to switch to the next generation ("-roll-
> forward", in a sense): I install something, go back with "--roll-back",
> and might like to go just one forward again.
>
> So how about the following:
> --switch-generation=1
> goes to generation 1;
> --switch-generation=-1
> goes 1 generation back;
> --switch-generation=+1
> goes one generation forward.

Thanks for the great idea!

————————————————————————————————————————————————————————————————
A patch is attached.  Some comments:

- ‘shitted-generation’ is not a very good name, I think.  Ideas?

- ‘previous-generation-number’ may use ‘shifted-generation’ now:


[-- Attachment #2: sample.scm --]
[-- Type: text/plain, Size: 389 bytes --]

(define* (previous-generation-number profile #:optional
                                     (number (generation-number profile)))
  "Return the number of the generation before generation NUMBER of
PROFILE, or 0 if none exists.  It could be NUMBER - 1, but it's not the
case when generations have been deleted (there are \"holes\")."
  (or (shifted-generation profile -1 number)
      0))

[-- Attachment #3: Type: text/plain, Size: 591 bytes --]


  Worth changing?

- Perhaps it would be better to make 2 commits (?): one for adding
  ‘shifted-generation’ and ‘switch-to-generation’ procedures to (guix
  profiles) and another is for adding the “--switch-generation” option
  itself.

- Also I made a couple of cosmetic changes in “guix/scripts/package.scm”:
  * ‘filter-map’ was replaced by 'for-each' because it was called only for
    side effects there;
  * ‘begin’ was removed from ‘cond’.
  I think these changes do not deserve a separate commit and may stay in
  this patch.  Is it OK?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0001-guix-package-Add-switch-generation-option.patch --]
[-- Type: text/x-diff, Size: 7887 bytes --]

From 3cc52d1aade5e9723c38c0af5fa4437cbdf1a9b6 Mon Sep 17 00:00:00 2001
From: Alex Kost <alezost@gmail.com>
Date: Mon, 6 Oct 2014 17:35:51 +0400
Subject: [PATCH] guix package: Add '--switch-generation' option.

* doc/guix.texi (Invoking guix package): Update documentation.
* guix/profiles.scm (shifted-generation, switch-to-generation): New
  procedures.
* guix/scripts/package.scm: Add '--switch-generation' option.
  (switch-to-previous-generation): Use 'switch-to-generation'.
---
 doc/guix.texi            | 15 +++++++++++++++
 guix/profiles.scm        | 36 +++++++++++++++++++++++++++++++++++-
 guix/scripts/package.scm | 44 +++++++++++++++++++++++++++++++++-----------
 3 files changed, 83 insertions(+), 12 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index f6357bd..c6921b1 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -784,6 +784,21 @@ Installing, removing, or upgrading packages from a generation that has
 been rolled back to overwrites previous future generations.  Thus, the
 history of a profile's generations is always linear.
 
+@item --switch-generation=@var{pattern}
+@itemx -S @var{pattern}
+Switch to a particular generation defined by @var{pattern}.
+
+@var{pattern} may be either a generation number or a number prefixed
+with ``+'' or ``-''.  The latter means: move forward/backward by a
+specified number of generations.  For example, if you want to return to
+the latest generation after @code{--roll-back}, use
+@code{--switch-generation=+1}.
+
+The difference between @code{--roll-back} and
+@code{--switch-generation=-1} is that @code{--switch-generation} will
+not make a zeroth generation, so if a specified generation does not
+exist, the current generation will not be changed.
+
 @item --search-paths
 @cindex search paths
 Report environment variable definitions, in Bash syntax, that may be
diff --git a/guix/profiles.scm b/guix/profiles.scm
index 18733a6..589402e 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -71,9 +71,11 @@
             generation-number
             generation-numbers
             profile-generations
+            shifted-generation
             previous-generation-number
             generation-time
-            generation-file-name))
+            generation-file-name
+            switch-to-generation))
 
 ;;; Commentary:
 ;;;
@@ -569,6 +571,21 @@ former profiles were found."
         '()
         generations)))
 
+(define* (shifted-generation profile shift
+                             #:optional (current (generation-number profile)))
+  "Return PROFILE's generation shifted from the CURRENT generation by SHIFT.
+SHIFT is a positive or negative number.
+Return #f if there is no such generation."
+  (let* ((abs-shift (abs shift))
+         (numbers (profile-generations profile))
+         (from-current (memq current
+                             (if (negative? shift)
+                                 (reverse numbers)
+                                 numbers))))
+    (and from-current
+         (< abs-shift (length from-current))
+         (list-ref from-current abs-shift))))
+
 (define (previous-generation-number profile number)
   "Return the number of the generation before generation NUMBER of
 PROFILE, or 0 if none exists.  It could be NUMBER - 1, but it's not the
@@ -589,4 +606,21 @@ case when generations have been deleted (there are \"holes\")."
   (make-time time-utc 0
              (stat:ctime (stat (generation-file-name profile number)))))
 
+(define (switch-to-generation profile number)
+  "Atomically switch PROFILE to the generation NUMBER."
+  (let ((current (generation-number profile))
+        (file (generation-file-name profile number)))
+    (cond ((not (file-exists? profile))
+           (format (current-error-port)
+                   (_ "profile '~a' does not exist~%")
+                   profile))
+          ((not (file-exists? file))
+           (format (current-error-port)
+                   (_ "generation ~a does not exist~%")
+                   number))
+          (else
+           (format #t (_ "switching from generation ~a to ~a~%")
+                   current number)
+           (switch-symlinks profile file)))))
+
 ;;; profiles.scm ends here
diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index fc9c37b..b071029 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -96,12 +96,9 @@ return PROFILE unchanged.  The goal is to treat '-p ~/.guix-profile' as if
 
 (define (switch-to-previous-generation profile)
   "Atomically switch PROFILE to the previous generation."
-  (let* ((number              (generation-number profile))
-         (previous-number     (previous-generation-number profile number))
-         (previous-generation (generation-file-name profile previous-number)))
-    (format #t (_ "switching from generation ~a to ~a~%")
-            number previous-number)
-    (switch-symlinks profile previous-generation)))
+  (let* ((current  (generation-number profile))
+         (previous (previous-generation-number profile current)))
+    (switch-to-generation profile previous)))
 
 (define (roll-back store profile)
   "Roll back to the previous generation of PROFILE."
@@ -409,6 +406,9 @@ Install, remove, or upgrade PACKAGES in a single transaction.\n"))
   -d, --delete-generations[=PATTERN]
                          delete generations matching PATTERN"))
   (display (_ "
+  -S, --switch-generation=PATTERN
+                         switch to a generation matching PATTERN"))
+  (display (_ "
   -p, --profile=PROFILE  use PROFILE instead of the user's default profile"))
   (newline)
   (display (_ "
@@ -488,6 +488,10 @@ Install, remove, or upgrade PACKAGES in a single transaction.\n"))
                    (values (alist-cons 'delete-generations (or arg "")
                                        result)
                            #f)))
+         (option '(#\S "switch-generation") #t #f
+                 (lambda (opt name arg result arg-handler)
+                   (values (alist-cons 'switch-generation arg result)
+                           #f)))
          (option '("search-paths") #f #f
                  (lambda (opt name arg result arg-handler)
                    (values (cons `(query search-paths) result)
@@ -713,13 +717,31 @@ more information.~%"))
       (generation-number profile))
 
     ;; First roll back if asked to.
-    (cond ((and (assoc-ref opts 'roll-back?) (not dry-run?))
-           (begin
-             (roll-back (%store) profile)
-             (process-actions (alist-delete 'roll-back? opts))))
+    (cond ((and (assoc-ref opts 'roll-back?)
+                (not dry-run?))
+           (roll-back (%store) profile)
+           (process-actions (alist-delete 'roll-back? opts)))
+          ((and (assoc-ref opts 'switch-generation)
+                (not dry-run?))
+           (for-each
+            (match-lambda
+              (('switch-generation . pattern)
+               (let* ((number (string->number pattern))
+                      (number (and number
+                                   (case (string-ref pattern 0)
+                                     ((#\+ #\-)
+                                      (shifted-generation profile number))
+                                     (else number)))))
+                 (if number
+                     (switch-to-generation profile number)
+                     (format (current-error-port)
+                             "Cannot switch to generation '~a'~%" pattern)))
+               (process-actions (alist-delete 'switch-generation opts)))
+              (_ #f))
+            opts))
           ((and (assoc-ref opts 'delete-generations)
                 (not dry-run?))
-           (filter-map
+           (for-each
             (match-lambda
              (('delete-generations . pattern)
               (cond ((not (file-exists? profile)) ; XXX: race condition
-- 
2.1.2


[-- Attachment #5: Type: text/plain, Size: 18 bytes --]


-- 
Thanks,
Alex

Re: [PATCH] guix package: Add '--switch-generation' option.

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> A patch is attached.  Some comments:
>
> - ‘shitted-generation’ is not a very good name, I think.  Ideas?

‘shifted-generation’ is better :-), but otherwise maybe
‘relative-generation’?  No strong opinion.

> - ‘previous-generation-number’ may use ‘shifted-generation’ now:
>
> (define* (previous-generation-number profile #:optional
>                                      (number (generation-number profile)))
>   "Return the number of the generation before generation NUMBER of
> PROFILE, or 0 if none exists.  It could be NUMBER - 1, but it's not the
> case when generations have been deleted (there are \"holes\")."
>   (or (shifted-generation profile -1 number)
>       0))
>
>   Worth changing?

Yes, why not.

> - Perhaps it would be better to make 2 commits (?): one for adding
>   ‘shifted-generation’ and ‘switch-to-generation’ procedures to (guix
>   profiles) and another is for adding the “--switch-generation” option
>   itself.

Yes.

> - Also I made a couple of cosmetic changes in “guix/scripts/package.scm”:
>   * ‘filter-map’ was replaced by 'for-each' because it was called only for
>     side effects there;
>   * ‘begin’ was removed from ‘cond’.
>   I think these changes do not deserve a separate commit and may stay in
>   this patch.  Is it OK?

Several patches make it easier to reason about the changes, but it’s OK
here.  Your call.

> From 3cc52d1aade5e9723c38c0af5fa4437cbdf1a9b6 Mon Sep 17 00:00:00 2001
> From: Alex Kost <alezost@gmail.com>
> Date: Mon, 6 Oct 2014 17:35:51 +0400
> Subject: [PATCH] guix package: Add '--switch-generation' option.
>
> * doc/guix.texi (Invoking guix package): Update documentation.
> * guix/profiles.scm (shifted-generation, switch-to-generation): New
>   procedures.
> * guix/scripts/package.scm: Add '--switch-generation' option.
>   (switch-to-previous-generation): Use 'switch-to-generation'.

Could you add a test in tests/guix-package.sh?

The rest looks good to me, thanks for working on it!

Ludo’.

Re: [PATCH] guix package: Add '--switch-generation' option.

[Alex Kost]

[-- Attachment #1: Type: text/plain, Size: 2455 bytes --]

Ludovic Courtès (2014-10-06 23:27 +0400) wrote:

> Alex Kost <alezost@gmail.com> skribis:
>
>> A patch is attached.  Some comments:
>>
>> - ‘shitted-generation’ is not a very good name, I think.  Ideas?
>
> ‘shifted-generation’ is better :-), but otherwise maybe
> ‘relative-generation’?  No strong opinion.

I like ‘relative-generation’, thanks.

>> - ‘previous-generation-number’ may use ‘shifted-generation’ now:
>>
>> (define* (previous-generation-number profile #:optional
>>                                      (number (generation-number profile)))
>>   "Return the number of the generation before generation NUMBER of
>> PROFILE, or 0 if none exists.  It could be NUMBER - 1, but it's not the
>> case when generations have been deleted (there are \"holes\")."
>>   (or (shifted-generation profile -1 number)
>>       0))
>>
>>   Worth changing?
>
> Yes, why not.

Done.

>> - Perhaps it would be better to make 2 commits (?): one for adding
>>   ‘shifted-generation’ and ‘switch-to-generation’ procedures to (guix
>>   profiles) and another is for adding the “--switch-generation” option
>>   itself.
>
> Yes.

Done.

>> - Also I made a couple of cosmetic changes in “guix/scripts/package.scm”:
>>   * ‘filter-map’ was replaced by 'for-each' because it was called only for
>>     side effects there;
>>   * ‘begin’ was removed from ‘cond’.
>>   I think these changes do not deserve a separate commit and may stay in
>>   this patch.  Is it OK?
>
> Several patches make it easier to reason about the changes, but it’s OK
> here.  Your call.

OK, thanks, I left those changes.

>> From 3cc52d1aade5e9723c38c0af5fa4437cbdf1a9b6 Mon Sep 17 00:00:00 2001
>> From: Alex Kost <alezost@gmail.com>
>> Date: Mon, 6 Oct 2014 17:35:51 +0400
>> Subject: [PATCH] guix package: Add '--switch-generation' option.
>>
>> * doc/guix.texi (Invoking guix package): Update documentation.
>> * guix/profiles.scm (shifted-generation, switch-to-generation): New
>>   procedures.
>> * guix/scripts/package.scm: Add '--switch-generation' option.
>>   (switch-to-previous-generation): Use 'switch-to-generation'.
>
> Could you add a test in tests/guix-package.sh?
>
> The rest looks good to me, thanks for working on it!

Thanks, I've added a couple of tests.  The new patches are attached.
Further improvements (documentation may be unsatisfactory)?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-profiles-Add-procedures-for-switching-generations.patch --]
[-- Type: text/x-diff, Size: 4753 bytes --]

From 9493421a4e094be6686ff6f28749946d491f81cd Mon Sep 17 00:00:00 2001
From: Alex Kost <alezost@gmail.com>
Date: Tue, 7 Oct 2014 11:50:44 +0400
Subject: [PATCH 1/2] profiles: Add procedures for switching generations.

* guix/scripts/package.scm (switch-to-previous-generation): Move to...
* guix/profiles.scm: ... here. Use 'switch-to-generation'.
  (relative-generation): New procedure.
  (previous-generation-number): Use it.
  (switch-to-generation): New procedure.
---
 guix/profiles.scm        | 53 ++++++++++++++++++++++++++++++++++++++++--------
 guix/scripts/package.scm |  9 --------
 2 files changed, 45 insertions(+), 17 deletions(-)

diff --git a/guix/profiles.scm b/guix/profiles.scm
index 18733a6..9920881 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -71,9 +71,12 @@
             generation-number
             generation-numbers
             profile-generations
+            relative-generation
             previous-generation-number
             generation-time
-            generation-file-name))
+            generation-file-name
+            switch-to-generation
+            switch-to-previous-generation))
 
 ;;; Commentary:
 ;;;
@@ -569,16 +572,28 @@ former profiles were found."
         '()
         generations)))
 
-(define (previous-generation-number profile number)
+(define* (relative-generation profile shift #:optional
+                              (current (generation-number profile)))
+  "Return PROFILE's generation shifted from the CURRENT generation by SHIFT.
+SHIFT is a positive or negative number.
+Return #f if there is no such generation."
+  (let* ((abs-shift (abs shift))
+         (numbers (profile-generations profile))
+         (from-current (memq current
+                             (if (negative? shift)
+                                 (reverse numbers)
+                                 numbers))))
+    (and from-current
+         (< abs-shift (length from-current))
+         (list-ref from-current abs-shift))))
+
+(define* (previous-generation-number profile #:optional
+                                     (number (generation-number profile)))
   "Return the number of the generation before generation NUMBER of
 PROFILE, or 0 if none exists.  It could be NUMBER - 1, but it's not the
 case when generations have been deleted (there are \"holes\")."
-  (fold (lambda (candidate highest)
-          (if (and (< candidate number) (> candidate highest))
-              candidate
-              highest))
-        0
-        (generation-numbers profile)))
+  (or (relative-generation profile -1 number)
+      0))
 
 (define (generation-file-name profile generation)
   "Return the file name for PROFILE's GENERATION."
@@ -589,4 +604,26 @@ case when generations have been deleted (there are \"holes\")."
   (make-time time-utc 0
              (stat:ctime (stat (generation-file-name profile number)))))
 
+(define (switch-to-generation profile number)
+  "Atomically switch PROFILE to the generation NUMBER."
+  (let ((current (generation-number profile))
+        (file    (generation-file-name profile number)))
+    (cond ((not (file-exists? profile))
+           (format (current-error-port)
+                   (_ "profile '~a' does not exist~%")
+                   profile))
+          ((not (file-exists? file))
+           (format (current-error-port)
+                   (_ "generation ~a does not exist~%")
+                   number))
+          (else
+           (format #t (_ "switching from generation ~a to ~a~%")
+                   current number)
+           (switch-symlinks profile file)))))
+
+(define (switch-to-previous-generation profile)
+  "Atomically switch PROFILE to the previous generation."
+  (switch-to-generation profile
+                        (previous-generation-number profile)))
+
 ;;; profiles.scm ends here
diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index fc9c37b..d0f1458 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -94,15 +94,6 @@ return PROFILE unchanged.  The goal is to treat '-p ~/.guix-profile' as if
 
     (switch-symlinks generation prof)))
 
-(define (switch-to-previous-generation profile)
-  "Atomically switch PROFILE to the previous generation."
-  (let* ((number              (generation-number profile))
-         (previous-number     (previous-generation-number profile number))
-         (previous-generation (generation-file-name profile previous-number)))
-    (format #t (_ "switching from generation ~a to ~a~%")
-            number previous-number)
-    (switch-symlinks profile previous-generation)))
-
 (define (roll-back store profile)
   "Roll back to the previous generation of PROFILE."
   (let* ((number              (generation-number profile))
-- 
2.1.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-guix-package-Add-switch-generation-option.patch --]
[-- Type: text/x-diff, Size: 6071 bytes --]

From 0d89e5466741d8f80a1ac27502cb6cd600afb796 Mon Sep 17 00:00:00 2001
From: Alex Kost <alezost@gmail.com>
Date: Tue, 7 Oct 2014 12:05:06 +0400
Subject: [PATCH 2/2] guix package: Add '--switch-generation' option.

* guix/scripts/package.scm: Add '--switch-generation' option.
  (guix-package): Adjust accordingly.
* tests/guix-package.sh: Test it.
* doc/guix.texi (Invoking guix package): Document it.
---
 doc/guix.texi            | 15 +++++++++++++++
 guix/scripts/package.scm | 35 ++++++++++++++++++++++++++++++-----
 tests/guix-package.sh    | 14 +++++++++++++-
 3 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index f6357bd..c6921b1 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -784,6 +784,21 @@ Installing, removing, or upgrading packages from a generation that has
 been rolled back to overwrites previous future generations.  Thus, the
 history of a profile's generations is always linear.
 
+@item --switch-generation=@var{pattern}
+@itemx -S @var{pattern}
+Switch to a particular generation defined by @var{pattern}.
+
+@var{pattern} may be either a generation number or a number prefixed
+with ``+'' or ``-''.  The latter means: move forward/backward by a
+specified number of generations.  For example, if you want to return to
+the latest generation after @code{--roll-back}, use
+@code{--switch-generation=+1}.
+
+The difference between @code{--roll-back} and
+@code{--switch-generation=-1} is that @code{--switch-generation} will
+not make a zeroth generation, so if a specified generation does not
+exist, the current generation will not be changed.
+
 @item --search-paths
 @cindex search paths
 Report environment variable definitions, in Bash syntax, that may be
diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index d0f1458..4a4417e 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -400,6 +400,9 @@ Install, remove, or upgrade PACKAGES in a single transaction.\n"))
   -d, --delete-generations[=PATTERN]
                          delete generations matching PATTERN"))
   (display (_ "
+  -S, --switch-generation=PATTERN
+                         switch to a generation matching PATTERN"))
+  (display (_ "
   -p, --profile=PROFILE  use PROFILE instead of the user's default profile"))
   (newline)
   (display (_ "
@@ -479,6 +482,10 @@ Install, remove, or upgrade PACKAGES in a single transaction.\n"))
                    (values (alist-cons 'delete-generations (or arg "")
                                        result)
                            #f)))
+         (option '(#\S "switch-generation") #t #f
+                 (lambda (opt name arg result arg-handler)
+                   (values (alist-cons 'switch-generation arg result)
+                           #f)))
          (option '("search-paths") #f #f
                  (lambda (opt name arg result arg-handler)
                    (values (cons `(query search-paths) result)
@@ -704,13 +711,31 @@ more information.~%"))
       (generation-number profile))
 
     ;; First roll back if asked to.
-    (cond ((and (assoc-ref opts 'roll-back?) (not dry-run?))
-           (begin
-             (roll-back (%store) profile)
-             (process-actions (alist-delete 'roll-back? opts))))
+    (cond ((and (assoc-ref opts 'roll-back?)
+                (not dry-run?))
+           (roll-back (%store) profile)
+           (process-actions (alist-delete 'roll-back? opts)))
+          ((and (assoc-ref opts 'switch-generation)
+                (not dry-run?))
+           (for-each
+            (match-lambda
+              (('switch-generation . pattern)
+               (let* ((number (string->number pattern))
+                      (number (and number
+                                   (case (string-ref pattern 0)
+                                     ((#\+ #\-)
+                                      (relative-generation profile number))
+                                     (else number)))))
+                 (if number
+                     (switch-to-generation profile number)
+                     (format (current-error-port)
+                             "Cannot switch to generation '~a'~%" pattern)))
+               (process-actions (alist-delete 'switch-generation opts)))
+              (_ #f))
+            opts))
           ((and (assoc-ref opts 'delete-generations)
                 (not dry-run?))
-           (filter-map
+           (for-each
             (match-lambda
              (('delete-generations . pattern)
               (cond ((not (file-exists? profile)) ; XXX: race condition
diff --git a/tests/guix-package.sh b/tests/guix-package.sh
index 9b0e75e..5ad0873 100644
--- a/tests/guix-package.sh
+++ b/tests/guix-package.sh
@@ -100,6 +100,16 @@ then
     test "`readlink_base "$profile"`" = "$profile-1-link"
     test -x "$profile/bin/guile" && ! test -x "$profile/bin/make"
 
+    # Switch to the rolled generation and switch back.
+    guix package -p "$profile" --switch-generation=2
+    test "`readlink_base "$profile"`" = "$profile-2-link"
+    guix package -p "$profile" --switch-generation=-1
+    test "`readlink_base "$profile"`" = "$profile-1-link"
+
+    # Switching to a non-existing generation does not change the current one.
+    guix package -p "$profile" --switch-generation=99
+    test "`readlink_base "$profile"`" = "$profile-1-link"
+
     # Move to the empty profile.
     for i in `seq 1 3`
     do
@@ -132,10 +142,12 @@ then
     grep "`guix build -e "$boot_make"`" "$profile/manifest"
 
     # Make a "hole" in the list of generations, and make sure we can
-    # roll back "over" it.
+    # roll back and switch "over" it.
     rm "$profile-1-link"
     guix package --bootstrap -p "$profile" --roll-back
     test "`readlink_base "$profile"`" = "$profile-0-link"
+    guix package -p "$profile" --switch-generation=+1
+    test "`readlink_base "$profile"`" = "$profile-2-link"
 
     # Make sure LIBRARY_PATH gets listed by `--search-paths'.
     guix package --bootstrap -p "$profile" -i guile-bootstrap -i gcc-bootstrap
-- 
2.1.2

Re: [PATCH] guix package: Add '--switch-generation' option.

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> Thanks, I've added a couple of tests.  The new patches are attached.

Thanks for the quick reply.

> Further improvements (documentation may be unsatisfactory)?

> From 9493421a4e094be6686ff6f28749946d491f81cd Mon Sep 17 00:00:00 2001
> From: Alex Kost <alezost@gmail.com>
> Date: Tue, 7 Oct 2014 11:50:44 +0400
> Subject: [PATCH 1/2] profiles: Add procedures for switching generations.
>
> * guix/scripts/package.scm (switch-to-previous-generation): Move to...
> * guix/profiles.scm: ... here. Use 'switch-to-generation'.
>   (relative-generation): New procedure.
>   (previous-generation-number): Use it.
>   (switch-to-generation): New procedure.

[...]

> +(define* (relative-generation profile shift #:optional
> +                              (current (generation-number profile)))
> +  "Return PROFILE's generation shifted from the CURRENT generation by SHIFT.
> +SHIFT is a positive or negative number.
> +Return #f if there is no such generation."

[...]

> +(define (switch-to-generation profile number)
> +  "Atomically switch PROFILE to the generation NUMBER."
> +  (let ((current (generation-number profile))
> +        (file    (generation-file-name profile number)))
> +    (cond ((not (file-exists? profile))
> +           (format (current-error-port)
> +                   (_ "profile '~a' does not exist~%")
> +                   profile))
> +          ((not (file-exists? file))
> +           (format (current-error-port)
> +                   (_ "generation ~a does not exist~%")
> +                   number))
> +          (else
> +           (format #t (_ "switching from generation ~a to ~a~%")
> +                   current number)
> +           (switch-symlinks profile file)))))

Could this procedure raise an exception instead of writing messages?
The reason is that I’d like UI code to remain in (guix scripts package),
in the Emacs code, and in guix-web, with (guix profiles) remaining
generic.

It’d be enough for me to just call ‘switch-symlinks’ and let it throw
‘system-error’ if something’s wrong.  The exception will be caught, the
user will see a “No such file” error, and ‘guix package’ with exit with
non-zero (this is done by ‘call-with-error-handling’.)

It’s less informative than what you did, though.  The other option would
be to define specific error condition types and throw them from here.

WDYT?

My apologies for being sloppy and not catching it earlier!

> From 0d89e5466741d8f80a1ac27502cb6cd600afb796 Mon Sep 17 00:00:00 2001
> From: Alex Kost <alezost@gmail.com>
> Date: Tue, 7 Oct 2014 12:05:06 +0400
> Subject: [PATCH 2/2] guix package: Add '--switch-generation' option.
>
> * guix/scripts/package.scm: Add '--switch-generation' option.
>   (guix-package): Adjust accordingly.
> * tests/guix-package.sh: Test it.
> * doc/guix.texi (Invoking guix package): Document it.

[...]

> +              (('switch-generation . pattern)
> +               (let* ((number (string->number pattern))
> +                      (number (and number
> +                                   (case (string-ref pattern 0)
> +                                     ((#\+ #\-)
> +                                      (relative-generation profile number))
> +                                     (else number)))))
> +                 (if number
> +                     (switch-to-generation profile number)
> +                     (format (current-error-port)
> +                             "Cannot switch to generation '~a'~%" pattern)))

Use ‘leave’ instead of ‘format’ here, with lower-case “cannot”.

The rest is perfect.

Thanks for your patience,
Ludo’.

Re: [PATCH] guix package: Add '--switch-generation' option.

[Alex Kost]

[-- Attachment #1: Type: text/plain, Size: 2916 bytes --]

Ludovic Courtès (2014-10-07 20:00 +0400) wrote:

> Alex Kost <alezost@gmail.com> skribis:
>
> [...]
>
>> +(define (switch-to-generation profile number)
>> +  "Atomically switch PROFILE to the generation NUMBER."
>> +  (let ((current (generation-number profile))
>> +        (file    (generation-file-name profile number)))
>> +    (cond ((not (file-exists? profile))
>> +           (format (current-error-port)
>> +                   (_ "profile '~a' does not exist~%")
>> +                   profile))
>> +          ((not (file-exists? file))
>> +           (format (current-error-port)
>> +                   (_ "generation ~a does not exist~%")
>> +                   number))
>> +          (else
>> +           (format #t (_ "switching from generation ~a to ~a~%")
>> +                   current number)
>> +           (switch-symlinks profile file)))))
>
> Could this procedure raise an exception instead of writing messages?
> The reason is that I’d like UI code to remain in (guix scripts package),
> in the Emacs code, and in guix-web, with (guix profiles) remaining
> generic.

I see, thanks for the explanation.

> It’d be enough for me to just call ‘switch-symlinks’ and let it throw
> ‘system-error’ if something’s wrong.  The exception will be caught, the
> user will see a “No such file” error, and ‘guix package’ with exit with
> non-zero (this is done by ‘call-with-error-handling’.)

‘switch-symlinks’ does not throw an error even if files don't exist, so…

> It’s less informative than what you did, though.  The other option would
> be to define specific error condition types and throw them from here.
>
> WDYT?

… I tried to make it this way, thank you for pointing.  I made another
commit for adding and using condition types (3 patches are attached
now).

Also I moved ‘process-query’ inside ‘with-error-handling’ (because I
used ‘raise’ there).  Could there be unwanted consequences after that?

> My apologies for being sloppy and not catching it earlier!

No problem at all.  I hope you catch something now if it is there.

>
> [...]
>
>> +              (('switch-generation . pattern)
>> +               (let* ((number (string->number pattern))
>> +                      (number (and number
>> +                                   (case (string-ref pattern 0)
>> +                                     ((#\+ #\-)
>> +                                      (relative-generation profile number))
>> +                                     (else number)))))
>> +                 (if number
>> +                     (switch-to-generation profile number)
>> +                     (format (current-error-port)
>> +                             "Cannot switch to generation '~a'~%" pattern)))
>
> Use ‘leave’ instead of ‘format’ here, with lower-case “cannot”.

Done.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-profiles-Add-condition-types-for-profile-and-generat.patch --]
[-- Type: text/x-diff, Size: 5004 bytes --]

From d5e9abb0395a21e79d4f77181597103d4daf138c Mon Sep 17 00:00:00 2001
From: Alex Kost <alezost@gmail.com>
Date: Wed, 8 Oct 2014 00:32:28 +0400
Subject: [PATCH 1/3] profiles: Add condition types for profile and generation.

* guix/profiles.scm (&profile-error, &generation-error): New condition types.
* guix/ui.scm (call-with-error-handling): Handle these types.
* guix/scripts/package.scm (roll-back, guix-package): Raise '&profile-error'
  where needed.
---
 guix/profiles.scm        | 24 +++++++++++++++++++++++-
 guix/scripts/package.scm | 15 +++++++--------
 guix/ui.scm              |  7 +++++++
 3 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/guix/profiles.scm b/guix/profiles.scm
index 18733a6..0e19d7a 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -35,7 +35,16 @@
   #:use-module (srfi srfi-11)
   #:use-module (srfi srfi-19)
   #:use-module (srfi srfi-26)
-  #:export (manifest make-manifest
+  #:use-module (srfi srfi-34)
+  #:use-module (srfi srfi-35)
+  #:export (&profile-error
+            profile-error?
+            profile-error-profile
+            &generation-error
+            generation-error?
+            generation-error-generation
+
+            manifest make-manifest
             manifest?
             manifest-entries
 
@@ -84,6 +93,19 @@
 
 \f
 ;;;
+;;; Condition types.
+;;;
+
+(define-condition-type &profile-error &error
+  profile-error?
+  (profile profile-error-profile))
+
+(define-condition-type &generation-error &error
+  generation-error?
+  (generation generation-error-generation))
+
+\f
+;;;
 ;;; Manifests.
 ;;;
 
diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index fc9c37b..7e2143c 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -38,6 +38,8 @@
   #:use-module (srfi srfi-11)
   #:use-module (srfi srfi-19)
   #:use-module (srfi srfi-26)
+  #:use-module (srfi srfi-34)
+  #:use-module (srfi srfi-35)
   #:use-module (srfi srfi-37)
   #:use-module (gnu packages)
   #:use-module (gnu packages base)
@@ -109,8 +111,7 @@ return PROFILE unchanged.  The goal is to treat '-p ~/.guix-profile' as if
          (previous-number     (previous-generation-number profile number))
          (previous-generation (generation-file-name profile previous-number)))
     (cond ((not (file-exists? profile))                 ; invalid profile
-           (leave (_ "profile '~a' does not exist~%")
-                  profile))
+           (raise (condition (&profile-error (profile profile)))))
           ((zero? number)                               ; empty profile
            (format (current-error-port)
                    (_ "nothing to do: already at the empty profile~%")))
@@ -723,8 +724,7 @@ more information.~%"))
             (match-lambda
              (('delete-generations . pattern)
               (cond ((not (file-exists? profile)) ; XXX: race condition
-                     (leave (_ "profile '~a' does not exist~%")
-                            profile))
+                     (raise (condition (&profile-error (profile profile)))))
                     ((string-null? pattern)
                      (delete-generations
                       (%store) profile
@@ -833,8 +833,7 @@ more information.~%"))
              (newline)))
 
          (cond ((not (file-exists? profile)) ; XXX: race condition
-                (leave (_ "profile '~a' does not exist~%")
-                       profile))
+                (raise (condition (&profile-error (profile profile)))))
                ((string-null? pattern)
                 (for-each list-generation (profile-generations profile)))
                ((matching-generations pattern profile)
@@ -915,8 +914,8 @@ more information.~%"))
         (_ #f))))
 
   (let ((opts (parse-options)))
-    (or (process-query opts)
-        (with-error-handling
+    (with-error-handling
+      (or (process-query opts)
           (parameterize ((%store (open-connection)))
             (set-build-options-from-command-line (%store) opts)
 
diff --git a/guix/ui.scm b/guix/ui.scm
index 04345d4..9c0a5d2 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -23,6 +23,7 @@
   #:use-module (guix store)
   #:use-module (guix config)
   #:use-module (guix packages)
+  #:use-module (guix profiles)
   #:use-module (guix build-system)
   #:use-module (guix derivations)
   #:use-module ((guix build utils) #:select (mkdir-p))
@@ -229,6 +230,12 @@ interpreted."
                       (location->string loc)
                       (package-full-name package)
                       (build-system-name system))))
+            ((profile-error? c)
+             (leave (_ "profile '~a' does not exist~%")
+                    (profile-error-profile c)))
+            ((generation-error? c)
+             (leave (_ "generation '~a' does not exist~%")
+                    (generation-error-generation c)))
             ((nix-connection-error? c)
              (leave (_ "failed to connect to `~a': ~a~%")
                     (nix-connection-error-file c)
-- 
2.1.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-profiles-Add-procedures-for-switching-generations.patch --]
[-- Type: text/x-diff, Size: 4663 bytes --]

From e47644b43aaa73885ca648118b6fc59fdb499303 Mon Sep 17 00:00:00 2001
From: Alex Kost <alezost@gmail.com>
Date: Wed, 8 Oct 2014 00:39:42 +0400
Subject: [PATCH 2/3] profiles: Add procedures for switching generations.

* guix/scripts/package.scm (switch-to-previous-generation): Move to...
* guix/profiles.scm: ... here. Use 'switch-to-generation'.
  (relative-generation): New procedure.
  (previous-generation-number): Use it.
  (switch-to-generation): New procedure.
---
 guix/profiles.scm        | 49 ++++++++++++++++++++++++++++++++++++++++--------
 guix/scripts/package.scm |  9 ---------
 2 files changed, 41 insertions(+), 17 deletions(-)

diff --git a/guix/profiles.scm b/guix/profiles.scm
index 0e19d7a..d064351 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -80,9 +80,12 @@
             generation-number
             generation-numbers
             profile-generations
+            relative-generation
             previous-generation-number
             generation-time
-            generation-file-name))
+            generation-file-name
+            switch-to-generation
+            switch-to-previous-generation))
 
 ;;; Commentary:
 ;;;
@@ -591,16 +594,28 @@ former profiles were found."
         '()
         generations)))
 
-(define (previous-generation-number profile number)
+(define* (relative-generation profile shift #:optional
+                              (current (generation-number profile)))
+  "Return PROFILE's generation shifted from the CURRENT generation by SHIFT.
+SHIFT is a positive or negative number.
+Return #f if there is no such generation."
+  (let* ((abs-shift (abs shift))
+         (numbers (profile-generations profile))
+         (from-current (memq current
+                             (if (negative? shift)
+                                 (reverse numbers)
+                                 numbers))))
+    (and from-current
+         (< abs-shift (length from-current))
+         (list-ref from-current abs-shift))))
+
+(define* (previous-generation-number profile #:optional
+                                     (number (generation-number profile)))
   "Return the number of the generation before generation NUMBER of
 PROFILE, or 0 if none exists.  It could be NUMBER - 1, but it's not the
 case when generations have been deleted (there are \"holes\")."
-  (fold (lambda (candidate highest)
-          (if (and (< candidate number) (> candidate highest))
-              candidate
-              highest))
-        0
-        (generation-numbers profile)))
+  (or (relative-generation profile -1 number)
+      0))
 
 (define (generation-file-name profile generation)
   "Return the file name for PROFILE's GENERATION."
@@ -611,4 +626,22 @@ case when generations have been deleted (there are \"holes\")."
   (make-time time-utc 0
              (stat:ctime (stat (generation-file-name profile number)))))
 
+(define (switch-to-generation profile number)
+  "Atomically switch PROFILE to the generation NUMBER."
+  (let ((current    (generation-number profile))
+        (generation (generation-file-name profile number)))
+    (cond ((not (file-exists? profile))
+           (raise (condition (&profile-error (profile profile)))))
+          ((not (file-exists? generation))
+           (raise (condition (&generation-error (generation generation)))))
+          (else
+           (format #t (_ "switching from generation ~a to ~a~%")
+                   current number)
+           (switch-symlinks profile generation)))))
+
+(define (switch-to-previous-generation profile)
+  "Atomically switch PROFILE to the previous generation."
+  (switch-to-generation profile
+                        (previous-generation-number profile)))
+
 ;;; profiles.scm ends here
diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index 7e2143c..df8a7f2 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -96,15 +96,6 @@ return PROFILE unchanged.  The goal is to treat '-p ~/.guix-profile' as if
 
     (switch-symlinks generation prof)))
 
-(define (switch-to-previous-generation profile)
-  "Atomically switch PROFILE to the previous generation."
-  (let* ((number              (generation-number profile))
-         (previous-number     (previous-generation-number profile number))
-         (previous-generation (generation-file-name profile previous-number)))
-    (format #t (_ "switching from generation ~a to ~a~%")
-            number previous-number)
-    (switch-symlinks profile previous-generation)))
-
 (define (roll-back store profile)
   "Roll back to the previous generation of PROFILE."
   (let* ((number              (generation-number profile))
-- 
2.1.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-guix-package-Add-switch-generation-option.patch --]
[-- Type: text/x-diff, Size: 6202 bytes --]

From 003e5c192796e8ea07491a94a85824a533155825 Mon Sep 17 00:00:00 2001
From: Alex Kost <alezost@gmail.com>
Date: Wed, 8 Oct 2014 00:45:38 +0400
Subject: [PATCH 3/3] guix package: Add '--switch-generation' option.

* guix/scripts/package.scm: Add '--switch-generation' option.
  (guix-package): Adjust accordingly.
* tests/guix-package.sh: Test it.
* doc/guix.texi (Invoking guix package): Document it.
---
 doc/guix.texi            | 15 +++++++++++++++
 guix/scripts/package.scm | 35 ++++++++++++++++++++++++++++++-----
 tests/guix-package.sh    | 12 +++++++++++-
 3 files changed, 56 insertions(+), 6 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index f6357bd..c6921b1 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -784,6 +784,21 @@ Installing, removing, or upgrading packages from a generation that has
 been rolled back to overwrites previous future generations.  Thus, the
 history of a profile's generations is always linear.
 
+@item --switch-generation=@var{pattern}
+@itemx -S @var{pattern}
+Switch to a particular generation defined by @var{pattern}.
+
+@var{pattern} may be either a generation number or a number prefixed
+with ``+'' or ``-''.  The latter means: move forward/backward by a
+specified number of generations.  For example, if you want to return to
+the latest generation after @code{--roll-back}, use
+@code{--switch-generation=+1}.
+
+The difference between @code{--roll-back} and
+@code{--switch-generation=-1} is that @code{--switch-generation} will
+not make a zeroth generation, so if a specified generation does not
+exist, the current generation will not be changed.
+
 @item --search-paths
 @cindex search paths
 Report environment variable definitions, in Bash syntax, that may be
diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index df8a7f2..0278f62 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -401,6 +401,9 @@ Install, remove, or upgrade PACKAGES in a single transaction.\n"))
   -d, --delete-generations[=PATTERN]
                          delete generations matching PATTERN"))
   (display (_ "
+  -S, --switch-generation=PATTERN
+                         switch to a generation matching PATTERN"))
+  (display (_ "
   -p, --profile=PROFILE  use PROFILE instead of the user's default profile"))
   (newline)
   (display (_ "
@@ -480,6 +483,10 @@ Install, remove, or upgrade PACKAGES in a single transaction.\n"))
                    (values (alist-cons 'delete-generations (or arg "")
                                        result)
                            #f)))
+         (option '(#\S "switch-generation") #t #f
+                 (lambda (opt name arg result arg-handler)
+                   (values (alist-cons 'switch-generation arg result)
+                           #f)))
          (option '("search-paths") #f #f
                  (lambda (opt name arg result arg-handler)
                    (values (cons `(query search-paths) result)
@@ -705,13 +712,31 @@ more information.~%"))
       (generation-number profile))
 
     ;; First roll back if asked to.
-    (cond ((and (assoc-ref opts 'roll-back?) (not dry-run?))
-           (begin
-             (roll-back (%store) profile)
-             (process-actions (alist-delete 'roll-back? opts))))
+    (cond ((and (assoc-ref opts 'roll-back?)
+                (not dry-run?))
+           (roll-back (%store) profile)
+           (process-actions (alist-delete 'roll-back? opts)))
+          ((and (assoc-ref opts 'switch-generation)
+                (not dry-run?))
+           (for-each
+            (match-lambda
+              (('switch-generation . pattern)
+               (let* ((number (string->number pattern))
+                      (number (and number
+                                   (case (string-ref pattern 0)
+                                     ((#\+ #\-)
+                                      (relative-generation profile number))
+                                     (else number)))))
+                 (if number
+                     (switch-to-generation profile number)
+                     (leave (_ "cannot switch to generation '~a'~%")
+                            pattern)))
+               (process-actions (alist-delete 'switch-generation opts)))
+              (_ #f))
+            opts))
           ((and (assoc-ref opts 'delete-generations)
                 (not dry-run?))
-           (filter-map
+           (for-each
             (match-lambda
              (('delete-generations . pattern)
               (cond ((not (file-exists? profile)) ; XXX: race condition
diff --git a/tests/guix-package.sh b/tests/guix-package.sh
index 9b0e75e..c01e914 100644
--- a/tests/guix-package.sh
+++ b/tests/guix-package.sh
@@ -86,6 +86,8 @@ then
     # Exit with 1 when a generation does not exist.
     if guix package -p "$profile" --list-generations=42;
     then false; else true; fi
+    if guix package -p "$profile" --switch-generation=99;
+    then false; else true; fi
 
     # Remove a package.
     guix package --bootstrap -p "$profile" -r "guile-bootstrap"
@@ -100,6 +102,12 @@ then
     test "`readlink_base "$profile"`" = "$profile-1-link"
     test -x "$profile/bin/guile" && ! test -x "$profile/bin/make"
 
+    # Switch to the rolled generation and switch back.
+    guix package -p "$profile" --switch-generation=2
+    test "`readlink_base "$profile"`" = "$profile-2-link"
+    guix package -p "$profile" --switch-generation=-1
+    test "`readlink_base "$profile"`" = "$profile-1-link"
+
     # Move to the empty profile.
     for i in `seq 1 3`
     do
@@ -132,10 +140,12 @@ then
     grep "`guix build -e "$boot_make"`" "$profile/manifest"
 
     # Make a "hole" in the list of generations, and make sure we can
-    # roll back "over" it.
+    # roll back and switch "over" it.
     rm "$profile-1-link"
     guix package --bootstrap -p "$profile" --roll-back
     test "`readlink_base "$profile"`" = "$profile-0-link"
+    guix package -p "$profile" --switch-generation=+1
+    test "`readlink_base "$profile"`" = "$profile-2-link"
 
     # Make sure LIBRARY_PATH gets listed by `--search-paths'.
     guix package --bootstrap -p "$profile" -i guile-bootstrap -i gcc-bootstrap
-- 
2.1.2

Re: [PATCH] guix package: Add '--switch-generation' option.

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> Ludovic Courtès (2014-10-07 20:00 +0400) wrote:
>
>> Alex Kost <alezost@gmail.com> skribis:

[...]

>> It’d be enough for me to just call ‘switch-symlinks’ and let it throw
>> ‘system-error’ if something’s wrong.  The exception will be caught, the
>> user will see a “No such file” error, and ‘guix package’ with exit with
>> non-zero (this is done by ‘call-with-error-handling’.)
>
> ‘switch-symlinks’ does not throw an error even if files don't exist, so…

Oh right, ‘symlink’ doesn’t care about its first argument.

>> It’s less informative than what you did, though.  The other option would
>> be to define specific error condition types and throw them from here.
>>
>> WDYT?
>
> … I tried to make it this way, thank you for pointing.  I made another
> commit for adding and using condition types (3 patches are attached
> now).

Great.

> Also I moved ‘process-query’ inside ‘with-error-handling’ (because I
> used ‘raise’ there).  Could there be unwanted consequences after that?

No, I think it’s fine.

> From d5e9abb0395a21e79d4f77181597103d4daf138c Mon Sep 17 00:00:00 2001
> From: Alex Kost <alezost@gmail.com>
> Date: Wed, 8 Oct 2014 00:32:28 +0400
> Subject: [PATCH 1/3] profiles: Add condition types for profile and generation.
>
> * guix/profiles.scm (&profile-error, &generation-error): New condition types.
> * guix/ui.scm (call-with-error-handling): Handle these types.
> * guix/scripts/package.scm (roll-back, guix-package): Raise '&profile-error'
>   where needed.

This is bikeshedding, but I would make a hierarchy like this:

                     &profile-error, with ‘profile’ field
                            ^
           .———————————————–+———————————————–.
           |                                 |
  &profile-not-found-error        &missing-generation-error, with ‘generation’ field

WDYT?

Other than that the patch looks good.

> From e47644b43aaa73885ca648118b6fc59fdb499303 Mon Sep 17 00:00:00 2001
> From: Alex Kost <alezost@gmail.com>
> Date: Wed, 8 Oct 2014 00:39:42 +0400
> Subject: [PATCH 2/3] profiles: Add procedures for switching generations.
>
> * guix/scripts/package.scm (switch-to-previous-generation): Move to...
> * guix/profiles.scm: ... here. Use 'switch-to-generation'.
>   (relative-generation): New procedure.
>   (previous-generation-number): Use it.
>   (switch-to-generation): New procedure.

OK.

> From 003e5c192796e8ea07491a94a85824a533155825 Mon Sep 17 00:00:00 2001
> From: Alex Kost <alezost@gmail.com>
> Date: Wed, 8 Oct 2014 00:45:38 +0400
> Subject: [PATCH 3/3] guix package: Add '--switch-generation' option.
>
> * guix/scripts/package.scm: Add '--switch-generation' option.
>   (guix-package): Adjust accordingly.
> * tests/guix-package.sh: Test it.
> * doc/guix.texi (Invoking guix package): Document it.

OK.

Thanks again!

Ludo’.

Re: [PATCH] guix package: Export generation procedures.

[Andreas Enge]

On Sat, Oct 04, 2014 at 09:59:06PM +0400, Alex Kost wrote:
> - (Not related to this patch, but still …)  Currently with “roll-back”,
>   we can only switch to the previous generation.  What about adding a
>   possibility to switch to any generation?  So that we could use
>   something like this:
>     guix package --switch-generation=7

Actually, sometimes I would like to switch to the next generation ("-roll-
forward", in a sense): I install something, go back with "--roll-back",
and might like to go just one forward again.

So how about the following:
--switch-generation=1
goes to generation 1;
--switch-generation=-1
goes 1 generation back;
--switch-generation=+1
goes one generation forward.

Andreas

Re: [PATCH] guix package: Export generation procedures.

[Ludovic Courtès]

Andreas Enge <andreas@enge.fr> skribis:

> So how about the following:
> --switch-generation=1
> goes to generation 1;
> --switch-generation=-1
> goes 1 generation back;
> --switch-generation=+1
> goes one generation forward.

Sounds like a good idea.

Ludo’.

Re: Emacs interface for Guix

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> (“package->manifest-entry” from “options->installable” would be very
> useful for example)

Commit 462f5cc makes it public.

Ludo’.