[bug#63647] [PATCH 0/2] Fix annoyance with "guix import go"

[Simon Tournier]

Hi Ludo,

On Mon, 05 Jun 2023 at 14:45, Ludovic Courtès <ludo@gnu.org> wrote:
> Simon Tournier <zimon.toutoune@gmail.com> skribis:
>
>> * guix/import/go.scm (git-checkout-hash): Catch Git error.
>
> [...]
>
>> +  (catch 'git-error
>> +    (lambda _
>> +      (let-values (((checkout commit _)
>> +                    (parameterize ((%repository-cache-directory cache))
>> +                      (update-cached-checkout url
>> +                                              #:ref
>> +                                              `(tag-or-commit . 
>> ,reference)))))
>> +        (file-hash* checkout #:algorithm algorithm #:recursive? #true)))
>> +    (lambda (key error . rest)
>> +      (warning (G_ "Git error: ~a in ~a~%") (git-error-message error) url)
>> +      (nix-base32-string->bytevector
>> +       "0000000000000000000000000000000000000000000000000000"))))
>
> I’d rather let the exception through.  How about adding
> ‘with-git-error-handling’ at the UI level, in (guix scripts import go)?

What do you mean by “let the exception through”?  It seems better to be
non-blocking and thus catch the exception then raise a meaningful
warning; it’s required when running with the option recursive.

Well, maybe an improvement could be in the addition of some
’report-git-warning’ and/or ’with-git-error-handling*’, in (guix git);
hum, I do not know.

Last, considering that the module (guix import go) already contains 4 UI
messages (G_), trying to move this warning about Git to (guix scripts
import go) will add some complexity – re-raise the exception 2 or 3
times, IIUC – and thus it will not change much about the UI, IMHO.  I
mean, such move should be for all the messages or nothing.

Cheers,
simon