diff mbox series

[bug#65352,v2] DRAFT git: Avoid touching the network unless needed in 'reference-available?'.

Message ID 32d3fb5066e0b20e200dabef0fba897634e21dda.1694009405.git.zimon.toutoune@gmail.com
State New
Headers show
Series [bug#65352,v2] DRAFT git: Avoid touching the network unless needed in 'reference-available?'. | expand

Commit Message

Simon Tournier Sept. 6, 2023, 2:17 p.m. UTC
Follow-up of 756e336fa008c2469b4a7317ad5c641ed48f25d6 fixing the issue.

* guix/git/scm (reference-available?): Address case by case to determine
whether the reference exists in the local Git checkout.
---
 guix/git.scm | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

Hi,

Here a draft about what I think is the correct solution.

Well, the tests we have talked about are all passing.

Let me know what you think.

Cheers,
simon



base-commit: 6113e0529d61df7425f64e30a6bf77f7cfdfe5a5

Comments

Ludovic Courtès Sept. 13, 2023, 8:16 p.m. UTC | #1
Hi,

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

> Follow-up of 756e336fa008c2469b4a7317ad5c641ed48f25d6 fixing the issue.
>
> * guix/git/scm (reference-available?): Address case by case to determine
> whether the reference exists in the local Git checkout.

[...]

>  (define (reference-available? repository ref)
>    "Return true if REF, a reference such as '(commit . \"cabba9e\"), is
>  definitely available in REPOSITORY, false otherwise."
> -  ;; Note: this must not rely on 'resolve-reference', as that procedure always
> -  ;; resolves the references for branch names such as master.  The semantic we
> -  ;; want here is that unless the reference is exact (e.g. a commit), the
> -  ;; reference should not be considered available, as it could have changed on
> -  ;; the remote.
>    (match ref
> -    ((or ('commit . commit)
> -         ('tag-or-commit . (? commit-id? commit)))
> -     (let ((len (string-length commit))
> -           (oid (string->oid commit)))
> -       (false-if-git-not-found
> -        (->bool (if (< len 40)
> -                    (object-lookup-prefix repository oid len OBJ-COMMIT)
> -                    (commit-lookup repository oid))))))
> +    (('commit . (? commit-id? commit))
> +     (let ((oid (string->oid commit)))
> +       (->bool (commit-lookup repository oid))))
> +    ((or ('tag . str)
> +         ('tag-or-commit . str))
> +     (false-if-git-not-found
> +      (->bool (resolve-reference repository ref))))

IIUC, the differences compared to what we had are:

  1. 'tag references are now handled on the fast path
     (‘reference-available?’ can return #t);

  2. short commit strings are now always on the slow path
     (‘reference-available?’ always returns #f).

Is that correct?

It would be nice to have #1 without #2.

>      (_
> +     ;; For the others REF as branch or symref, the REF cannot be available

“For other values of REF such as branch or symref, the target is by
definition unavailable locally.”

Thanks,
Ludo’.
diff mbox series

Patch

diff --git a/guix/git.scm b/guix/git.scm
index 1cb87a45607b..1b3355109e42 100644
--- a/guix/git.scm
+++ b/guix/git.scm
@@ -5,6 +5,7 @@ 
 ;;; Copyright © 2021 Marius Bakke <marius@gnu.org>
 ;;; Copyright © 2022 Maxime Devos <maximedevos@telenet.be>
 ;;; Copyright © 2023 Tobias Geerinckx-Rice <me@tobias.gr>
+;;; Copyright © 2023 Simon Tournier <zimon.toutoune@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -360,21 +361,16 @@  (define-syntax-rule (false-if-git-not-found exp)
 (define (reference-available? repository ref)
   "Return true if REF, a reference such as '(commit . \"cabba9e\"), is
 definitely available in REPOSITORY, false otherwise."
-  ;; Note: this must not rely on 'resolve-reference', as that procedure always
-  ;; resolves the references for branch names such as master.  The semantic we
-  ;; want here is that unless the reference is exact (e.g. a commit), the
-  ;; reference should not be considered available, as it could have changed on
-  ;; the remote.
   (match ref
-    ((or ('commit . commit)
-         ('tag-or-commit . (? commit-id? commit)))
-     (let ((len (string-length commit))
-           (oid (string->oid commit)))
-       (false-if-git-not-found
-        (->bool (if (< len 40)
-                    (object-lookup-prefix repository oid len OBJ-COMMIT)
-                    (commit-lookup repository oid))))))
+    (('commit . (? commit-id? commit))
+     (let ((oid (string->oid commit)))
+       (->bool (commit-lookup repository oid))))
+    ((or ('tag . str)
+         ('tag-or-commit . str))
+     (false-if-git-not-found
+      (->bool (resolve-reference repository ref))))
     (_
+     ;; For the others REF as branch or symref, the REF cannot be available
      #f)))
 
 (define (clone-from-swh url tag-or-commit output)