Skip to contentSkip to conversation

builtin/fetch-pack: indicate when we have an exact oid

PATCH In review Nathan Froyd via GitGitGadget 2 participants 3 messages Sep 24 – Sep 25

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 builtin/fetch-pack: indicate when we have an exact oid

From: Nathan Froyd <[email protected]>

The git fetch path, when parsing OIDs, properly sets exact_oid on the relevant refs; the equivalent path for git fetch-pack does not. This oversight results in an invocation of git fetch-pack $OID sending want-ref $OID, which results in errors like:

fatal: unknown ref $OID
fatal: remote error: unknown ref $OID

Make the two paths equivalent by setting exact_oid properly.

---
builtin/fetch-pack: indicate when we have an exact oid

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2420%2Ffroydnj%2Ffroydnj-fetch-pack-exact-oid-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2420/froydnj/froydnj-fetch-pack-exact-oid-v1
Pull-Request: https://github.com/git/git/pull/2420

 builtin/fetch-pack.c               |  4 +-
 t/t5703-upload-pack-ref-in-want.sh | 68 ++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 1 deletion(-)
builtin/fetch-pack.c +3 -1
index 86754296fa..bef8a3dfc5 100644
@@ -23,13 +23,14 @@ static void add_sought_entry(struct ref ***sought, int *nr, int *alloc,
 	struct ref *ref;
 	struct object_id oid;
 	const char *p;
+	int exact_oid = 0;
 
 	if (!parse_oid_hex(name, &oid, &p)) {
 		if (*p == ' ') {
 			/* <oid> <ref>, find refname */
 			name = p + 1;
 		} else if (*p == '\0') {
-			; /* <oid>, leave oid as name */
+			exact_oid = 1; /* <oid>, leave oid as name */
 		} else {
 			/* <ref>, clear cruft from oid */
 			oidclr(&oid, the_repository->hash_algo);
@@ -41,6 +42,7 @@ static void add_sought_entry(struct ref ***sought, int *nr, int *alloc,
 
 	ref = alloc_ref(name);
 	oidcpy(&ref->old_oid, &oid);
+	ref->exact_oid = exact_oid;
 	(*nr)++;
 	ALLOC_GROW(*sought, *nr, *alloc);
 	(*sought)[*nr - 1] = ref;
t/t5703-upload-pack-ref-in-want.sh +68 -0

74 lines not on this page · raw message

base-commit: 3bc0341126508f78f5869cbfc0005e987efdf0c7

Signature · 1 line
gitgitgadget
#1 lore raw
Quoted 34 lines · Nathan Froyd via GitGitGadget

From: Nathan Froyd <[email protected]>

The git fetch path, when parsing OIDs, properly sets exact_oid on the relevant refs; the equivalent path for git fetch-pack does not. This oversight results in an invocation of git fetch-pack $OID sending want-ref $OID, which results in errors like:

fatal: unknown ref $OID
fatal: remote error: unknown ref $OID

Make the two paths equivalent by setting exact_oid properly.

---
builtin/fetch-pack: indicate when we have an exact oid

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2420%2Ffroydnj%2Ffroydnj-fetch-pack-exact-oid-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2420/froydnj/froydnj-fetch-pack-exact-oid-v1
Pull-Request: https://github.com/git/git/pull/2420

 builtin/fetch-pack.c               |  4 +-
 t/t5703-upload-pack-ref-in-want.sh | 68 ++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 1 deletion(-)
builtin/fetch-pack.c +1 -0
index 86754296fa..bef8a3dfc5 100644
@@ -23,13 +23,14 @@ static void add_sought_entry(struct ref ***sought, int *nr, int *alloc,
 	struct ref *ref;
 	struct object_id oid;
 	const char *p;
+	int exact_oid = 0;
 

builtin/fetch-pack.c static void add_sought_entry(struct ref ***sought, int *nr, int *alloc, · line 36

Quoted 10 lines
builtin/fetch-pack.c +1 -1
 	if (!parse_oid_hex(name, &oid, &p)) {
 		if (*p == ' ') {
 			/* <oid> <ref>, find refname */
 			name = p + 1;
 		} else if (*p == '\0') {
-			; /* <oid>, leave oid as name */
+			exact_oid = 1; /* <oid>, leave oid as name */
 		} else {
 			/* <ref>, clear cruft from oid */
 			oidclr(&oid, the_repository->hash_algo);

Unlike "git fetch" that is a higher level wrapper, in "git fetch-pack", a heuristic dwim like this is unwelcome. In

    $ git fetch-pack <repository> <ref>...

these <ref> arguments are meant to be passed exactly as given.

I am not sure if there days there still is a reason to run "fetch-pack" directly instead of "git fetch", as the manual of "git fetch-pack" itself suggets. But if there is, then shouldn't we give it an explicit way to say "this is not a ref but is an object name" in a more unambiguous way. Otherwise, we lose the ability to send "want-ref 3ce1e94f7d5607971b5de6762bbdb57e86fc5707", even if we wanted to, which is something "git fetch" does not let you do, and in turn may be a valid reason why somebody want to use "fetch-pack" over "fetch" in the first place.

#2 lore raw
Quoted 6 lines · Junio C Hamano

Unlike "git fetch" that is a higher level wrapper, in "git fetch-pack", a heuristic dwim like this is unwelcome. In

    $ git fetch-pack <repository> <ref>...

these <ref> arguments are meant to be passed exactly as given.

Need a bit of clarification. I should have said

... passed to the other side on "want-ref" packet as given on the command line.