Package: guix;
Reported by: Ricardo Wurmus <rekado <at> elephly.net>
Date: Fri, 11 Dec 2020 21:04:02 UTC
Severity: normal
Done: Ludovic Courtès <ludo <at> gnu.org>
Bug is archived. No further changes may be made.
Message #35 received at 45187 <at> debbugs.gnu.org (full text, mbox):
From: Kyle Meyer <kyle <at> kyleam.com> To: Ludovic Courtès <ludo <at> gnu.org> Cc: Ricardo Wurmus <rekado <at> elephly.net>, 45187 <at> debbugs.gnu.org, Marius Bakke <marius <at> gnu.org> Subject: Re: bug#45187: git download defaults to origin/master Date: Fri, 09 Apr 2021 23:50:13 -0400
Ludovic Courtès writes:
> Kyle Meyer <kyle <at> kyleam.com> skribis:
[...]
>> git-checkout make-git-checkout
>> git-checkout?
>> (url git-checkout-url)
>> - (branch git-checkout-branch (default "master"))
>> + (branch git-checkout-branch (default #f))
>> + (symref git-checkout-symref (default "HEAD"))
>
> I know it’s established Git jargon, but “symref” looks obscure to me.
> I find it OK for ‘update-cached-checkout’, because it’s an “internal”
> procedure for die-hard hackers, but a bit ugly here.
Agreed. As mentioned upthread, I don't like it either.
> Another option would be to not add this ‘symref’ field and instead, when
> ‘branch’ and ‘commit’ are both #f, translate that to '(symref . "HEAD").
>
> WDYT?
Thanks for the suggestion. Dropping the field and translating
downstream sounds good to me.
Working through the patch update, I ended up moving things even more
internally, making update-cached-checkout translate the empty list into
(symref . "refs/remotes/origin/HEAD") for resolve-reference. I don't
think there will be a use for symrefs other than HEAD, so it seemed
reasonable to add the special case (or rather shift the special case all
the way down into update-cached-checkout). Let me know if you'd rather
use a (symref . "HEAD") default for update-cached-checkout.
> The rest of the patch LGTM, thanks!
Thanks for the review.
-- >8 --
Subject: [PATCH v2] git: Update cached checkout to the remote HEAD by default.
Fixes <https://bugs.gnu.org/45187>.
Reported by Ricardo Wurmus <rekado <at> elephly.net>.
update-cached-checkout hard codes "master" as the default branch, leading to a
failure when the clone doesn't have a "master" branch. Instead use the remote
HEAD symref as an indicator of what the primary branch is.
* guix/git.scm (resolve-reference): Support resolving symrefs.
(update-cached-checkout, latest-repository-commit): Change the default for REF
to the empty list and translate it to the remote HEAD symref.
(<git-checkout>): Change branch field's default to #f.
(git-checkout-compiler): When branch and commit fields are both #f, call
latest-repository-commit* with the empty list as the ref.
---
guix/git.scm | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/guix/git.scm b/guix/git.scm
index 1820036f25..5d9f0bf205 100644
--- a/guix/git.scm
+++ b/guix/git.scm
@@ -1,6 +1,7 @@
;;; GNU Guix --- Functional package management for GNU
;;; Copyright © 2017, 2020 Mathieu Othacehe <m.othacehe <at> gmail.com>
;;; Copyright © 2018, 2019, 2020, 2021 Ludovic Courtès <ludo <at> gnu.org>
+;;; Copyright © 2021 Kyle Meyer <kyle <at> kyleam.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -209,6 +210,9 @@ (define (resolve-reference repository ref)
(let ((oid (reference-target
(branch-lookup repository branch BRANCH-REMOTE))))
(object-lookup repository oid)))
+ (('symref . symref)
+ (let ((oid (reference-name->oid repository symref)))
+ (object-lookup repository oid)))
(('commit . commit)
(let ((len (string-length commit)))
;; 'object-lookup-prefix' appeared in Guile-Git in Mar. 2018, so we
@@ -340,7 +344,7 @@ (define (delete-checkout directory)
(define* (update-cached-checkout url
#:key
- (ref '(branch . "master"))
+ (ref '())
recursive?
(check-out? #t)
starting-commit
@@ -356,6 +360,7 @@ (define* (update-cached-checkout url
REF is pair whose key is [branch | commit | tag | tag-or-commit ] and value
the associated data: [<branch name> | <sha1> | <tag name> | <string>].
+IF REF is the empty list, the remote HEAD is used.
When RECURSIVE? is true, check out submodules as well, if any.
@@ -374,6 +379,7 @@ (define* (update-cached-checkout url
;; made little sense since the cache should be transparent to them. So
;; here we append "origin/" if it's missing and otherwise keep it.
(match ref
+ (() '(symref . "refs/remotes/origin/HEAD"))
(('branch . branch)
`(branch . ,(if (string-prefix? "origin/" branch)
branch
@@ -433,12 +439,13 @@ (define* (latest-repository-commit store url
(log-port (%make-void-port "w"))
(cache-directory
(%repository-cache-directory))
- (ref '(branch . "master")))
+ (ref '()))
"Return two values: the content of the git repository at URL copied into a
store directory and the sha1 of the top level commit in this directory. The
reference to be checkout, once the repository is fetched, is specified by REF.
REF is pair whose key is [branch | commit | tag] and value the associated
-data, respectively [<branch name> | <sha1> | <tag name>].
+data, respectively [<branch name> | <sha1> | <tag name>]. IF REF is the empty
+list, the remote HEAD is used.
When RECURSIVE? is true, check out submodules as well, if any.
@@ -548,7 +555,7 @@ (define-record-type* <git-checkout>
git-checkout make-git-checkout
git-checkout?
(url git-checkout-url)
- (branch git-checkout-branch (default "master"))
+ (branch git-checkout-branch (default #f))
(commit git-checkout-commit (default #f)) ;#f | tag | commit
(recursive? git-checkout-recursive? (default #f)))
@@ -587,9 +594,11 @@ (define-gexp-compiler (git-checkout-compiler (checkout <git-checkout>)
(match checkout
(($ <git-checkout> url branch commit recursive?)
(latest-repository-commit* url
- #:ref (if commit
- `(tag-or-commit . ,commit)
- `(branch . ,branch))
+ #:ref (cond (commit
+ `(tag-or-commit . ,commit))
+ (branch
+ `(branch . ,branch))
+ (else '()))
#:recursive? recursive?
#:log-port (current-error-port)))))
base-commit: f68bcc1bc3aa0a8d1829e2eb5d9ef256c817c17c
--
2.31.1
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.