Re: [PATCH v3 7/8] fetch: fetch objects by their exact SHA-1 object names
Jeff King writes: > On Wed, Jan 30, 2013 at 10:45:41AM -0800, Junio C Hamano wrote: > >> Teach "git fetch" to accept an exact SHA-1 object name the user may >> obtain out of band on the LHS of a pathspec, and send it on a "want" >> message when the server side advertises the allow-tip-sha1-in-want >> capability. > > Hmm. The UI on this is a little less nice than I would have hoped. Naming with unadvertised *refname*, not object name, needs protocol extension for the serving side to expand the name to object name; otherwise the receiving end wouldn't know what tip what it asked resulted in. And that belongs to a separate "expand refs" extension (aka "delaying ref advertisement") that is outside the scope of this series but can be built on top, as I said in the cover letter. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 7/8] fetch: fetch objects by their exact SHA-1 object names
On Tue, Feb 05, 2013 at 04:19:38AM -0500, Jeff King wrote: > But taking a step back, this really seems quite inferior to an > extension that would allow the client to share its refspecs with the > server. That would solve the bandwidth efficiency problem for normal > fetchers who are I should have read your cover letter more closely, as I see you make the point there that this is no longer about the efficiency, but about uncluttering. I'm not sure I like it as an uncluttering tool, though; for the reasons I stated in my previous mail, it makes it much more awkward for the moments when you actually do want to fetch some of the "clutter". -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 7/8] fetch: fetch objects by their exact SHA-1 object names
On Wed, Jan 30, 2013 at 10:45:41AM -0800, Junio C Hamano wrote: > Teach "git fetch" to accept an exact SHA-1 object name the user may > obtain out of band on the LHS of a pathspec, and send it on a "want" > message when the server side advertises the allow-tip-sha1-in-want > capability. Hmm. The UI on this is a little less nice than I would have hoped. Right now if you want a ref outside of refs/heads, it's up to you to configure a refspec or do a one-off fetch of the ref: git config --add remote.origin.fetch '+refs/pull/*:refs/pull/*' git fetch git checkout refs/pull/123/head ... inspect the contents ... Without advertisement, we have to learn that refs/pull/123/head exists out of band. We can no longer fetch all of the refs/pull hierarchy preemptively, but we can in theory grab at least that one ref like this: git fetch refs/pull/123/head git checkout FETCH_HEAD ... inspect the contents ... But that does not work with your patch; instead you have to learn not just the existence of the ref, but also its sha1. This may seem like a little thing, since you are already learning of the ref out-of-band, but: 1. The full sha1 is more annoying to work with. You'd have to cut and paste or otherwise script getting it to fetch. A human-readable ref, though, is much easier to remember. The "refs/pull/N/head" pattern is simple to learn and type. 2. Related to (1) above, is that it may be easier to come up with a hidden ref name out of band than the full sha1. E.g., if I am looking at https://github.com/me/foo.git/pulls/123, I can easily construct the ref from that. Getting the sha1 will take extra steps. 3. You have to do the out-of-band step, which may be inconvenient, every time the ref is updated. There is no way to say "just give me what is at the tip of refs/pull/123/head". I think you could solve it by teaching upload-pack to understand refs on "want" lines and convert them into the pointed-to object. But taking a step back, this really seems quite inferior to an extension that would allow the client to share its refspecs with the server. That would solve the bandwidth efficiency problem for normal fetchers who are looking at "refs/heads/*", while still giving people who are interested in "refs/pull/*" (or even a specific refs/pull tip) the information they need to fetch. The obvious problem is that the server speaks first. But I recall somebody suggested a combination of: 1. For git-over-ssh and git-over-tcp, the server advertises tell-me-your-refspecs as it starts advertising. Client interrupts advertisement with refspecs once it sees that it is OK to do so. We waste some bandwidth during the round-trip, but there will still be a benefit for repos with many refs (I wonder if we could even re-order the advertisement to show refs/heads/ first, as they are the most likely case to be requested). And as time goes on and the majority of clients support tell-me-your-refspecs, the server side can introduce a short delay after the first advertisement. 2. For git-over-http, the client speaks first via the http protocol. We can stuff the refspecs into extra query parameters. It's a little more complicated as a solution, but I feel like it gets the efficiency without a loss of functionality. And it helps in more situations than the hidden refs proposal (e.g., fetching refs/heads/foo can avoid enumerating all of refs/heads/*). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 7/8] fetch: fetch objects by their exact SHA-1 object names
Teach "git fetch" to accept an exact SHA-1 object name the user may
obtain out of band on the LHS of a pathspec, and send it on a "want"
message when the server side advertises the allow-tip-sha1-in-want
capability.
Signed-off-by: Junio C Hamano
---
fetch-pack.c | 22 +-
remote.c | 12 +++-
remote.h | 1 +
t/t5516-fetch-push.sh | 34 ++
4 files changed, 67 insertions(+), 2 deletions(-)
diff --git a/fetch-pack.c b/fetch-pack.c
index 915c0b7..70db646 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -36,7 +36,7 @@ static int marked;
#define MAX_IN_VAIN 256
static struct commit_list *rev_list;
-static int non_common_revs, multi_ack, use_sideband;
+static int non_common_revs, multi_ack, use_sideband, allow_tip_sha1_in_want;
static void rev_list_push(struct commit *commit, int mark)
{
@@ -563,6 +563,21 @@ static void filter_refs(struct fetch_pack_args *args,
}
}
+ /* Append unmatched requests to the list */
+ if (allow_tip_sha1_in_want) {
+ for (i = 0; i < nr_sought; i++) {
+ ref = sought[i];
+ if (ref->matched)
+ continue;
+ if (get_sha1_hex(ref->name, ref->old_sha1))
+ continue;
+
+ ref->matched = 1;
+ *newtail = ref;
+ ref->next = NULL;
+ newtail = &ref->next;
+ }
+ }
*refs = newlist;
}
@@ -803,6 +818,11 @@ static struct ref *do_fetch_pack(struct fetch_pack_args
*args,
fprintf(stderr, "Server supports side-band\n");
use_sideband = 1;
}
+ if (server_supports("allow-tip-sha1-in-want")) {
+ if (args->verbose)
+ fprintf(stderr, "Server supports
allow-tip-sha1-in-want\n");
+ allow_tip_sha1_in_want = 1;
+ }
if (!server_supports("thin-pack"))
args->use_thin_pack = 0;
if (!server_supports("no-progress"))
diff --git a/remote.c b/remote.c
index 1b7828d..1118d05 100644
--- a/remote.c
+++ b/remote.c
@@ -15,6 +15,7 @@ static struct refspec s_tag_refspec = {
0,
1,
0,
+ 0,
"refs/tags/*",
"refs/tags/*"
};
@@ -565,9 +566,13 @@ static struct refspec *parse_refspec_internal(int
nr_refspec, const char **refsp
flags = REFNAME_ALLOW_ONELEVEL | (is_glob ?
REFNAME_REFSPEC_PATTERN : 0);
if (fetch) {
+ unsigned char unused[40];
+
/* LHS */
if (!*rs[i].src)
; /* empty is ok; it means "HEAD" */
+ else if (llen == 40 && !get_sha1_hex(rs[i].src, unused))
+ rs[i].exact_sha1 = 1; /* ok */
else if (!check_refname_format(rs[i].src, flags))
; /* valid looking ref is ok */
else
@@ -1495,7 +1500,12 @@ int get_fetch_map(const struct ref *remote_refs,
} else {
const char *name = refspec->src[0] ? refspec->src : "HEAD";
- ref_map = get_remote_ref(remote_refs, name);
+ if (refspec->exact_sha1) {
+ ref_map = alloc_ref(name);
+ get_sha1_hex(name, ref_map->old_sha1);
+ } else {
+ ref_map = get_remote_ref(remote_refs, name);
+ }
if (!missing_ok && !ref_map)
die("Couldn't find remote ref %s", name);
if (ref_map) {
diff --git a/remote.h b/remote.h
index 251d8fd..f7b08f1 100644
--- a/remote.h
+++ b/remote.h
@@ -62,6 +62,7 @@ struct refspec {
unsigned force : 1;
unsigned pattern : 1;
unsigned matching : 1;
+ unsigned exact_sha1 : 1;
char *src;
char *dst;
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 852efb6..522056f 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1037,6 +1037,40 @@ test_expect_success 'push --prune refspec' '
! check_push_result $the_first_commit tmp/foo tmp/bar
'
+test_expect_success 'fetch exact SHA1' '
+ mk_test heads/master hidden/one &&
+ git push testrepo master:refs/hidden/one &&
+ (
+ cd testrepo &&
+ git config transfer.hiderefs refs/hidden
+ ) &&
+ check_push_result $the_commit hidden/one &&
+
+ mk_child child &&
+ (
+ cd child &&
+
+ # make sure $the_commit does not exist here
+ git repack -a -d &&
+ git prune &&
+ test_must_fail git cat-file -t $the_commit &&
+
+ # fetching the hidden object should fail by default
+ test
