Re: [PATCH 2/3] upload-pack: Prepare to extend allow-tip-sha1-in-want

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 2/3] upload-pack: Prepare to extend allow-tip-sha1-in-want

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:04:44

Fredrik Medley [off-list ref] writes:
Subject: Re: [PATCH 2/3] upload-pack: Prepare to extend allow-tip-sha1-in-want
s/: Prepare/: prepare/;
Rename the allow_tip_sha1_in_want variable to
allow_request_with_bare_object_name to allow for future extensions, e.g.
allowing non-tip sha1.
---
Sign-off?
quoted hunk
 fetch-pack.c  |  9 ++++++---
 upload-pack.c | 18 +++++++++++-------
 2 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/fetch-pack.c b/fetch-pack.c
index 48526aa..77174f9 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -43,7 +43,10 @@ static int marked;
 #define MAX_IN_VAIN 256
 
 static struct prio_queue rev_list = { compare_commits_by_commit_date };
-static int non_common_revs, multi_ack, use_sideband, allow_tip_sha1_in_want;
+static int non_common_revs, multi_ack, use_sideband;
+/* Allow specifying sha1 if it is a ref tip. */
+#define ALLOW_TIP	01
+static int allow_request_with_bare_object_name;
This side is OK, as "request" is by the end user giving the object
name from its command line.
quoted hunk
diff --git a/upload-pack.c b/upload-pack.c
index aa84576..708a502 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -35,7 +35,9 @@ static int multi_ack;
 static int no_done;
 static int use_thin_pack, use_ofs_delta, use_include_tag;
 static int no_progress, daemon_mode;
-static int allow_tip_sha1_in_want;
+/* Allow specifying sha1 if it is a ref tip. */
+#define ALLOW_TIP	01
+static int allow_request_with_bare_object_name;
This side is not quite good, as the request coming over wire is
always 40-hex bare object name.  We are allowing requests against
what we did not advertise (either the tip of hidden refs, or
somewhere deep in the history from some tip that may or may not have
been advertised).

allow-unadvertised-object-request or something, perhaps?
quoted hunk
 static int shallow_nr;
 static struct object_array have_obj;
 static struct object_array want_obj;
@@ -442,8 +444,8 @@ static int get_common_commits(void)
 
 static int is_our_ref(struct object *o)
 {
-	return o->flags &
-		((allow_tip_sha1_in_want ? HIDDEN_REF : 0) | OUR_REF);
+	int allow_hidden_ref = (allow_request_with_bare_object_name & ALLOW_TIP);
+	return o->flags & ((allow_hidden_ref ? HIDDEN_REF : 0) | OUR_REF);
 }
 
 static void check_non_tip(void)
@@ -727,7 +729,8 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo
 		packet_write(1, "%s %s%c%s%s%s%s agent=%s\n",
 			     sha1_to_hex(sha1), refname_nons,
 			     0, capabilities,
-			     allow_tip_sha1_in_want ? " allow-tip-sha1-in-want" : "",
+			     (allow_request_with_bare_object_name & ALLOW_TIP) ?
+				     " allow-tip-sha1-in-want" : "",
 			     stateless_rpc ? " no-done" : "",
 			     symref_info.buf,
 			     git_user_agent_sanitized());
@@ -787,9 +790,10 @@ static void upload_pack(void)
 
 static int upload_pack_config(const char *var, const char *value, void *unused)
 {
-	if (!strcmp("uploadpack.allowtipsha1inwant", var))
-		allow_tip_sha1_in_want = git_config_bool(var, value);
-	else if (!strcmp("uploadpack.keepalive", var)) {
+	if (!strcmp("uploadpack.allowtipsha1inwant", var)) {
+		if (git_config_bool(var, value))
+			allow_request_with_bare_object_name |= ALLOW_TIP;
+	} else if (!strcmp("uploadpack.keepalive", var)) {
 		keepalive = git_config_int(var, value);
 		if (!keepalive)
 			keepalive = -1;

Re: [PATCH 2/3] upload-pack: Prepare to extend allow-tip-sha1-in-want

From: Fredrik Medley <hidden>
Date: 2016-06-15 23:04:49

2015-05-12 23:39 GMT+02:00 Junio C Hamano [off-list ref]:
Fredrik Medley [off-list ref] writes:
quoted
Subject: Re: [PATCH 2/3] upload-pack: Prepare to extend allow-tip-sha1-in-want
diff --git a/fetch-pack.c b/fetch-pack.c
index 48526aa..77174f9 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -43,7 +43,10 @@ static int marked;
 #define MAX_IN_VAIN 256

 static struct prio_queue rev_list = { compare_commits_by_commit_date };
-static int non_common_revs, multi_ack, use_sideband, allow_tip_sha1_in_want;
+static int non_common_revs, multi_ack, use_sideband;
+/* Allow specifying sha1 if it is a ref tip. */
+#define ALLOW_TIP    01
+static int allow_request_with_bare_object_name;
This side is OK, as "request" is by the end user giving the object
name from its command line.
quoted
diff --git a/upload-pack.c b/upload-pack.c
index aa84576..708a502 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -35,7 +35,9 @@ static int multi_ack;
 static int no_done;
 static int use_thin_pack, use_ofs_delta, use_include_tag;
 static int no_progress, daemon_mode;
-static int allow_tip_sha1_in_want;
+/* Allow specifying sha1 if it is a ref tip. */
+#define ALLOW_TIP    01
+static int allow_request_with_bare_object_name;
This side is not quite good, as the request coming over wire is
always 40-hex bare object name.  We are allowing requests against
what we did not advertise (either the tip of hidden refs, or
somewhere deep in the history from some tip that may or may not have
been advertised).

allow-unadvertised-object-request or something, perhaps?
My imagination is not any better, so I will stick with
allow_unadvertised_object_request. I do think that that name
fits okay in fetch-pack.c as well. The name will also allow for a
possibly future option of requesting (unadvertised) hidden refs by
name, but it would of course require larger changes of the protocol.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help