[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: guix-package --roll-back
From: |
Ludovic Courtès |
Subject: |
Re: guix-package --roll-back |
Date: |
Thu, 10 Jan 2013 23:26:58 +0100 |
User-agent: |
Gnus/5.130005 (Ma Gnus v0.5) Emacs/24.2 (gnu/linux) |
Hi,
Nikita Karetnikov <address@hidden> 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, 2013/01/01
- Re: guix-package --roll-back, Nikita Karetnikov, 2013/01/03
- Re: guix-package --roll-back, Ludovic Courtès, 2013/01/03
- Re: guix-package --roll-back, Nikita Karetnikov, 2013/01/04
- Re: guix-package --roll-back, Ludovic Courtès, 2013/01/05
- Re: guix-package --roll-back, Nikita Karetnikov, 2013/01/09
- Re: guix-package --roll-back, Nikita Karetnikov, 2013/01/10
- Re: guix-package --roll-back,
Ludovic Courtès <=
- Re: guix-package --roll-back, Nikita Karetnikov, 2013/01/11
- Re: guix-package --roll-back, Ludovic Courtès, 2013/01/11
- Re: guix-package --roll-back, Nikita Karetnikov, 2013/01/12
- Re: guix-package --roll-back, Ludovic Courtès, 2013/01/13
- Re: guix-package --roll-back, Nikita Karetnikov, 2013/01/16
- Re: guix-package --roll-back, Ludovic Courtès, 2013/01/17
- Re: guix-package --roll-back, Nikita Karetnikov, 2013/01/21
- Re: guix-package --roll-back, Ludovic Courtès, 2013/01/22