Re: guix-package --roll-back

[ludo@gnu.org (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’.