guix-package --roll-back

guix-package --roll-back

[Nikita Karetnikov]

[-- Attachment #1.1: Type: text/plain, Size: 392 bytes --]

Hi,

What do you think?

(I'll send the output of 'git format-patch' when I implement the
command-line part.)

By the way, could you explain how the following works?

  (match (scandir (dirname profile)
                  (cut regexp-exec %profile-rx <>)) ...)

How does 'regexp-exec' get its second argument? (I know how 'cut' works,
but I don't understand this particular snippet.)

Nikita


[-- Attachment #1.2: guix-package.diff --]
[-- Type: text/x-diff, Size: 3187 bytes --]

--- guix-package-orig	2012-12-16 17:38:40.000000000 +0000
+++ guix-package	2012-12-21 22:28:08.000000000 +0000
@@ -13,6 +13,7 @@
 !#
 ;;; Guix --- Nix package management from Guile.         -*- coding: utf-8 -*-
 ;;; Copyright (C) 2012 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright (C) 2012 Nikita Karetnikov <nikita@karetnikov.org>
 ;;;
 ;;; This file is part of Guix.
 ;;;
@@ -47,7 +48,8 @@
   #:use-module (srfi srfi-37)
   #:use-module (distro)
   #:use-module (distro packages guile)
-  #:export (guix-package))
+  #:export (guix-package)
+  #:export (roll-back))
 
 (define %store
   (open-connection))
@@ -87,13 +89,13 @@
     (_
      (error "unsupported manifest format" manifest))))
 
+(define (profile-rx profile)
+  (make-regexp (string-append "^" (regexp-quote (basename profile))
+                              "-([0-9]+)")))
+
 (define (latest-profile-number profile)
   "Return the identifying number of the latest generation of PROFILE.
 PROFILE is the name of the symlink to the current generation."
-  (define %profile-rx
-    (make-regexp (string-append "^" (regexp-quote (basename profile))
-                                "-([0-9]+)")))
-
   (define* (scandir name #:optional (select? (const #t))
                     (entry<? (@ (ice-9 i18n) string-locale<?)))
     ;; XXX: Bug-fix version introduced in Guile v2.0.6-62-g139ce19.
@@ -129,7 +131,7 @@
              (sort files entry<?))))
 
   (match (scandir (dirname profile)
-                  (cut regexp-exec %profile-rx <>))
+                  (cut regexp-exec (profile-rx profile) <>))
     (#f                                         ; no profile directory
      0)
     (()                                         ; no profiles
@@ -137,7 +139,7 @@
     ((profiles ...)                             ; former profiles around
      (let ((numbers (map (compose string->number
                                   (cut match:substring <> 1)
-                                  (cut regexp-exec %profile-rx <>))
+                                  (cut regexp-exec (profile-rx profile) <>))
                          profiles)))
        (fold (lambda (number highest)
                (if (> number highest)
@@ -177,6 +179,25 @@
                                      packages)
                                 #:modules '((guix build union))))
 
+(define (profile-number profile)
+  "Return PROFILE's number. PROFILE should be an absolute filename."
+  (match:substring (regexp-exec (profile-rx profile)
+                                (basename (readlink profile))) 1))
+
+(define (roll-back)
+  "Roll back to the previous profile."
+  (let* ((current-profile-number
+          (string->number (profile-number %current-profile)))
+         (previous-profile
+          (string-append %current-profile "-"
+                         (number->string (- current-profile-number 1))
+                         "-link")))
+    (if (= current-profile-number 1)
+        (error "there are no other profiles.")
+        (delete-file %current-profile))
+
+    (symlink previous-profile %current-profile)))
+
 \f
 ;;;
 ;;; Command-line options.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: guix-package --roll-back

[Nikita Karetnikov]

[-- Attachment #1.1: Type: text/plain, Size: 495 bytes --]

Hi,

I've slightly changed the 'roll-back' function. Now it prints an
informative message. And it's also possible to call it from a
command-line.

  # ./pre-inst-env guix-package --roll-back
  guix-package: switching from generation 6 to 5

The only problem I see is the following line.

  (error "there are no other profiles.")

It will print an ugly backtrace if there are no other profiles. I'm
trying to understand how to handle it in a non-ugly way.

Any comments are appreciated.

Nikita


[-- Attachment #1.2: guix-package.diff --]
[-- Type: text/x-diff, Size: 4115 bytes --]

--- guix-package-orig	2012-12-29 22:19:46.000000000 +0000
+++ guix-package	2012-12-29 22:42:41.000000000 +0000
@@ -13,6 +13,7 @@
 !#
 ;;; Guix --- Nix package management from Guile.         -*- coding: utf-8 -*-
 ;;; Copyright (C) 2012 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright (C) 2012 Nikita Karetnikov <nikita@karetnikov.org>
 ;;;
 ;;; This file is part of Guix.
 ;;;
@@ -87,13 +88,14 @@
     (_
      (error "unsupported manifest format" manifest))))
 
+(define (profile-rx profile)
+  "Return a regular expression that matches PROFILE's name and number."
+  (make-regexp (string-append "^" (regexp-quote (basename profile))
+                              "-([0-9]+)")))
+
 (define (latest-profile-number profile)
   "Return the identifying number of the latest generation of PROFILE.
 PROFILE is the name of the symlink to the current generation."
-  (define %profile-rx
-    (make-regexp (string-append "^" (regexp-quote (basename profile))
-                                "-([0-9]+)")))
-
   (define* (scandir name #:optional (select? (const #t))
                     (entry<? (@ (ice-9 i18n) string-locale<?)))
     ;; XXX: Bug-fix version introduced in Guile v2.0.6-62-g139ce19.
@@ -129,7 +131,7 @@
              (sort files entry<?))))
 
   (match (scandir (dirname profile)
-                  (cut regexp-exec %profile-rx <>))
+                  (cut regexp-exec (profile-rx profile) <>))
     (#f                                         ; no profile directory
      0)
     (()                                         ; no profiles
@@ -137,7 +139,7 @@
     ((profiles ...)                             ; former profiles around
      (let ((numbers (map (compose string->number
                                   (cut match:substring <> 1)
-                                  (cut regexp-exec %profile-rx <>))
+                                  (cut regexp-exec (profile-rx profile) <>))
                          profiles)))
        (fold (lambda (number highest)
                (if (> number highest)
@@ -177,6 +179,31 @@
                                      packages)
                                 #:modules '((guix build union))))
 
+(define (profile-number profile)
+  "Return PROFILE's number. PROFILE should be an absolute filename."
+  (match:substring (regexp-exec (profile-rx profile)
+                                (basename (readlink profile))) 1))
+
+(define (roll-back)
+  "Roll back to the previous profile."
+  (let* ((current-profile-number
+          (string->number (profile-number %current-profile)))
+         (previous-profile-number (number->string (1- current-profile-number)))
+         (previous-profile
+          (string-append %current-profile "-"
+                         previous-profile-number "-link")))
+
+    (define (switch)
+      "Switch to the previous generation."
+      (simple-format #t "guix-package: switching from generation ~a to ~a~%"
+                     current-profile-number previous-profile-number)
+      (delete-file %current-profile)
+      (symlink previous-profile %current-profile))
+
+    (if (= current-profile-number 1)
+        (error "there are no other profiles.") ; XXX: handle this error
+        (switch))))
+
 \f
 ;;;
 ;;; Command-line options.
@@ -201,6 +228,8 @@
   (display (_ "
   -n, --dry-run          show what would be done without actually doing it"))
   (display (_ "
+      --roll-back        roll back to the previous generation"))
+  (display (_ "
   -b, --bootstrap        use the bootstrap Guile to build the profile"))
   (display (_ "
       --verbose          produce verbose output"))
@@ -236,6 +265,10 @@
         (option '(#\r "remove") #t #f
                 (lambda (opt name arg result)
                   (alist-cons 'remove arg result)))
+        (option '("roll-back") #f #f
+                (lambda args
+                  (roll-back)
+                  (exit 0)))
         (option '(#\p "profile") #t #f
                 (lambda (opt name arg result)
                   (alist-cons 'profile arg

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: guix-package --roll-back

[Andreas Enge]

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

Hello,
Am Sonntag, 30. Dezember 2012 schrieb Nikita Karetnikov:
> The only problem I see is the following line.
>   (error "there are no other profiles.")
> It will print an ugly backtrace if there are no other profiles. I'm
> trying to understand how to handle it in a non-ugly way.

How about just printing an error message using "display" and doing nothing 
else?

Andreas

[-- Attachment #2: Type: text/html, Size: 2329 bytes --]

Re: guix-package --roll-back

[Ludovic Courtès]

Hi!

Looks like a good start!

Nikita Karetnikov <nikita@karetnikov.org> skribis:

> +(define (profile-rx profile)
> +  "Return a regular expression that matches PROFILE's name and number."
> +  (make-regexp (string-append "^" (regexp-quote (basename profile))
> +                              "-([0-9]+)")))

OK.

> +(define (profile-number profile)
> +  "Return PROFILE's number. PROFILE should be an absolute filename."

Two spaces after period.  Please write “file name”.

> +  (match:substring (regexp-exec (profile-rx profile)
> +                                (basename (readlink profile))) 1))

Instead write:

  (and=> (regexp-exec ...)
         (cut match:substring <> 1))

So that the thing returns #f when there are is no associated profile
number.  OTOH, does that ever occur?

> +(define (roll-back)
> +  "Roll back to the previous profile."

Please add a ‘profile’ parameter, as for the other functions.

It should be possible to run:

  $ guix-package -p foo --roll-back

> +  (let* ((current-profile-number
> +          (string->number (profile-number %current-profile)))
> +         (previous-profile-number (number->string (1- current-profile-number)))
> +         (previous-profile
> +          (string-append %current-profile "-"
> +                         previous-profile-number "-link")))
> +
> +    (define (switch)
> +      "Switch to the previous generation."

For internal procedures, just use regular comments instead of docstrings.

> +      (simple-format #t "guix-package: switching from generation ~a to ~a~%"
> +                     current-profile-number previous-profile-number)
> +      (delete-file %current-profile)
> +      (symlink previous-profile %current-profile))

It should be based on rename(2) to be atomic.  See the ‘switchLink’
function in Nix for how to do it.

> +    (if (= current-profile-number 1)
> +        (error "there are no other profiles.") ; XXX: handle this error

Here you you use (leave (_ "no other profiles; not rolling back")).

> +        (option '("roll-back") #f #f
> +                (lambda args
> +                  (roll-back)
> +                  (exit 0)))

Instead of calling ‘roll-back’, just do like the other actions:

  (alist-cons 'roll-back #t result)

Then ‘roll-back’ can be called from ‘process-actions’, with the right
profile passed as an argument.  Perhaps other actions should be ignored
when rolling back.  At any rate, you may need to split ‘process-actions’
into several procedures, for readability.

Can you add a test case in ‘tests/guix-package.sh’?

Thanks!

Ludo’.

Re: guix-package --roll-back

[Nikita Karetnikov]

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

>> +  (match:substring (regexp-exec (profile-rx profile)
>> +                                (basename (readlink profile))) 1))

> Instead write:

>   (and=> (regexp-exec ...)
>          (cut match:substring <> 1))

> So that the thing returns #f when there are is no associated profile
> number.  OTOH, does that ever occur?

I doubt so, but I also think that it would be better to keep 'and=>' to
prevent possible problems.

> It should be based on rename(2) to be atomic.  See the ‘switchLink’
> function in Nix for how to do it.

Could you elaborate? Should I call C from Guile?

Nikita


[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: guix-package --roll-back

[Ludovic Courtès]

Nikita Karetnikov <nikita@karetnikov.org> skribis:

>> It should be based on rename(2) to be atomic.  See the ‘switchLink’
>> function in Nix for how to do it.
>
> Could you elaborate? Should I call C from Guile?

No no, Scheme code can use ‘rename-file’, which calls rename(2).

Ludo’.

Re: guix-package --roll-back

[Nikita Karetnikov]

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

>> +(define (roll-back)
>> +  "Roll back to the previous profile."

> Please add a ‘profile’ parameter, as for the other functions.
>
> It should be possible to run:
> 
>   $ guix-package -p foo --roll-back

Should I add any profile-related sanity checks to 'roll-back'?

AFAICT, there are only manifest-specific ones (e.g., 'profile-manifest',
'manifest-packages').

Should 'roll-back' only work for 'guix-profile-*-link' and
'default-*-link'? If not, then we have a problem because 'profile-rx'
can't handle something like '42-custom-profile'. Is it possible to
create such a profile?

Nikita

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: guix-package --roll-back

[Ludovic Courtès]

Hi,

Nikita Karetnikov <nikita@karetnikov.org> skribis:

>>> +(define (roll-back)
>>> +  "Roll back to the previous profile."
>
>> Please add a ‘profile’ parameter, as for the other functions.
>>
>> It should be possible to run:
>> 
>>   $ guix-package -p foo --roll-back
>
> Should I add any profile-related sanity checks to 'roll-back'?

What kind of sanity check?

> Should 'roll-back' only work for 'guix-profile-*-link' and
> 'default-*-link'? If not, then we have a problem because 'profile-rx'
> can't handle something like '42-custom-profile'. Is it possible to
> create such a profile?

Yes, but I think the regexp handles it just fine, no?

Thanks,
Ludo’.

Re: guix-package --roll-back

[Nikita Karetnikov]

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

>>>   $ guix-package -p foo --roll-back

>> Should I add any profile-related sanity checks to 'roll-back'?

> What kind of sanity check?

The one that should check that 'foo' is actually a profile.

>> Should 'roll-back' only work for 'guix-profile-*-link' and
>> 'default-*-link'? If not, then we have a problem because 'profile-rx'
>> can't handle something like '42-custom-profile'. Is it possible to
>> create such a profile?

> Yes, but I think the regexp handles it just fine, no?

It depends on how profiles are stored. It turned out that there are
several issues. By the way, what should I do to create a custom profile?

Both 'profile-rx' and 'profile-number' work fine when '%current-profile'
is used.

scheme@(guile-user)> (define %current-profile "/nix/var/nix/profiles/per-user/root/guix-profile")
scheme@(guile-user)> (profile-number %current-profile)
$2 = "1"
scheme@(guile-user)> (readlink %current-profile)
$3 = "/nix/var/nix/profiles/per-user/root/guix-profile-1-link"

But fail for other inputs.

scheme@(guile-user)> (define profile-1 "/nix/var/nix/profiles/per-user/root/guix-profile-1-link")
scheme@(guile-user)> (profile-number profile-1)
$4 = #f

This implementation of 'profile-number' works for 'profile-1', but it
doesn't work for '%current-profile'.

(define (profile-number-1 profile)
  (and=> (regexp-exec (profile-rx %current-profile)
                      (basename profile)) ; 'readlink' was removed
		 (cut match:substring <> 1)))

scheme@(guile-user)> (profile-number-1 profile-1)
$5 = "1"
scheme@(guile-user)> (profile-number-1 %current-profile)
$6 = #f

Also, both 'profile-rx' and 'profile-number' can't handle custom
profiles.

Here is what I used to create a bogus custom profile.

# cd /home
# mkdir testdir
# cd testdir
# touch env-42
# ln -s env-42 42-custom-profile
# ln -s 42-custom-profile guix-profile

[...]

scheme@(guile-user)> (define custom-profile "/home/testdir/guix-profile")
scheme@(guile-user)> (profile-number custom-profile)
$7 = #f

So, what kinds of profiles should be supported? Maybe it's not possible
to create something like 'custom-profile' and there is no need to
bother.

Nikita

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: guix-package --roll-back

[Ludovic Courtès]

Hi,

Nikita Karetnikov <nikita@karetnikov.org> skribis:

> It depends on how profiles are stored. It turned out that there are
> several issues. By the way, what should I do to create a custom profile?

The only way to manipulate profiles is via ‘guix-package -p xxx’.

> Both 'profile-rx' and 'profile-number' work fine when '%current-profile'
> is used.
>
> scheme@(guile-user)> (define %current-profile "/nix/var/nix/profiles/per-user/root/guix-profile")
> scheme@(guile-user)> (profile-number %current-profile)
> $2 = "1"
> scheme@(guile-user)> (readlink %current-profile)
> $3 = "/nix/var/nix/profiles/per-user/root/guix-profile-1-link"
>
> But fail for other inputs.

That’s expected.

> Here is what I used to create a bogus custom profile.
>
> # cd /home
> # mkdir testdir
> # cd testdir
> # touch env-42
> # ln -s env-42 42-custom-profile
> # ln -s 42-custom-profile guix-profile
>
> [...]
>
> scheme@(guile-user)> (define custom-profile "/home/testdir/guix-profile")
> scheme@(guile-user)> (profile-number custom-profile)
> $7 = #f

Yeah, that’s expected.  Basically, if you do

  guix-package -p /dev/null --roll-back

it should fail with an error message saying that there is no previous
profile or something like that.

> So, what kinds of profiles should be supported? Maybe it's not possible
> to create something like 'custom-profile' and there is no need to
> bother.

There are two ways to use ‘guix-package’:

  1. without -p, it uses the user’s default profile, by default
     /nix/var/nix/profiles/per-user/$USER/guix-profile;

  2. with -p, it uses the specified profile (see tests/guix-package.sh,
     for an example).

Thanks,
Ludo’.

Re: guix-package --roll-back

[Nikita Karetnikov]

[-- Attachment #1.1: Type: text/plain, Size: 732 bytes --]

Hi,

I'm attaching a slightly modified version.

> Yeah, that’s expected.  Basically, if you do

>   guix-package -p /dev/null --roll-back

> it should fail with an error message saying that there is no previous
> profile or something like that.

'profile-number' will fail if I call it with a bogus file name.

For instance:

scheme@(guile-user)> (profile-number "/foo/bar")
ERROR: In procedure readlink:
ERROR: In procedure readlink: No such file or directory

What should I use to handle this error?  Please show an example.  The
ones from the manual aren't helpful.

I don't understand how to add a command-line option that should accept
an optional argument.  I commented out my attempts.

Nikita


[-- Attachment #1.2: guix-package.diff --]
[-- Type: text/x-diff, Size: 5438 bytes --]

--- guix-package-orig	2013-01-09 18:28:03.000000000 +0000
+++ guix-package	2013-01-09 18:38:23.000000000 +0000
@@ -13,6 +13,7 @@
 !#
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2012, 2013 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2013 Nikita Karetnikov <nikita@karetnikov.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -39,6 +40,7 @@
   #:use-module (ice-9 ftw)
   #:use-module (ice-9 format)
   #:use-module (ice-9 match)
+  #:use-module (ice-9 optargs)
   #:use-module (ice-9 regex)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-11)
@@ -88,13 +90,14 @@
     (_
      (error "unsupported manifest format" manifest))))
 
+(define (profile-rx profile)
+  "Return a regular expression that matches PROFILE's name and number."
+  (make-regexp (string-append "^" (regexp-quote (basename profile))
+                              "-([0-9]+)")))
+
 (define (latest-profile-number profile)
   "Return the identifying number of the latest generation of PROFILE.
 PROFILE is the name of the symlink to the current generation."
-  (define %profile-rx
-    (make-regexp (string-append "^" (regexp-quote (basename profile))
-                                "-([0-9]+)")))
-
   (define* (scandir name #:optional (select? (const #t))
                     (entry<? (@ (ice-9 i18n) string-locale<?)))
     ;; XXX: Bug-fix version introduced in Guile v2.0.6-62-g139ce19.
@@ -130,7 +133,7 @@
              (sort files entry<?))))
 
   (match (scandir (dirname profile)
-                  (cut regexp-exec %profile-rx <>))
+                  (cut regexp-exec (profile-rx profile) <>))
     (#f                                         ; no profile directory
      0)
     (()                                         ; no profiles
@@ -138,7 +141,7 @@
     ((profiles ...)                             ; former profiles around
      (let ((numbers (map (compose string->number
                                   (cut match:substring <> 1)
-                                  (cut regexp-exec %profile-rx <>))
+                                  (cut regexp-exec (profile-rx profile) <>))
                          profiles)))
        (fold (lambda (number highest)
                (if (> number highest)
@@ -178,6 +181,41 @@
                                      packages)
                                 #:modules '((guix build union))))
 
+(define (profile-number profile)
+  "Return PROFILE's number.  An absolute file name must be used."
+  (and=> (regexp-exec (profile-rx profile)
+                      (basename (readlink profile)))
+         (cut match:substring <> 1)))
+
+(define* (roll-back #:optional profile)
+  "Roll back to the previous profile."
+  (let* ((current-profile-number
+          (string->number (profile-number (or profile %current-profile))))
+         (previous-profile-number (number->string (1- current-profile-number)))
+         (previous-profile
+          (string-append (or profile %current-profile) "-"
+                         previous-profile-number "-link"))
+         (manifest (string-append previous-profile "/manifest")))
+
+    (define (switch-link)
+      ;; Switch to the previous generation.
+      (let ((tmp-profile (string-append (dirname (or profile %current-profile))
+                                        "/tmp-"
+                                        (basename previous-profile))))
+
+        (simple-format #t "guix-package: switching from generation ~a to ~a~%"
+                       current-profile-number previous-profile-number)
+        (symlink previous-profile tmp-profile)
+        (rename-file tmp-profile (or profile %current-profile))))
+
+    (if (equal? (map (cut file-exists? <>)
+                     (list previous-profile manifest))
+                '(#t #t))
+        (switch-link)
+        (leave (_ (string-append
+                   "guix-package: previous profile doesn't exist; "
+                   "not rolling back~%"))))))
+
 \f
 ;;;
 ;;; Command-line options.
@@ -202,6 +240,8 @@
   (display (_ "
   -n, --dry-run          show what would be done without actually doing it"))
   (display (_ "
+      --roll-back        roll back to the previous generation"))
+  (display (_ "
       --bootstrap        use the bootstrap Guile to build the profile"))
   (display (_ "
       --verbose          produce verbose output"))
@@ -236,6 +276,25 @@
         (option '(#\r "remove") #t #f
                 (lambda (opt name arg result)
                   (alist-cons 'remove arg result)))
+
+        ;; (option '("roll-back") #f #t
+        ;;         (lambda (opt name arg result)
+        ;;           (roll-back (or arg #f))
+        ;;           (exit 0)))
+
+                ;; (lambda (opt name arg result)
+                ;;   (alist-cons 'roll-back arg result)))
+
+                ;; (lambda (opt name arg result)
+                ;;   (cons `(query roll-back ,(or arg #f))
+                ;;         result)))
+
+                ;; (lambda (opt name arg result)
+                ;;   (alist-cons 'roll-back (or arg #f) result)))
+
+                ;; (lambda (opt name arg result)
+                ;;   (alist-cons 'roll-back (or arg "") result)))
+
         (option '(#\p "profile") #t #f
                 (lambda (opt name arg result)
                   (alist-cons 'profile arg

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: guix-package --roll-back

[Nikita Karetnikov]

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

Hi,

I've forgotten to ask about the following.

  (equal? (map (cut file-exists? <>)
               (list previous-profile manifest))
          '(#t #t))

Is it enough to check that 'previous-profile' and 'manifest' exist.
Should it actually check 'manifest'?

Nikita

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: guix-package --roll-back

[Ludovic Courtès]

Hi,

Nikita Karetnikov <nikita@karetnikov.org> skribis:

> I'm attaching a slightly modified version.

Thanks!

>> Yeah, that’s expected.  Basically, if you do
>
>>   guix-package -p /dev/null --roll-back
>
>> it should fail with an error message saying that there is no previous
>> profile or something like that.
>
> 'profile-number' will fail if I call it with a bogus file name.
>
> For instance:
>
> scheme@(guile-user)> (profile-number "/foo/bar")
> ERROR: In procedure readlink:
> ERROR: In procedure readlink: No such file or directory
>
> What should I use to handle this error?  Please show an example.  The
> ones from the manual aren't helpful.

At the command-line level, an example would be:

  $ guix-package -p /dev/null --roll-back ; echo $?
  error: no previous profile to roll back to
  1

(Please, add this example to tests/guix-package.sh.)

Internally, I would expect this ‘profile-number’ to return #f.

> I don't understand how to add a command-line option that should accept
> an optional argument.  I commented out my attempts.

  (option '("foo")
          #f  ; no required argument
          #t  ; an optional argument
          (lambda (opt name arg result)
            ;; Function called when ‘--foo’ is passed.
            ;; ARG is #f or the optional argument.
            ;; ...
            ))

See SRFI-37 in Guile’s manual.

> +(define (profile-rx profile)

Perhaps ‘profile-regexp’ (since it’s a global variable.)

> +(define (profile-number profile)
> +  "Return PROFILE's number.  An absolute file name must be used."

I think it works even if PROFILE is not an absolute file name, no?

Just like ‘latest-profile-number’ returns 0 if there was no file
matching %PROFILE-RX, ‘profile-number’ should return 0.

So you’d want to enclose the whole body like this:

  (or (and=> ...) 0)

> +  (and=> (regexp-exec (profile-rx profile)
> +                      (basename (readlink profile)))
> +         (cut match:substring <> 1)))

First, you need to move ‘readlink’ above, in ‘false-if-exception’.

Second, it should return a number, not a string.

So the result should be along these lines:

  (let ((target (false-if-exception (readlink profile))))
    (and target
         (and=> (regexp-exec (profile-regexp profile) (basename target))
                (compose string->number (cut match:substring <> 1))

> +(define* (roll-back #:optional profile)

Make ‘profile’ mandatory.

> +  "Roll back to the previous profile."

Rather: “Roll back to the previous generation of PROFILE.”

> +  (let* ((current-profile-number
> +          (string->number (profile-number (or profile %current-profile))))

The ‘or’ and ‘string->number’ can now be removed.
Name the variable just ‘number’, because long names for local vars
hinder clarity.

Now you need to introduce an

  (if number
      ;; then do the thing
      (format (current-error-port) (_ "error: ‘~a’ is not a valid profile~%")
              profile))

> +         (previous-profile-number (number->string (1- current-profile-number)))
> +         (previous-profile
> +          (string-append (or profile %current-profile) "-"
> +                         previous-profile-number "-link"))
> +         (manifest (string-append previous-profile "/manifest")))
> +
> +    (define (switch-link)
> +      ;; Switch to the previous generation.
> +      (let ((tmp-profile (string-append (dirname (or profile %current-profile))
> +                                        "/tmp-"
> +                                        (basename previous-profile))))
> +
> +        (simple-format #t "guix-package: switching from generation ~a to ~a~%"
> +                       current-profile-number previous-profile-number)
> +        (symlink previous-profile tmp-profile)
> +        (rename-file tmp-profile (or profile %current-profile))))

Looks good.
However, just use ‘format’, and also make sure that messages and i18n’d:

  (format #t (_ "switching from generation ~a to ~a...~%")
          number previous-number)

> +    (if (equal? (map (cut file-exists? <>)
> +                     (list previous-profile manifest))
> +                '(#t #t))

I think you just need to check for ‘previous-profile’.  If it turns out
not to be a genuine profile, that’s not our problem.

Besides, this is a mundane (and inefficient) way of writing:

  (every file-exists? (list previous-profile manifest))

or simply:

  (and (file-exists? previous-profile)
       (file-exists? manifest))

:-)

> +        (switch-link)
> +        (leave (_ (string-append
> +                   "guix-package: previous profile doesn't exist; "
> +                   "not rolling back~%"))))))

Strip “guix-package: ”.

Woow, this was long.

Thanks,
Ludo’.

Re: guix-package --roll-back

[Nikita Karetnikov]

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

> See SRFI-37 in Guile’s manual.

Yeah, I've already checked it, but it's very brief.

I created a simple function to test command-line options, but I'm having
the same problems.  Here is the function:

(define* (foo #:optional arg)
  (if arg
      (simple-format #t "Just ~a~%" arg)
      (simple-format #t "Nothing~%")))

The following version works:

(option '("foo") #f #t
        (lambda (opt name arg result)
          (foo arg)
          (exit 0)))

# ./pre-inst-env guix-package --foo=42
Just 42

# ./pre-inst-env guix-package --foo
Nothing

This one doesn't work at all:

(option '("foo") #f #t
        (lambda (opt name arg result)
          (alist-cons 'foo arg result)))

Actually, the above helped me to understand that we want '--roll-back'
to behave differently than '--foo'.  '--roll-back' shouldn't accept any
options at all, but it should somehow get the argument of '--profile'.
How can I do it?

Nikita

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: guix-package --roll-back

[Ludovic Courtès]

Hi,

Nikita Karetnikov <nikita@karetnikov.org> skribis:

> This one doesn't work at all:
>
> (option '("foo") #f #t
>         (lambda (opt name arg result)
>           (alist-cons 'foo arg result)))

It actually does, but it has no side effect.

‘args-fold’ uses a common functional programming pattern, whereby the
functions called when an option is encountered just /contribute/ to the
resulting value.

Here’s an example:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> (use-modules(srfi srfi-37))
scheme@(guile-user)> (define options
                       (list (option '("foo") #f #t
                               (lambda (opt name arg result)
                                  (if arg
                                      (cons arg result)
                                      'nothing)))))
scheme@(guile-user)> (args-fold '("--foo") options (const #f) (const #f) '(something))
$19 = nothing
scheme@(guile-user)> (args-fold '("--foo=42") options (const #f) (const #f) '(something))
$20 = ("42" something)
scheme@(guile-user)> (args-fold '("--foo=42" "--foo=3") options (const #f) (const #f) '(something))
$21 = ("3" "42" something)
scheme@(guile-user)> (args-fold '("--foo=42" "--foo=3" "--foo") options (const #f) (const #f) '(something))
$22 = nothing
--8<---------------cut here---------------end--------------->8---

In guix-package, ‘parse-options’ returns an alist (list of pairs) that
indicates the actions to be performed, etc.  IOW, ‘args-fold’ is a tool
to build “translators” from command-line options to an internal
representation of those options.

> Actually, the above helped me to understand that we want '--roll-back'
> to behave differently than '--foo'.  '--roll-back' shouldn't accept any
> options at all, but it should somehow get the argument of '--profile'.
> How can I do it?

When ‘--profile’ is passed, the result of ‘parse-options’ contains a
pair whose key is ‘profile’.  The (assoc-ref opts 'profile) calls that
you see retrieve the argument given to ‘--profile’.

Ludo’.

Re: guix-package --roll-back

[Nikita Karetnikov]

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

> It actually does, but it has no side effect.

I know that printing is a side effect.  Why do you recommend to use
'alist-cons'?  If it's not possible to display anything, then
'roll-back' won't be able to print the following messages.

(simple-format #t "guix-package: switching from generation ~a to ~a~%"
               current-profile-number previous-profile-number)

(leave (_ (string-append
           "guix-package: previous profile doesn't exist; "
           "not rolling back~%")))

Actually, if the above is correct, then it won't be possible to use
'roll-back' at all.  (Because it creates symlinks.)

I think that I misunderstood you.  Please elaborate.

> When ‘--profile’ is passed, the result of ‘parse-options’ contains a
> pair whose key is ‘profile’.  The (assoc-ref opts 'profile) calls that
> you see retrieve the argument given to ‘--profile’.

It's clear.  But where should I call (assoc-ref opts 'profile)?  Should
I do it in 'process-query'?

Nikita

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: guix-package --roll-back

[Ludovic Courtès]

Nikita Karetnikov <nikita@karetnikov.org> skribis:

>> It actually does, but it has no side effect.
>
> I know that printing is a side effect.  Why do you recommend to use
> 'alist-cons'?  If it's not possible to display anything, then
> 'roll-back' won't be able to print the following messages.

It’s just the /option handler/ (the procedure passed to SRFI-37
‘option’) that should not print anything.  It is then OK to print
things, of course, like is done for other options.

Check how the other options are handled: during ‘args-fold’, the option
handler just adds to RESULT an element saying that this option was
passed.  Once ‘args-fold’ has completed, we check its result: if it
contains a pair whose key is ‘install’, then it means we want to install
something, and so on.

>> When ‘--profile’ is passed, the result of ‘parse-options’ contains a
>> pair whose key is ‘profile’.  The (assoc-ref opts 'profile) calls that
>> you see retrieve the argument given to ‘--profile’.
>
> It's clear.  But where should I call (assoc-ref opts 'profile)?  Should
> I do it in 'process-query'?

In ‘process-actions’ more precisely, because roll-back is an action, not
a query.  And ‘process-actions’ already does that.

HTH,
Ludo’.

Re: guix-package --roll-back

[Nikita Karetnikov]

[-- Attachment #1.1: Type: text/plain, Size: 1096 bytes --]

Hi,

I changed 'roll-back', but didn't add the command-line option.  Could
you add it?

Why do these lines raise the "non-literal format string" warning?

+        (format (current-error-port)
+                "error: '~a' is not a valid profile~%"
+                profile)

Also, is it possible to remove nested if statements?

+    (if (= number 0)
+        (format (current-error-port)
+                "error: '~a' is not a valid profile~%"
+                profile)
+        (if (file-exists? previous-profile)
+            (switch-link)
+            (format (current-error-port)
+                    (string-append "error: previous profile doesn't exist; "
+                                   "not rolling back~%"))))))

> I think it works even if PROFILE is not an absolute file name, no?

Maybe I misunderstood, but the following doesn't work.

scheme@(guile-user)> (define %current-profile "/nix/var/nix/profiles/per-user/root/guix-profile")
scheme@(guile-user)> (profile-number %current-profile)
$1 = 1
scheme@(guile-user)> (profile-number (basename %current-profile))
$2 = 0

Nikita


[-- Attachment #1.2: guix-package.diff --]
[-- Type: text/x-diff, Size: 4743 bytes --]

--- guix-package-orig	2013-01-16 20:56:13.000000000 +0000
+++ guix-package	2013-01-16 21:05:09.000000000 +0000
@@ -13,6 +13,7 @@
 !#
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2012, 2013 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2013 Nikita Karetnikov <nikita@karetnikov.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -89,13 +90,14 @@
     (_
      (error "unsupported manifest format" manifest))))
 
+(define (profile-regexp profile)
+  "Return a regular expression that matches PROFILE's name and number."
+  (make-regexp (string-append "^" (regexp-quote (basename profile))
+                              "-([0-9]+)")))
+
 (define (latest-profile-number profile)
   "Return the identifying number of the latest generation of PROFILE.
 PROFILE is the name of the symlink to the current generation."
-  (define %profile-rx
-    (make-regexp (string-append "^" (regexp-quote (basename profile))
-                                "-([0-9]+)")))
-
   (define* (scandir name #:optional (select? (const #t))
                     (entry<? (@ (ice-9 i18n) string-locale<?)))
     ;; XXX: Bug-fix version introduced in Guile v2.0.6-62-g139ce19.
@@ -131,16 +133,17 @@
              (sort files entry<?))))
 
   (match (scandir (dirname profile)
-                  (cut regexp-exec %profile-rx <>))
+                  (cut regexp-exec (profile-regexp profile) <>))
     (#f                                         ; no profile directory
      0)
     (()                                         ; no profiles
      0)
     ((profiles ...)                             ; former profiles around
-     (let ((numbers (map (compose string->number
-                                  (cut match:substring <> 1)
-                                  (cut regexp-exec %profile-rx <>))
-                         profiles)))
+     (let ((numbers
+            (map (compose string->number
+                          (cut match:substring <> 1)
+                          (cut regexp-exec (profile-regexp profile) <>))
+                 profiles)))
        (fold (lambda (number highest)
                (if (> number highest)
                    number
@@ -179,6 +182,41 @@
                                      packages)
                                 #:modules '((guix build union))))
 
+(define (profile-number profile)
+  "Return PROFILE's number or 0.  An absolute file name must be used."
+  (or (and=> (false-if-exception (regexp-exec (profile-regexp profile)
+                                              (basename (readlink profile))))
+             (compose string->number (cut match:substring <> 1)))
+      0))
+
+(define (roll-back profile)
+  "Roll back to the previous generation of PROFILE."
+  (let* ((number (profile-number profile))
+         (previous-number (1- number))
+         (previous-profile
+          (string-append profile "-" (number->string previous-number) "-link"))
+         (manifest (string-append previous-profile "/manifest")))
+
+    (define (switch-link)
+      (let ((tmp-profile (string-append (dirname profile)
+                                        "/tmp-"
+                                        (basename previous-profile))))
+
+        (format #t "switching from generation ~a to ~a~%"
+                number previous-number)
+        (symlink previous-profile tmp-profile)
+        (rename-file tmp-profile profile)))
+
+    (if (= number 0)
+        (format (current-error-port)
+                "error: '~a' is not a valid profile~%"
+                profile)
+        (if (file-exists? previous-profile)
+            (switch-link)
+            (format (current-error-port)
+                    (string-append "error: previous profile doesn't exist; "
+                                   "not rolling back~%"))))))
+
 \f
 ;;;
 ;;; Command-line options.
@@ -203,6 +241,8 @@
   (display (_ "
   -n, --dry-run          show what would be done without actually doing it"))
   (display (_ "
+      --roll-back        roll back to the previous generation"))
+  (display (_ "
       --bootstrap        use the bootstrap Guile to build the profile"))
   (display (_ "
       --verbose          produce verbose output"))
@@ -237,6 +277,9 @@
         (option '(#\r "remove") #t #f
                 (lambda (opt name arg result)
                   (alist-cons 'remove arg result)))
+        ;; (option '("roll-back") #f #f
+        ;;         (lambda (opt name arg result)
+        ;;           (alist-cons 'roll-back arg result)))
         (option '(#\p "profile") #t #f
                 (lambda (opt name arg result)
                   (alist-cons 'profile arg

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: guix-package --roll-back

[Ludovic Courtès]

Hi!

Nikita Karetnikov <nikita@karetnikov.org> skribis:

> I changed 'roll-back', but didn't add the command-line option.  Could
> you add it?

Yes, I just did it in 24e262f, along with a test case and documentation.

Thanks a lot for taking the time to work on this, I appreciate.

> Why do these lines raise the "non-literal format string" warning?
>
> +        (format (current-error-port)
> +                "error: '~a' is not a valid profile~%"
> +                profile)

Hmm, could be a bug, but I don’t seem to have them here.

> Also, is it possible to remove nested if statements?

Yes, with ‘cond’ or ‘match’.

I think we now have something that works well.  Commit 9241172 slightly
changes ‘roll-back’ to support roll-back when a past generation is
missing (which could happen when the user deletes it.)

So now I think I’m almost ready to tag 0.1 later tonight!

Thanks!

Ludo’.

Re: guix-package --roll-back

[Nikita Karetnikov]

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

> I think we now have something that works well.

It doesn't work.

# ls -l /nix/var/nix/profiles/per-user/root/

[...]

guix-profile -> /nix/var/nix/profiles/per-user/root/guix-profile-16-link
guix-profile-15-link -> /nix/store/05gws0ijjf8bczrrsi8mrhh16xii0vis-user-environment
guix-profile-16-link -> /nix/store/aslx7hfjzkzjr73hbyk25997aap3avcb-user-environment

[...]

# ./pre-inst-env guix-package --roll-back
error: no previous profile; not rolling back

# ./pre-inst-env guix-package --roll-back
      -p /nix/var/nix/profiles/per-user/root/guix-profile
error: no previous profile; not rolling back

I'll try to investigate later.

Nikita

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: guix-package --roll-back

[Ludovic Courtès]

Nikita Karetnikov <nikita@karetnikov.org> skribis:

>> I think we now have something that works well.
>
> It doesn't work.
>
> # ls -l /nix/var/nix/profiles/per-user/root/
>
> [...]
>
> guix-profile -> /nix/var/nix/profiles/per-user/root/guix-profile-16-link
> guix-profile-15-link -> /nix/store/05gws0ijjf8bczrrsi8mrhh16xii0vis-user-environment
> guix-profile-16-link -> /nix/store/aslx7hfjzkzjr73hbyk25997aap3avcb-user-environment
>
> [...]
>
> # ./pre-inst-env guix-package --roll-back
> error: no previous profile; not rolling back

I was driven by curiosity, and looked at it: a simple thinko fixed in 6766815.

Thanks!

Ludo’.