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

From: Ludovic Courtès <[email-masked]>
Date: Tue, 06 Jun 2023 17:42:00 +0200

Hello!

Simon Tournier <[email-masked]> skribis:

> On Mon, 05 Jun 2023 at 14:45, Ludovic Courtès <[email-masked]> wrote:
>> Simon Tournier <[email-masked]> 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.

I thought it would be more appropriate to error out as soon as we have a
Git problem, rather than print a warning and emit an incorrect hash.

I understand that, when using ‘--recursive’, this means aborting the
whole process without producing anything.  But maybe that’s better that
producing an incorrect (origin …) form?

Now, I don’t use ‘guix import go -r’ so it’s possible that I don’t
understand the scenario being considered here!

Thanks,
Ludo’.