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.