[bug#74481] [PATCH 1/2] git: Catch Git errors when updating cached checkout.

["Ludovic Courtès" <ludo@gnu.org>]

Hi,

Simon Tournier <zimon.toutoune@gmail.com> skribis:

> * guix/git.scm (resolve-reference): Catch Git error when reference does not
> exist and return #false.
> (switch-to-ref): Adjust.
> (update-cached-checkout)[close-and-clean!]: New helper.
> Catch Git error when reference does not exist and warn.
> Return #false values when reference does not exist.
>
> Change-Id: If6e244fe40ebee978ec8de51a6a68bcbd4a2c79e

[...]

>  (define (resolve-reference repository ref)
>    "Resolve the branch, commit or tag specified by REF, and return the
> -corresponding Git object."
> +corresponding Git object. Return #false if REF is not found."
>    (let resolve ((ref ref))
>      (match ref
>        (('branch . branch)
> @@ -281,9 +281,12 @@ (define (resolve-reference repository ref)
>           ;; can't be sure it's available.  Furthermore, 'string->oid' used to
>           ;; read out-of-bounds when passed a string shorter than 40 chars,
>           ;; which is why we delay calls to it below.
> -         (if (< len 40)
> -             (object-lookup-prefix repository (string->oid commit) len)
> -             (object-lookup repository (string->oid commit)))))
> +         (catch 'git-error
> +           (lambda ()
> +             (if (< len 40)
> +                 (object-lookup-prefix repository (string->oid commit) len)
> +                 (object-lookup repository (string->oid commit))))
> +           (lambda _ #f))))
>        (('tag-or-commit . str)
>         (cond ((and (string-contains str "-g")
>                     (match (string-split str #\-)
> @@ -300,7 +303,10 @@ (define (resolve-reference repository ref)
>                => (lambda (commit) (resolve `(commit . ,commit))))
>               ((or (> (string-length str) 40)
>                    (not (string-every char-set:hex-digit str)))
> -              (resolve `(tag . ,str)))      ;definitely a tag
> +              (catch 'git-error         ;definitely a tag
> +                (lambda ()
> +                  (resolve `(tag . ,str)))
> +                (lambda _ #f)))

Sorry for not replynig earlier.  I would avoid such a change: it changes
the semantics of ‘resolve-reference’ in a non-trivial and hard-to-test
way.

Instead, since the goal is to address a problem that’s specific to
importers (or to one importer), I would suggest making a local change in
the importer itself, or in code that is shared among importers only.

(This comment applies to this entire patch.)

Ludo’.