Re: [PATCH v2] upload-pack: Optionally allow fetching reachable sha1

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

Re: [PATCH v2] upload-pack: Optionally allow fetching reachable sha1

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

Fredrik Medley [off-list ref] writes:
2015-05-06 0:16 GMT+02:00 Junio C Hamano [off-list ref]:
quoted
Fredrik Medley [off-list ref] writes:
quoted
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2e5ceaf..76cd713 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2538,6 +2538,12 @@ uploadpack.allowtipsha1inwant::
      of a hidden ref (by default, such a request is rejected).
      see also `uploadpack.hideRefs`.

+uploadpack.allowreachablesha1inwant::
I know that the existing allowtipsha1inwant is spelled that way, and
it may be better done as a separate clean-up patch (either before or
after this step), but the documentation and the first line of the
log message would be easier to read with

        uploadpack.allowReachableSHA1InWant

I'd think.
I would prefer using allowReachableSha1InWant. Please tell
me if I should use SHA1InWant instead of Sha1InWant.
(I cannot find anything similar in the repository.)
Keep in mind what was discussed recently:

  http://thread.gmane.org/gmane.comp.version-control.git/265225/focus=265322

I would think SHA1 should be upcased (so should SSL, SMTP, etc.)
even in the existing ones when we do the "clean-up" pass.  Even
though this patch is not about cleaning up existing mess, there
is no point adding more cruft that we need to clean up later ;-)

[PATCH 1/3] config.txt: Clarify allowTipSHA1InWant with camelCase

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

Most of the options in config.txt are camelCase. Improve the readability
for allowtipsha1inwant by changing to allowTipSHA1InWant.
---
 Documentation/config.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2e5ceaf..2b86fe6 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2530,9 +2530,9 @@ uploadpack.hideRefs::
 	are under the hierarchies listed on the value of this
 	variable is excluded, and is hidden from `git ls-remote`,
 	`git fetch`, etc.  An attempt to fetch a hidden ref by `git
-	fetch` will fail.  See also `uploadpack.allowtipsha1inwant`.
+	fetch` will fail.  See also `uploadpack.allowTipSHA1InWant`.
 
-uploadpack.allowtipsha1inwant::
+uploadpack.allowTipSHA1InWant::
 	When `uploadpack.hideRefs` is in effect, allow `upload-pack`
 	to accept a fetch request that asks for an object at the tip
 	of a hidden ref (by default, such a request is rejected).
-- 
1.9.1

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

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

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.
---
 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;
 
 static void rev_list_push(struct commit *commit, int mark)
 {
@@ -542,7 +545,7 @@ static void filter_refs(struct fetch_pack_args *args,
 	}
 
 	/* Append unmatched requests to the list */
-	if (allow_tip_sha1_in_want) {
+	if (allow_request_with_bare_object_name & ALLOW_TIP) {
 		for (i = 0; i < nr_sought; i++) {
 			unsigned char sha1[20];
 
@@ -821,7 +824,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
 	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;
+		allow_request_with_bare_object_name |= ALLOW_TIP;
 	}
 	if (!server_supports("thin-pack"))
 		args->use_thin_pack = 0;
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;
 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;
-- 
1.9.1

[PATCH 3/3] upload-pack: Optionally allow fetching reachable sha1

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

With uploadpack.allowReachableSHA1InWant configuration option set on the
server side, "git fetch" can make a request with a "want" line that names
an object that has not been advertised (likely to have been obtained out
of band or from a submodule pointer). Only objects reachable from the
branch tips, i.e. the union of advertised branches and branches hidden by
transfer.hideRefs, will be processed. Note that there is an associated
cost of having to walk back the hstory to check the reachability.

This feature can be used when obtaining the content of a certain commit,
for which the sha1 is known, without the need of cloning the whole
repository, especially if a shallow fetch is used. Useful cases are e.g.
repositories containing large files in the history, fetching only the
needed data for a submodule checkout, when sharing a sha1 without telling
which exact branch it belongs to and in Gerrit, if you think in terms of
commits instead of change numbers. (The Gerrit case has already been
solved through allowTipSHA1InWant as every Gerrit change has a ref.)

Signed-off-by: Fredrik Medley <redacted>
---
 Documentation/config.txt                          |  6 +++
 Documentation/technical/http-protocol.txt         |  3 +-
 Documentation/technical/protocol-capabilities.txt |  7 +++
 fetch-pack.c                                      |  9 +++-
 t/t5516-fetch-push.sh                             | 55 +++++++++++++++++++++++
 upload-pack.c                                     | 20 +++++++--
 6 files changed, 94 insertions(+), 6 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2b86fe6..ffc4cf4 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2538,6 +2538,12 @@ uploadpack.allowTipSHA1InWant::
 	of a hidden ref (by default, such a request is rejected).
 	see also `uploadpack.hideRefs`.
 
+uploadpack.allowReachableSHA1InWant::
+	Allow `upload-pack` to accept a fetch request that asks for an
+	object that is reachable from any ref tip. However, note that
+	calculating	object reachability is computationally expensive.
+	Defaults to `false`.
+
 uploadpack.keepAlive::
 	When `upload-pack` has started `pack-objects`, there may be a
 	quiet period while `pack-objects` prepares the pack. Normally
diff --git a/Documentation/technical/http-protocol.txt b/Documentation/technical/http-protocol.txt
index 229f845..1c561bd 100644
--- a/Documentation/technical/http-protocol.txt
+++ b/Documentation/technical/http-protocol.txt
@@ -319,7 +319,8 @@ Servers SHOULD support all capabilities defined here.
 Clients MUST send at least one "want" command in the request body.
 Clients MUST NOT reference an id in a "want" command which did not
 appear in the response obtained through ref discovery unless the
-server advertises capability `allow-tip-sha1-in-want`.
+server advertises capability `allow-tip-sha1-in-want` or
+`allow-reachable-sha1-in-want`.
 
   compute_request   =  want_list
 		       have_list
diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt
index 4f8a7bf..265fcab 100644
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -260,6 +260,13 @@ If the upload-pack server advertises this capability, fetch-pack may
 send "want" lines with SHA-1s that exist at the server but are not
 advertised by upload-pack.
 
+allow-reachable-sha1-in-want
+----------------------
+
+If the upload-pack server advertises this capability, fetch-pack may
+send "want" lines with SHA-1s that exist at the server but are not
+advertised by upload-pack.
+
 push-cert=<nonce>
 -----------------
 
diff --git a/fetch-pack.c b/fetch-pack.c
index 77174f9..6c63d8e 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -46,6 +46,8 @@ static struct prio_queue rev_list = { compare_commits_by_commit_date };
 static int non_common_revs, multi_ack, use_sideband;
 /* Allow specifying sha1 if it is a ref tip. */
 #define ALLOW_TIP	01
+/* Allow request of a sha1 if it is reachable from a ref (possibly hidden ref). */
+#define ALLOW_REACHABLE	02
 static int allow_request_with_bare_object_name;
 
 static void rev_list_push(struct commit *commit, int mark)
@@ -545,7 +547,7 @@ static void filter_refs(struct fetch_pack_args *args,
 	}
 
 	/* Append unmatched requests to the list */
-	if (allow_request_with_bare_object_name & ALLOW_TIP) {
+	if (allow_request_with_bare_object_name & (ALLOW_TIP | ALLOW_REACHABLE)) {
 		for (i = 0; i < nr_sought; i++) {
 			unsigned char sha1[20];
 
@@ -826,6 +828,11 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
 			fprintf(stderr, "Server supports allow-tip-sha1-in-want\n");
 		allow_request_with_bare_object_name |= ALLOW_TIP;
 	}
+	if (server_supports("allow-reachable-sha1-in-want")) {
+		if (args->verbose)
+			fprintf(stderr, "Server supports allow-reachable-sha1-in-want\n");
+		allow_request_with_bare_object_name |= ALLOW_REACHABLE;
+	}
 	if (!server_supports("thin-pack"))
 		args->use_thin_pack = 0;
 	if (!server_supports("no-progress"))
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 8a5f236..fdcc114 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1120,6 +1120,61 @@ test_expect_success 'fetch exact SHA1' '
 	)
 '
 
+for configallowtipsha1inwant in true false
+do
+	test_expect_success "shallow fetch reachable SHA1 (but not a ref), allowtipsha1inwant=$configallowtipsha1inwant" '
+		mk_empty testrepo &&
+		(
+			cd testrepo &&
+			git config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&
+			git commit --allow-empty -m foo &&
+			git commit --allow-empty -m bar
+		) &&
+		SHA1=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&
+		mk_empty shallow &&
+		(
+			cd shallow &&
+			test_must_fail git fetch --depth=1 ../testrepo/.git $SHA1 &&
+			git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
+			git fetch --depth=1 ../testrepo/.git $SHA1 &&
+			git cat-file commit $SHA1 >/dev/null
+		)
+	'
+
+	test_expect_success "deny fetch unreachable SHA1, allowtipsha1inwant=$configallowtipsha1inwant" '
+		mk_empty testrepo &&
+		(
+			cd testrepo &&
+			git config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&
+			git commit --allow-empty -m foo &&
+			git commit --allow-empty -m bar &&
+			git commit --allow-empty -m xyz
+		)
+		SHA1_1=$(git --git-dir=testrepo/.git rev-parse HEAD^^) &&
+		SHA1_2=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&
+		SHA1_3=$(git --git-dir=testrepo/.git rev-parse HEAD) &&
+		(
+			cd testrepo &&
+			git reset --hard $SHA1_2 &&
+			git cat-file commit $SHA1_3 >/dev/null &&
+			git cat-file commit $SHA1_3 >/dev/null
+		) &&
+		mk_empty shallow &&
+		(
+			cd shallow &&
+			test_must_fail git fetch ../testrepo/.git $SHA1_3 &&
+			test_must_fail git fetch ../testrepo/.git $SHA1_1 &&
+			git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
+			git fetch ../testrepo/.git $SHA1_1 &&
+			git cat-file commit $SHA1_1 >/dev/null &&
+			test_must_fail git cat-file commit $SHA1_2 >/dev/null &&
+			git fetch ../testrepo/.git $SHA1_2 &&
+			git cat-file commit $SHA1_2 >/dev/null &&
+			test_must_fail git fetch ../testrepo/.git $SHA1_3
+		)
+	'
+done
+
 test_expect_success 'fetch follows tags by default' '
 	mk_test testrepo heads/master &&
 	rm -fr src dst &&
diff --git a/upload-pack.c b/upload-pack.c
index 708a502..bd14a81 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -37,6 +37,8 @@ static int use_thin_pack, use_ofs_delta, use_include_tag;
 static int no_progress, daemon_mode;
 /* Allow specifying sha1 if it is a ref tip. */
 #define ALLOW_TIP	01
+/* Allow request of a sha1 if it is reachable from a ref (possibly hidden ref). */
+#define ALLOW_REACHABLE	02
 static int allow_request_with_bare_object_name;
 static int shallow_nr;
 static struct object_array have_obj;
@@ -444,7 +446,8 @@ static int get_common_commits(void)
 
 static int is_our_ref(struct object *o)
 {
-	int allow_hidden_ref = (allow_request_with_bare_object_name & ALLOW_TIP);
+	int allow_hidden_ref = (allow_request_with_bare_object_name &
+		(ALLOW_TIP | ALLOW_REACHABLE));
 	return o->flags & ((allow_hidden_ref ? HIDDEN_REF : 0) | OUR_REF);
 }
 
@@ -458,8 +461,12 @@ static void check_non_tip(void)
 	char namebuf[42]; /* ^ + SHA-1 + LF */
 	int i;
 
-	/* In the normal in-process case non-tip request can never happen */
-	if (!stateless_rpc)
+	/*
+	 * In the normal in-process case without
+	 * uploadpack.allowReachableSHA1InWant,
+	 * non-tip requests can never happen.
+	 */
+	if (!stateless_rpc && !(allow_request_with_bare_object_name & ALLOW_REACHABLE))
 		goto error;
 
 	cmd.argv = argv;
@@ -726,11 +733,13 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo
 		struct strbuf symref_info = STRBUF_INIT;
 
 		format_symref_info(&symref_info, cb_data);
-		packet_write(1, "%s %s%c%s%s%s%s agent=%s\n",
+		packet_write(1, "%s %s%c%s%s%s%s%s agent=%s\n",
 			     sha1_to_hex(sha1), refname_nons,
 			     0, capabilities,
 			     (allow_request_with_bare_object_name & ALLOW_TIP) ?
 				     " allow-tip-sha1-in-want" : "",
+			     (allow_request_with_bare_object_name & ALLOW_REACHABLE) ?
+				     " allow-reachable-sha1-in-want" : "",
 			     stateless_rpc ? " no-done" : "",
 			     symref_info.buf,
 			     git_user_agent_sanitized());
@@ -793,6 +802,9 @@ static int upload_pack_config(const char *var, const char *value, void *unused)
 	if (!strcmp("uploadpack.allowtipsha1inwant", var)) {
 		if (git_config_bool(var, value))
 			allow_request_with_bare_object_name |= ALLOW_TIP;
+	} else if (!strcmp("uploadpack.allowreachablesha1inwant", var)) {
+		if (git_config_bool(var, value))
+			allow_request_with_bare_object_name |= ALLOW_REACHABLE;
 	} else if (!strcmp("uploadpack.keepalive", var)) {
 		keepalive = git_config_int(var, value);
 		if (!keepalive)
-- 
1.9.1

Re: [PATCH 1/3] config.txt: Clarify allowTipSHA1InWant with camelCase

From: Eric Sunshine <hidden>
Date: 2016-06-15 23:04:44

On Tue, May 12, 2015 at 5:14 PM, Fredrik Medley
[off-list ref] wrote:
Most of the options in config.txt are camelCase. Improve the readability
for allowtipsha1inwant by changing to allowTipSHA1InWant.
Missing sign-off. More below.
quoted hunk
---
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2e5ceaf..2b86fe6 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2530,9 +2530,9 @@ uploadpack.hideRefs::
        are under the hierarchies listed on the value of this
        variable is excluded, and is hidden from `git ls-remote`,
        `git fetch`, etc.  An attempt to fetch a hidden ref by `git
-       fetch` will fail.  See also `uploadpack.allowtipsha1inwant`.
+       fetch` will fail.  See also `uploadpack.allowTipSHA1InWant`.

-uploadpack.allowtipsha1inwant::
+uploadpack.allowTipSHA1InWant::
This was already attempted here[1]; some inconsistencies with acronyms
and abbreviations pointed out here[2]; and delayed here[3]. Perhaps
it's now time to revisit the acronyms/abbreviations issue?

[1]: http://thread.gmane.org/gmane.comp.version-control.git/265225/focus=265225
[2]: http://thread.gmane.org/gmane.comp.version-control.git/265225/focus=265262
[3]: http://thread.gmane.org/gmane.comp.version-control.git/265225/focus=265282
        When `uploadpack.hideRefs` is in effect, allow `upload-pack`
        to accept a fetch request that asks for an object at the tip
        of a hidden ref (by default, such a request is rejected).
--
1.9.1

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

From: Eric Sunshine <hidden>
Date: 2016-06-15 23:04:44

On Tue, May 12, 2015 at 5:14 PM, Fredrik Medley
[off-list ref] wrote:
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.
'allow_request_with_bare_object_name' is quite a mouthful. Does it
need to be this long?

Regarding the commit message: Isn't this preparatory step really about
changing the variable from a simple boolean to a "flag"-style so that
it can hold bits for multiple options? Perhaps the commit message
should mention something about that?

Missing sign-off.

More below.
quoted hunk
---
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;

 static void rev_list_push(struct commit *commit, int mark)
 {
@@ -542,7 +545,7 @@ static void filter_refs(struct fetch_pack_args *args,
        }

        /* Append unmatched requests to the list */
-       if (allow_tip_sha1_in_want) {
+       if (allow_request_with_bare_object_name & ALLOW_TIP) {
Some compilers are going to warn about this (warning: did you mean
"&&" rather than "&"?). Wrap it in an extra set of parentheses to
avoid the warning.
quoted hunk
                for (i = 0; i < nr_sought; i++) {
                        unsigned char sha1[20];
@@ -821,7 +824,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
        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;
+               allow_request_with_bare_object_name |= ALLOW_TIP;
        }
        if (!server_supports("thin-pack"))
                args->use_thin_pack = 0;
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;
 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;
--
1.9.1

Re: [PATCH 3/3] upload-pack: Optionally allow fetching reachable sha1

From: Eric Sunshine <hidden>
Date: 2016-06-15 23:04:44

On Tue, May 12, 2015 at 5:14 PM, Fredrik Medley
[off-list ref] wrote:
With uploadpack.allowReachableSHA1InWant configuration option set on the
server side, "git fetch" can make a request with a "want" line that names
an object that has not been advertised (likely to have been obtained out
of band or from a submodule pointer). Only objects reachable from the
branch tips, i.e. the union of advertised branches and branches hidden by
transfer.hideRefs, will be processed. Note that there is an associated
cost of having to walk back the hstory to check the reachability.
Mentioned previously[1]: s/hstory/history/
quoted hunk
This feature can be used when obtaining the content of a certain commit,
for which the sha1 is known, without the need of cloning the whole
repository, especially if a shallow fetch is used. Useful cases are e.g.
repositories containing large files in the history, fetching only the
needed data for a submodule checkout, when sharing a sha1 without telling
which exact branch it belongs to and in Gerrit, if you think in terms of
commits instead of change numbers. (The Gerrit case has already been
solved through allowTipSHA1InWant as every Gerrit change has a ref.)

Signed-off-by: Fredrik Medley <redacted>
---
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2b86fe6..ffc4cf4 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2538,6 +2538,12 @@ uploadpack.allowTipSHA1InWant::
        of a hidden ref (by default, such a request is rejected).
        see also `uploadpack.hideRefs`.

+uploadpack.allowReachableSHA1InWant::
+       Allow `upload-pack` to accept a fetch request that asks for an
+       object that is reachable from any ref tip. However, note that
+       calculating     object reachability is computationally expensive.
Mentioned previously[1]: s/\s+/ /
quoted hunk
+       Defaults to `false`.
+
diff --git a/fetch-pack.c b/fetch-pack.c
index 77174f9..6c63d8e 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -545,7 +547,7 @@ static void filter_refs(struct fetch_pack_args *args,
        }

        /* Append unmatched requests to the list */
-       if (allow_request_with_bare_object_name & ALLOW_TIP) {
+       if (allow_request_with_bare_object_name & (ALLOW_TIP | ALLOW_REACHABLE)) {
Wrap this in extra parentheses to avoid compilation warnings:

    if ((foo & (bar | baz))) {
                for (i = 0; i < nr_sought; i++) {
                        unsigned char sha1[20];
[1]: http://article.gmane.org/gmane.comp.version-control.git/268428

[PATCH 1/3] config.txt: clarify allowTipSHA1InWant with camelCase

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

Most of the options in config.txt are camelCase. Improve the readability
for allowtipsha1inwant by changing to allowTipSHA1InWant.

Signed-off-by: Fredrik Medley <redacted>
---
This patch is optional. There has been work on fixing the whole
Documentation/config.txt which has not been merged yet. When adding
allowReachableSHA1InWant later, it would be good to already have changed
to allowTipSHA1InWant.

 Documentation/config.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2e5ceaf..2b86fe6 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2530,9 +2530,9 @@ uploadpack.hideRefs::
 	are under the hierarchies listed on the value of this
 	variable is excluded, and is hidden from `git ls-remote`,
 	`git fetch`, etc.  An attempt to fetch a hidden ref by `git
-	fetch` will fail.  See also `uploadpack.allowtipsha1inwant`.
+	fetch` will fail.  See also `uploadpack.allowTipSHA1InWant`.
 
-uploadpack.allowtipsha1inwant::
+uploadpack.allowTipSHA1InWant::
 	When `uploadpack.hideRefs` is in effect, allow `upload-pack`
 	to accept a fetch request that asks for an object at the tip
 	of a hidden ref (by default, such a request is rejected).
-- 
1.9.1

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

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

To allow future extensions, e.g. allowing non-tip sha1, replace the
boolean allow_tip_sha1_in_want variable with the flag-style
allow_request_with_bare_object_name variable.

Signed-off-by: Fredrik Medley <redacted>
---
 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..699f586 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_SHA1	01
+static int allow_unadvertised_object_request;
 
 static void rev_list_push(struct commit *commit, int mark)
 {
@@ -542,7 +545,7 @@ static void filter_refs(struct fetch_pack_args *args,
 	}
 
 	/* Append unmatched requests to the list */
-	if (allow_tip_sha1_in_want) {
+	if ((allow_unadvertised_object_request & ALLOW_TIP_SHA1)) {
 		for (i = 0; i < nr_sought; i++) {
 			unsigned char sha1[20];
 
@@ -821,7 +824,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
 	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;
+		allow_unadvertised_object_request |= ALLOW_TIP_SHA1;
 	}
 	if (!server_supports("thin-pack"))
 		args->use_thin_pack = 0;
diff --git a/upload-pack.c b/upload-pack.c
index aa84576..d443e58 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_SHA1	01
+static int allow_unadvertised_object_request;
 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_unadvertised_object_request & ALLOW_TIP_SHA1);
+	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_unadvertised_object_request & ALLOW_TIP_SHA1) ?
+				     " 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_unadvertised_object_request |= ALLOW_TIP_SHA1;
+	} else if (!strcmp("uploadpack.keepalive", var)) {
 		keepalive = git_config_int(var, value);
 		if (!keepalive)
 			keepalive = -1;
-- 
1.9.1

[PATCH 3/3] upload-pack: optionally allow fetching reachable sha1

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

With uploadpack.allowReachableSHA1InWant configuration option set on the
server side, "git fetch" can make a request with a "want" line that names
an object that has not been advertised (likely to have been obtained out
of band or from a submodule pointer). Only objects reachable from the
branch tips, i.e. the union of advertised branches and branches hidden by
transfer.hideRefs, will be processed. Note that there is an associated
cost of having to walk back the history to check the reachability.

This feature can be used when obtaining the content of a certain commit,
for which the sha1 is known, without the need of cloning the whole
repository, especially if a shallow fetch is used. Useful cases are e.g.
repositories containing large files in the history, fetching only the
needed data for a submodule checkout, when sharing a sha1 without telling
which exact branch it belongs to and in Gerrit, if you think in terms of
commits instead of change numbers. (The Gerrit case has already been
solved through allowTipSHA1InWant as every Gerrit change has a ref.)

Signed-off-by: Fredrik Medley <redacted>
---
 Documentation/config.txt                          |  6 +++
 Documentation/technical/http-protocol.txt         |  3 +-
 Documentation/technical/protocol-capabilities.txt |  7 +++
 fetch-pack.c                                      | 11 ++++-
 t/t5516-fetch-push.sh                             | 55 +++++++++++++++++++++++
 upload-pack.c                                     | 22 ++++++---
 6 files changed, 96 insertions(+), 8 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2b86fe6..980520b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2538,6 +2538,12 @@ uploadpack.allowTipSHA1InWant::
 	of a hidden ref (by default, such a request is rejected).
 	see also `uploadpack.hideRefs`.
 
+uploadpack.allowReachableSHA1InWant::
+	Allow `upload-pack` to accept a fetch request that asks for an
+	object that is reachable from any ref tip. However, note that
+	calculating object reachability is computationally expensive.
+	Defaults to `false`.
+
 uploadpack.keepAlive::
 	When `upload-pack` has started `pack-objects`, there may be a
 	quiet period while `pack-objects` prepares the pack. Normally
diff --git a/Documentation/technical/http-protocol.txt b/Documentation/technical/http-protocol.txt
index 229f845..1c561bd 100644
--- a/Documentation/technical/http-protocol.txt
+++ b/Documentation/technical/http-protocol.txt
@@ -319,7 +319,8 @@ Servers SHOULD support all capabilities defined here.
 Clients MUST send at least one "want" command in the request body.
 Clients MUST NOT reference an id in a "want" command which did not
 appear in the response obtained through ref discovery unless the
-server advertises capability `allow-tip-sha1-in-want`.
+server advertises capability `allow-tip-sha1-in-want` or
+`allow-reachable-sha1-in-want`.
 
   compute_request   =  want_list
 		       have_list
diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt
index 4f8a7bf..265fcab 100644
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -260,6 +260,13 @@ If the upload-pack server advertises this capability, fetch-pack may
 send "want" lines with SHA-1s that exist at the server but are not
 advertised by upload-pack.
 
+allow-reachable-sha1-in-want
+----------------------
+
+If the upload-pack server advertises this capability, fetch-pack may
+send "want" lines with SHA-1s that exist at the server but are not
+advertised by upload-pack.
+
 push-cert=<nonce>
 -----------------
 
diff --git a/fetch-pack.c b/fetch-pack.c
index 699f586..875b688 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -45,7 +45,9 @@ static int marked;
 static struct prio_queue rev_list = { compare_commits_by_commit_date };
 static int non_common_revs, multi_ack, use_sideband;
 /* Allow specifying sha1 if it is a ref tip. */
-#define ALLOW_TIP_SHA1	01
+#define ALLOW_TIP_SHA1		01
+/* Allow request of a sha1 if it is reachable from a ref (possibly hidden ref). */
+#define ALLOW_REACHABLE_SHA1	02
 static int allow_unadvertised_object_request;
 
 static void rev_list_push(struct commit *commit, int mark)
@@ -545,7 +547,7 @@ static void filter_refs(struct fetch_pack_args *args,
 	}
 
 	/* Append unmatched requests to the list */
-	if ((allow_unadvertised_object_request & ALLOW_TIP_SHA1)) {
+	if ((allow_unadvertised_object_request & (ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1))) {
 		for (i = 0; i < nr_sought; i++) {
 			unsigned char sha1[20];
 
@@ -826,6 +828,11 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
 			fprintf(stderr, "Server supports allow-tip-sha1-in-want\n");
 		allow_unadvertised_object_request |= ALLOW_TIP_SHA1;
 	}
+	if (server_supports("allow-reachable-sha1-in-want")) {
+		if (args->verbose)
+			fprintf(stderr, "Server supports allow-reachable-sha1-in-want\n");
+		allow_unadvertised_object_request |= ALLOW_REACHABLE_SHA1;
+	}
 	if (!server_supports("thin-pack"))
 		args->use_thin_pack = 0;
 	if (!server_supports("no-progress"))
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 8a5f236..fdcc114 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1120,6 +1120,61 @@ test_expect_success 'fetch exact SHA1' '
 	)
 '
 
+for configallowtipsha1inwant in true false
+do
+	test_expect_success "shallow fetch reachable SHA1 (but not a ref), allowtipsha1inwant=$configallowtipsha1inwant" '
+		mk_empty testrepo &&
+		(
+			cd testrepo &&
+			git config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&
+			git commit --allow-empty -m foo &&
+			git commit --allow-empty -m bar
+		) &&
+		SHA1=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&
+		mk_empty shallow &&
+		(
+			cd shallow &&
+			test_must_fail git fetch --depth=1 ../testrepo/.git $SHA1 &&
+			git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
+			git fetch --depth=1 ../testrepo/.git $SHA1 &&
+			git cat-file commit $SHA1 >/dev/null
+		)
+	'
+
+	test_expect_success "deny fetch unreachable SHA1, allowtipsha1inwant=$configallowtipsha1inwant" '
+		mk_empty testrepo &&
+		(
+			cd testrepo &&
+			git config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&
+			git commit --allow-empty -m foo &&
+			git commit --allow-empty -m bar &&
+			git commit --allow-empty -m xyz
+		)
+		SHA1_1=$(git --git-dir=testrepo/.git rev-parse HEAD^^) &&
+		SHA1_2=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&
+		SHA1_3=$(git --git-dir=testrepo/.git rev-parse HEAD) &&
+		(
+			cd testrepo &&
+			git reset --hard $SHA1_2 &&
+			git cat-file commit $SHA1_3 >/dev/null &&
+			git cat-file commit $SHA1_3 >/dev/null
+		) &&
+		mk_empty shallow &&
+		(
+			cd shallow &&
+			test_must_fail git fetch ../testrepo/.git $SHA1_3 &&
+			test_must_fail git fetch ../testrepo/.git $SHA1_1 &&
+			git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
+			git fetch ../testrepo/.git $SHA1_1 &&
+			git cat-file commit $SHA1_1 >/dev/null &&
+			test_must_fail git cat-file commit $SHA1_2 >/dev/null &&
+			git fetch ../testrepo/.git $SHA1_2 &&
+			git cat-file commit $SHA1_2 >/dev/null &&
+			test_must_fail git fetch ../testrepo/.git $SHA1_3
+		)
+	'
+done
+
 test_expect_success 'fetch follows tags by default' '
 	mk_test testrepo heads/master &&
 	rm -fr src dst &&
diff --git a/upload-pack.c b/upload-pack.c
index d443e58..f99eb99 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -36,7 +36,9 @@ static int no_done;
 static int use_thin_pack, use_ofs_delta, use_include_tag;
 static int no_progress, daemon_mode;
 /* Allow specifying sha1 if it is a ref tip. */
-#define ALLOW_TIP_SHA1	01
+#define ALLOW_TIP_SHA1		01
+/* Allow request of a sha1 if it is reachable from a ref (possibly hidden ref). */
+#define ALLOW_REACHABLE_SHA1	02
 static int allow_unadvertised_object_request;
 static int shallow_nr;
 static struct object_array have_obj;
@@ -444,7 +446,8 @@ static int get_common_commits(void)
 
 static int is_our_ref(struct object *o)
 {
-	int allow_hidden_ref = (allow_unadvertised_object_request & ALLOW_TIP_SHA1);
+	int allow_hidden_ref = (allow_unadvertised_object_request &
+		(ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1));
 	return o->flags & ((allow_hidden_ref ? HIDDEN_REF : 0) | OUR_REF);
 }
 
@@ -458,8 +461,12 @@ static void check_non_tip(void)
 	char namebuf[42]; /* ^ + SHA-1 + LF */
 	int i;
 
-	/* In the normal in-process case non-tip request can never happen */
-	if (!stateless_rpc)
+	/*
+	 * In the normal in-process case without
+	 * uploadpack.allowReachableSHA1InWant,
+	 * non-tip requests can never happen.
+	 */
+	if (!stateless_rpc && !(allow_unadvertised_object_request & ALLOW_REACHABLE_SHA1))
 		goto error;
 
 	cmd.argv = argv;
@@ -726,11 +733,13 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo
 		struct strbuf symref_info = STRBUF_INIT;
 
 		format_symref_info(&symref_info, cb_data);
-		packet_write(1, "%s %s%c%s%s%s%s agent=%s\n",
+		packet_write(1, "%s %s%c%s%s%s%s%s agent=%s\n",
 			     sha1_to_hex(sha1), refname_nons,
 			     0, capabilities,
 			     (allow_unadvertised_object_request & ALLOW_TIP_SHA1) ?
 				     " allow-tip-sha1-in-want" : "",
+			     (allow_unadvertised_object_request & ALLOW_REACHABLE_SHA1) ?
+				     " allow-reachable-sha1-in-want" : "",
 			     stateless_rpc ? " no-done" : "",
 			     symref_info.buf,
 			     git_user_agent_sanitized());
@@ -793,6 +802,9 @@ static int upload_pack_config(const char *var, const char *value, void *unused)
 	if (!strcmp("uploadpack.allowtipsha1inwant", var)) {
 		if (git_config_bool(var, value))
 			allow_unadvertised_object_request |= ALLOW_TIP_SHA1;
+	} else if (!strcmp("uploadpack.allowreachablesha1inwant", var)) {
+		if (git_config_bool(var, value))
+			allow_unadvertised_object_request |= ALLOW_REACHABLE_SHA1;
 	} else if (!strcmp("uploadpack.keepalive", var)) {
 		keepalive = git_config_int(var, value);
 		if (!keepalive)
-- 
1.9.1

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

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

To allow future extensions, e.g. allowing non-tip sha1, replace the
boolean allow_tip_sha1_in_want variable with the flag-style
allow_request_with_bare_object_name variable.

Signed-off-by: Fredrik Medley <redacted>
---
 fetch-pack.c  |  9 ++++++---
 upload-pack.c | 20 +++++++++++++-------
 2 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/fetch-pack.c b/fetch-pack.c
index 48526aa..699f586 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_SHA1	01
+static int allow_unadvertised_object_request;
 
 static void rev_list_push(struct commit *commit, int mark)
 {
@@ -542,7 +545,7 @@ static void filter_refs(struct fetch_pack_args *args,
 	}
 
 	/* Append unmatched requests to the list */
-	if (allow_tip_sha1_in_want) {
+	if ((allow_unadvertised_object_request & ALLOW_TIP_SHA1)) {
 		for (i = 0; i < nr_sought; i++) {
 			unsigned char sha1[20];
 
@@ -821,7 +824,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
 	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;
+		allow_unadvertised_object_request |= ALLOW_TIP_SHA1;
 	}
 	if (!server_supports("thin-pack"))
 		args->use_thin_pack = 0;
diff --git a/upload-pack.c b/upload-pack.c
index 745fda8..726486b 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_SHA1	01
+static int allow_unadvertised_object_request;
 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_unadvertised_object_request & ALLOW_TIP_SHA1);
+	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_unadvertised_object_request & ALLOW_TIP_SHA1) ?
+				     " allow-tip-sha1-in-want" : "",
 			     stateless_rpc ? " no-done" : "",
 			     symref_info.buf,
 			     git_user_agent_sanitized());
@@ -787,9 +790,12 @@ 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_unadvertised_object_request |= ALLOW_TIP_SHA1;
+		else
+			allow_unadvertised_object_request &= ~ALLOW_TIP_SHA1;
+	} else if (!strcmp("uploadpack.keepalive", var)) {
 		keepalive = git_config_int(var, value);
 		if (!keepalive)
 			keepalive = -1;
-- 
1.9.1

[PATCH v5 3/3] upload-pack: optionally allow fetching reachable sha1

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

With uploadpack.allowReachableSHA1InWant configuration option set on the
server side, "git fetch" can make a request with a "want" line that names
an object that has not been advertised (likely to have been obtained out
of band or from a submodule pointer). Only objects reachable from the
branch tips, i.e. the union of advertised branches and branches hidden by
transfer.hideRefs, will be processed. Note that there is an associated
cost of having to walk back the history to check the reachability.

This feature can be used when obtaining the content of a certain commit,
for which the sha1 is known, without the need of cloning the whole
repository, especially if a shallow fetch is used. Useful cases are e.g.
repositories containing large files in the history, fetching only the
needed data for a submodule checkout, when sharing a sha1 without telling
which exact branch it belongs to and in Gerrit, if you think in terms of
commits instead of change numbers. (The Gerrit case has already been
solved through allowTipSHA1InWant as every Gerrit change has a ref.)

Signed-off-by: Fredrik Medley <redacted>
---
 Documentation/config.txt                          |  6 +++
 Documentation/technical/http-protocol.txt         |  3 +-
 Documentation/technical/protocol-capabilities.txt |  7 +++
 fetch-pack.c                                      | 10 ++++-
 t/t5516-fetch-push.sh                             | 55 +++++++++++++++++++++++
 upload-pack.c                                     | 22 +++++++--
 6 files changed, 97 insertions(+), 6 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1311383..b8215ba 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2547,6 +2547,12 @@ uploadpack.allowTipSHA1InWant::
 	of a hidden ref (by default, such a request is rejected).
 	see also `uploadpack.hideRefs`.
 
+uploadpack.allowReachableSHA1InWant::
+	Allow `upload-pack` to accept a fetch request that asks for an
+	object that is reachable from any ref tip. However, note that
+	calculating object reachability is computationally expensive.
+	Defaults to `false`.
+
 uploadpack.keepAlive::
 	When `upload-pack` has started `pack-objects`, there may be a
 	quiet period while `pack-objects` prepares the pack. Normally
diff --git a/Documentation/technical/http-protocol.txt b/Documentation/technical/http-protocol.txt
index 229f845..1c561bd 100644
--- a/Documentation/technical/http-protocol.txt
+++ b/Documentation/technical/http-protocol.txt
@@ -319,7 +319,8 @@ Servers SHOULD support all capabilities defined here.
 Clients MUST send at least one "want" command in the request body.
 Clients MUST NOT reference an id in a "want" command which did not
 appear in the response obtained through ref discovery unless the
-server advertises capability `allow-tip-sha1-in-want`.
+server advertises capability `allow-tip-sha1-in-want` or
+`allow-reachable-sha1-in-want`.
 
   compute_request   =  want_list
 		       have_list
diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt
index 4f8a7bf..265fcab 100644
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -260,6 +260,13 @@ If the upload-pack server advertises this capability, fetch-pack may
 send "want" lines with SHA-1s that exist at the server but are not
 advertised by upload-pack.
 
+allow-reachable-sha1-in-want
+----------------------
+
+If the upload-pack server advertises this capability, fetch-pack may
+send "want" lines with SHA-1s that exist at the server but are not
+advertised by upload-pack.
+
 push-cert=<nonce>
 -----------------
 
diff --git a/fetch-pack.c b/fetch-pack.c
index 699f586..0d83d47 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -46,6 +46,8 @@ static struct prio_queue rev_list = { compare_commits_by_commit_date };
 static int non_common_revs, multi_ack, use_sideband;
 /* Allow specifying sha1 if it is a ref tip. */
 #define ALLOW_TIP_SHA1	01
+/* Allow request of a sha1 if it is reachable from a ref (possibly hidden ref). */
+#define ALLOW_REACHABLE_SHA1	02
 static int allow_unadvertised_object_request;
 
 static void rev_list_push(struct commit *commit, int mark)
@@ -545,7 +547,8 @@ static void filter_refs(struct fetch_pack_args *args,
 	}
 
 	/* Append unmatched requests to the list */
-	if ((allow_unadvertised_object_request & ALLOW_TIP_SHA1)) {
+	if ((allow_unadvertised_object_request &
+	    (ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1))) {
 		for (i = 0; i < nr_sought; i++) {
 			unsigned char sha1[20];
 
@@ -826,6 +829,11 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
 			fprintf(stderr, "Server supports allow-tip-sha1-in-want\n");
 		allow_unadvertised_object_request |= ALLOW_TIP_SHA1;
 	}
+	if (server_supports("allow-reachable-sha1-in-want")) {
+		if (args->verbose)
+			fprintf(stderr, "Server supports allow-reachable-sha1-in-want\n");
+		allow_unadvertised_object_request |= ALLOW_REACHABLE_SHA1;
+	}
 	if (!server_supports("thin-pack"))
 		args->use_thin_pack = 0;
 	if (!server_supports("no-progress"))
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 8a5f236..fdcc114 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1120,6 +1120,61 @@ test_expect_success 'fetch exact SHA1' '
 	)
 '
 
+for configallowtipsha1inwant in true false
+do
+	test_expect_success "shallow fetch reachable SHA1 (but not a ref), allowtipsha1inwant=$configallowtipsha1inwant" '
+		mk_empty testrepo &&
+		(
+			cd testrepo &&
+			git config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&
+			git commit --allow-empty -m foo &&
+			git commit --allow-empty -m bar
+		) &&
+		SHA1=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&
+		mk_empty shallow &&
+		(
+			cd shallow &&
+			test_must_fail git fetch --depth=1 ../testrepo/.git $SHA1 &&
+			git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
+			git fetch --depth=1 ../testrepo/.git $SHA1 &&
+			git cat-file commit $SHA1 >/dev/null
+		)
+	'
+
+	test_expect_success "deny fetch unreachable SHA1, allowtipsha1inwant=$configallowtipsha1inwant" '
+		mk_empty testrepo &&
+		(
+			cd testrepo &&
+			git config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&
+			git commit --allow-empty -m foo &&
+			git commit --allow-empty -m bar &&
+			git commit --allow-empty -m xyz
+		)
+		SHA1_1=$(git --git-dir=testrepo/.git rev-parse HEAD^^) &&
+		SHA1_2=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&
+		SHA1_3=$(git --git-dir=testrepo/.git rev-parse HEAD) &&
+		(
+			cd testrepo &&
+			git reset --hard $SHA1_2 &&
+			git cat-file commit $SHA1_3 >/dev/null &&
+			git cat-file commit $SHA1_3 >/dev/null
+		) &&
+		mk_empty shallow &&
+		(
+			cd shallow &&
+			test_must_fail git fetch ../testrepo/.git $SHA1_3 &&
+			test_must_fail git fetch ../testrepo/.git $SHA1_1 &&
+			git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
+			git fetch ../testrepo/.git $SHA1_1 &&
+			git cat-file commit $SHA1_1 >/dev/null &&
+			test_must_fail git cat-file commit $SHA1_2 >/dev/null &&
+			git fetch ../testrepo/.git $SHA1_2 &&
+			git cat-file commit $SHA1_2 >/dev/null &&
+			test_must_fail git fetch ../testrepo/.git $SHA1_3
+		)
+	'
+done
+
 test_expect_success 'fetch follows tags by default' '
 	mk_test testrepo heads/master &&
 	rm -fr src dst &&
diff --git a/upload-pack.c b/upload-pack.c
index 726486b..15571d7 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -37,6 +37,8 @@ static int use_thin_pack, use_ofs_delta, use_include_tag;
 static int no_progress, daemon_mode;
 /* Allow specifying sha1 if it is a ref tip. */
 #define ALLOW_TIP_SHA1	01
+/* Allow request of a sha1 if it is reachable from a ref (possibly hidden ref). */
+#define ALLOW_REACHABLE_SHA1	02
 static int allow_unadvertised_object_request;
 static int shallow_nr;
 static struct object_array have_obj;
@@ -444,7 +446,8 @@ static int get_common_commits(void)
 
 static int is_our_ref(struct object *o)
 {
-	int allow_hidden_ref = (allow_unadvertised_object_request & ALLOW_TIP_SHA1);
+	int allow_hidden_ref = (allow_unadvertised_object_request &
+			(ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1));
 	return o->flags & ((allow_hidden_ref ? HIDDEN_REF : 0) | OUR_REF);
 }
 
@@ -458,8 +461,12 @@ static void check_non_tip(void)
 	char namebuf[42]; /* ^ + SHA-1 + LF */
 	int i;
 
-	/* In the normal in-process case non-tip request can never happen */
-	if (!stateless_rpc)
+	/*
+	 * In the normal in-process case without
+	 * uploadpack.allowReachableSHA1InWant,
+	 * non-tip requests can never happen.
+	 */
+	if (!stateless_rpc && !(allow_unadvertised_object_request & ALLOW_REACHABLE_SHA1))
 		goto error;
 
 	cmd.argv = argv;
@@ -726,11 +733,13 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo
 		struct strbuf symref_info = STRBUF_INIT;
 
 		format_symref_info(&symref_info, cb_data);
-		packet_write(1, "%s %s%c%s%s%s%s agent=%s\n",
+		packet_write(1, "%s %s%c%s%s%s%s%s agent=%s\n",
 			     sha1_to_hex(sha1), refname_nons,
 			     0, capabilities,
 			     (allow_unadvertised_object_request & ALLOW_TIP_SHA1) ?
 				     " allow-tip-sha1-in-want" : "",
+			     (allow_unadvertised_object_request & ALLOW_REACHABLE_SHA1) ?
+				     " allow-reachable-sha1-in-want" : "",
 			     stateless_rpc ? " no-done" : "",
 			     symref_info.buf,
 			     git_user_agent_sanitized());
@@ -795,6 +804,11 @@ static int upload_pack_config(const char *var, const char *value, void *unused)
 			allow_unadvertised_object_request |= ALLOW_TIP_SHA1;
 		else
 			allow_unadvertised_object_request &= ~ALLOW_TIP_SHA1;
+	} else if (!strcmp("uploadpack.allowreachablesha1inwant", var)) {
+		if (git_config_bool(var, value))
+			allow_unadvertised_object_request |= ALLOW_REACHABLE_SHA1;
+		else
+			allow_unadvertised_object_request &= ~ALLOW_REACHABLE_SHA1;
 	} else if (!strcmp("uploadpack.keepalive", var)) {
 		keepalive = git_config_int(var, value);
 		if (!keepalive)
-- 
1.9.1

[PATCH v5 1/3] config.txt: clarify allowTipSHA1InWant with camelCase

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

Most of the options in config.txt are camelCase. Improve the readability
for allowtipsha1inwant by changing to allowTipSHA1InWant.

Signed-off-by: Fredrik Medley <redacted>
---
This patch is optional. There has been work on fixing the whole
Documentation/config.txt which has not been merged yet. When adding
allowReachableSHA1InWant later, it would be good to already have changed
to allowTipSHA1InWant.

 Documentation/config.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 948b8b0..1311383 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2539,9 +2539,9 @@ uploadpack.hideRefs::
 	are under the hierarchies listed on the value of this
 	variable is excluded, and is hidden from `git ls-remote`,
 	`git fetch`, etc.  An attempt to fetch a hidden ref by `git
-	fetch` will fail.  See also `uploadpack.allowtipsha1inwant`.
+	fetch` will fail.  See also `uploadpack.allowTipSHA1InWant`.
 
-uploadpack.allowtipsha1inwant::
+uploadpack.allowTipSHA1InWant::
 	When `uploadpack.hideRefs` is in effect, allow `upload-pack`
 	to accept a fetch request that asks for an object at the tip
 	of a hidden ref (by default, such a request is rejected).
-- 
1.9.1

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

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

Fredrik Medley [off-list ref] writes:
quoted hunk
To allow future extensions, e.g. allowing non-tip sha1, replace the
boolean allow_tip_sha1_in_want variable with the flag-style
allow_request_with_bare_object_name variable.

Signed-off-by: Fredrik Medley <redacted>
---
 fetch-pack.c  |  9 ++++++---
 upload-pack.c | 20 +++++++++++++-------
 2 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/fetch-pack.c b/fetch-pack.c
index 48526aa..699f586 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_SHA1	01
+static int allow_unadvertised_object_request;
It is better to use "unsigned int" for these bit masks, as we are
not interested in the top-most bit getting special-cased by using a
signed type.  I'll amend this (and the one in upload-pack.c) while
applying, so no need to resend only to correct these two, unless you
have other reasons to reroll.

Thanks.
quoted hunk
diff --git a/upload-pack.c b/upload-pack.c
index 745fda8..726486b 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_SHA1	01
+static int allow_unadvertised_object_request;

Re: [PATCH v5 3/3] upload-pack: optionally allow fetching reachable sha1

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

Fredrik Medley [off-list ref] writes:
quoted hunk
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -260,6 +260,13 @@ If the upload-pack server advertises this capability, fetch-pack may
 send "want" lines with SHA-1s that exist at the server but are not
 advertised by upload-pack.
 
+allow-reachable-sha1-in-want
+----------------------
This is an underline applied to one line prior, and their length
must match.  I'll amend while applying (attached at end), so there
is no need to resend with correction unless you have other reasons
to do so.
quoted hunk
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 8a5f236..fdcc114 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1120,6 +1120,61 @@ test_expect_success 'fetch exact SHA1' '
 	)
 '
It looks like this new set of tests are well thought out; good job.

I spotted a few minor nits, though.  All I'll amend while applying
so there is no need to resend only to correct them.
+for configallowtipsha1inwant in true false
+do
+	test_expect_success "shallow fetch reachable SHA1 (but not a ref), allowtipsha1inwant=$configallowtipsha1inwant" '
+		mk_empty testrepo &&
+		(
+			cd testrepo &&
+			git config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&
+			git commit --allow-empty -m foo &&
+			git commit --allow-empty -m bar
+		) &&
+		SHA1=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&
+		mk_empty shallow &&
+		(
+			cd shallow &&
+			test_must_fail git fetch --depth=1 ../testrepo/.git $SHA1 &&
This tries to fetch one before the tip with "allowTipSHA1InWant" set
to true or false; either should fail.  Good.
+			git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
+			git fetch --depth=1 ../testrepo/.git $SHA1 &&
And regardless of allowTip setting, with allowReachable set to true,
fetching the reachable HEAD^ would succeed.  Good.
+			git cat-file commit $SHA1 >/dev/null
Minor nit; drop ">/dev/null", as test framework will squelch the
output by default, and when the test is run with "-v" option, the
output would help debugging the script.
+		)
+	'
+
+	test_expect_success "deny fetch unreachable SHA1, allowtipsha1inwant=$configallowtipsha1inwant" '
+		mk_empty testrepo &&
+		(
+			cd testrepo &&
+			git config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&
+			git commit --allow-empty -m foo &&
+			git commit --allow-empty -m bar &&
+			git commit --allow-empty -m xyz
+		)
Broken && chain
+		SHA1_1=$(git --git-dir=testrepo/.git rev-parse HEAD^^) &&
+		SHA1_2=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&
+		SHA1_3=$(git --git-dir=testrepo/.git rev-parse HEAD) &&
+		(
+			cd testrepo &&
+			git reset --hard $SHA1_2 &&
We have one before the tip (SHA1_1), one after the tip and no longer
reachable (SHA1_3); SHA1_2 is sitting at the tip of a ref.
+			git cat-file commit $SHA1_3 >/dev/null &&
+			git cat-file commit $SHA1_3 >/dev/null
I think one of the latter two is $SHA1_1, i.e. you make sure SHA1_{1,2,3}
are there in testrepo.
+		) &&
+		mk_empty shallow &&
+		(
+			cd shallow &&
+			test_must_fail git fetch ../testrepo/.git $SHA1_3 &&
+			test_must_fail git fetch ../testrepo/.git $SHA1_1 &&
With allowTip only, whether it is set to true or false, fetching _1
or _3 that are not at tip will fail.  Good.
+			git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
+			git fetch ../testrepo/.git $SHA1_1 &&
+			git cat-file commit $SHA1_1 >/dev/null &&
With allowReachable, _1 which is reachable from tip is possible,
regardless of the setting of allowTip.  Good.
+			test_must_fail git cat-file commit $SHA1_2 >/dev/null &&
And fetching _1 will not pull in _2, which is _1's child, that we
did not ask for.  Good (but it is probably not very relevant for the
purpose of these tests).
+			git fetch ../testrepo/.git $SHA1_2 &&
+			git cat-file commit $SHA1_2 >/dev/null &&
And of course, _2 can be fetched.
+			test_must_fail git fetch ../testrepo/.git $SHA1_3
And _3 that is not reachable cannot.
+		)
+	'
+done
Again, very carefully covering combinations.  Good.




 Documentation/technical/protocol-capabilities.txt |  2 +-
 t/t5516-fetch-push.sh                             | 14 +++++++-------
 2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt
index 265fcab..eaab6b4 100644
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -261,7 +261,7 @@ send "want" lines with SHA-1s that exist at the server but are not
 advertised by upload-pack.
 
 allow-reachable-sha1-in-want
-----------------------
+----------------------------
 
 If the upload-pack server advertises this capability, fetch-pack may
 send "want" lines with SHA-1s that exist at the server but are not
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index fdcc114..ec22c98 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1137,7 +1137,7 @@ do
 			test_must_fail git fetch --depth=1 ../testrepo/.git $SHA1 &&
 			git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
 			git fetch --depth=1 ../testrepo/.git $SHA1 &&
-			git cat-file commit $SHA1 >/dev/null
+			git cat-file commit $SHA1
 		)
 	'
 
@@ -1149,15 +1149,15 @@ do
 			git commit --allow-empty -m foo &&
 			git commit --allow-empty -m bar &&
 			git commit --allow-empty -m xyz
-		)
+		) &&
 		SHA1_1=$(git --git-dir=testrepo/.git rev-parse HEAD^^) &&
 		SHA1_2=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&
 		SHA1_3=$(git --git-dir=testrepo/.git rev-parse HEAD) &&
 		(
 			cd testrepo &&
 			git reset --hard $SHA1_2 &&
-			git cat-file commit $SHA1_3 >/dev/null &&
-			git cat-file commit $SHA1_3 >/dev/null
+			git cat-file commit $SHA1_1 &&
+			git cat-file commit $SHA1_3
 		) &&
 		mk_empty shallow &&
 		(
@@ -1166,10 +1166,10 @@ do
 			test_must_fail git fetch ../testrepo/.git $SHA1_1 &&
 			git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
 			git fetch ../testrepo/.git $SHA1_1 &&
-			git cat-file commit $SHA1_1 >/dev/null &&
-			test_must_fail git cat-file commit $SHA1_2 >/dev/null &&
+			git cat-file commit $SHA1_1 &&
+			test_must_fail git cat-file commit $SHA1_2 &&
 			git fetch ../testrepo/.git $SHA1_2 &&
-			git cat-file commit $SHA1_2 >/dev/null &&
+			git cat-file commit $SHA1_2 &&
 			test_must_fail git fetch ../testrepo/.git $SHA1_3
 		)
 	'
-- 
2.4.1-439-gcfa393f

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

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

2015-05-22 0:07 GMT+02:00 Junio C Hamano [off-list ref]:
Fredrik Medley [off-list ref] writes:
quoted
To allow future extensions, e.g. allowing non-tip sha1, replace the
boolean allow_tip_sha1_in_want variable with the flag-style
allow_request_with_bare_object_name variable.

Signed-off-by: Fredrik Medley <redacted>
---
 fetch-pack.c  |  9 ++++++---
 upload-pack.c | 20 +++++++++++++-------
 2 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/fetch-pack.c b/fetch-pack.c
index 48526aa..699f586 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_SHA1       01
+static int allow_unadvertised_object_request;
It is better to use "unsigned int" for these bit masks, as we are
not interested in the top-most bit getting special-cased by using a
signed type.  I'll amend this (and the one in upload-pack.c) while
applying, so no need to resend only to correct these two, unless you
have other reasons to reroll.
Sounds like a good idea to change. Please amend it while applying.
Thanks.
Thank you too for the review.
quoted
diff --git a/upload-pack.c b/upload-pack.c
index 745fda8..726486b 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_SHA1       01
+static int allow_unadvertised_object_request;

Re: [PATCH v5 3/3] upload-pack: optionally allow fetching reachable sha1

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

2015-05-22 0:15 GMT+02:00 Junio C Hamano [off-list ref]:
Fredrik Medley [off-list ref] writes:
quoted
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -260,6 +260,13 @@ If the upload-pack server advertises this capability, fetch-pack may
 send "want" lines with SHA-1s that exist at the server but are not
 advertised by upload-pack.

+allow-reachable-sha1-in-want
+----------------------
This is an underline applied to one line prior, and their length
must match.  I'll amend while applying (attached at end), so there
is no need to resend with correction unless you have other reasons
to do so.
quoted
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 8a5f236..fdcc114 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1120,6 +1120,61 @@ test_expect_success 'fetch exact SHA1' '
      )
 '
It looks like this new set of tests are well thought out; good job.

I spotted a few minor nits, though.  All I'll amend while applying
so there is no need to resend only to correct them.
I agree on all your comments and your proposed amendment further down
looks good.
Should the test code contain the explanations you've written in this email?
quoted
+for configallowtipsha1inwant in true false
+do
+     test_expect_success "shallow fetch reachable SHA1 (but not a ref), allowtipsha1inwant=$configallowtipsha1inwant" '
+             mk_empty testrepo &&
+             (
+                     cd testrepo &&
+                     git config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&
+                     git commit --allow-empty -m foo &&
+                     git commit --allow-empty -m bar
+             ) &&
+             SHA1=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&
+             mk_empty shallow &&
+             (
+                     cd shallow &&
+                     test_must_fail git fetch --depth=1 ../testrepo/.git $SHA1 &&
This tries to fetch one before the tip with "allowTipSHA1InWant" set
to true or false; either should fail.  Good.
quoted
+                     git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
+                     git fetch --depth=1 ../testrepo/.git $SHA1 &&
And regardless of allowTip setting, with allowReachable set to true,
fetching the reachable HEAD^ would succeed.  Good.
quoted
+                     git cat-file commit $SHA1 >/dev/null
Minor nit; drop ">/dev/null", as test framework will squelch the
output by default, and when the test is run with "-v" option, the
output would help debugging the script.
quoted
+             )
+     '
+
+     test_expect_success "deny fetch unreachable SHA1, allowtipsha1inwant=$configallowtipsha1inwant" '
+             mk_empty testrepo &&
+             (
+                     cd testrepo &&
+                     git config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&
+                     git commit --allow-empty -m foo &&
+                     git commit --allow-empty -m bar &&
+                     git commit --allow-empty -m xyz
+             )
Broken && chain
quoted
+             SHA1_1=$(git --git-dir=testrepo/.git rev-parse HEAD^^) &&
+             SHA1_2=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&
+             SHA1_3=$(git --git-dir=testrepo/.git rev-parse HEAD) &&
+             (
+                     cd testrepo &&
+                     git reset --hard $SHA1_2 &&
We have one before the tip (SHA1_1), one after the tip and no longer
reachable (SHA1_3); SHA1_2 is sitting at the tip of a ref.
quoted
+                     git cat-file commit $SHA1_3 >/dev/null &&
+                     git cat-file commit $SHA1_3 >/dev/null
I think one of the latter two is $SHA1_1, i.e. you make sure SHA1_{1,2,3}
are there in testrepo.
Yes, that was intended.
quoted hunk
quoted
+             ) &&
+             mk_empty shallow &&
+             (
+                     cd shallow &&
+                     test_must_fail git fetch ../testrepo/.git $SHA1_3 &&
+                     test_must_fail git fetch ../testrepo/.git $SHA1_1 &&
With allowTip only, whether it is set to true or false, fetching _1
or _3 that are not at tip will fail.  Good.
quoted
+                     git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
+                     git fetch ../testrepo/.git $SHA1_1 &&
+                     git cat-file commit $SHA1_1 >/dev/null &&
With allowReachable, _1 which is reachable from tip is possible,
regardless of the setting of allowTip.  Good.
quoted
+                     test_must_fail git cat-file commit $SHA1_2 >/dev/null &&
And fetching _1 will not pull in _2, which is _1's child, that we
did not ask for.  Good (but it is probably not very relevant for the
purpose of these tests).
quoted
+                     git fetch ../testrepo/.git $SHA1_2 &&
+                     git cat-file commit $SHA1_2 >/dev/null &&
And of course, _2 can be fetched.
quoted
+                     test_must_fail git fetch ../testrepo/.git $SHA1_3
And _3 that is not reachable cannot.
quoted
+             )
+     '
+done
Again, very carefully covering combinations.  Good.




 Documentation/technical/protocol-capabilities.txt |  2 +-
 t/t5516-fetch-push.sh                             | 14 +++++++-------
 2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt
index 265fcab..eaab6b4 100644
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -261,7 +261,7 @@ send "want" lines with SHA-1s that exist at the server but are not
 advertised by upload-pack.

 allow-reachable-sha1-in-want
-----------------------
+----------------------------

 If the upload-pack server advertises this capability, fetch-pack may
 send "want" lines with SHA-1s that exist at the server but are not
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index fdcc114..ec22c98 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1137,7 +1137,7 @@ do
                        test_must_fail git fetch --depth=1 ../testrepo/.git $SHA1 &&
                        git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
                        git fetch --depth=1 ../testrepo/.git $SHA1 &&
-                       git cat-file commit $SHA1 >/dev/null
+                       git cat-file commit $SHA1
                )
        '
@@ -1149,15 +1149,15 @@ do
                        git commit --allow-empty -m foo &&
                        git commit --allow-empty -m bar &&
                        git commit --allow-empty -m xyz
-               )
+               ) &&
                SHA1_1=$(git --git-dir=testrepo/.git rev-parse HEAD^^) &&
                SHA1_2=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&
                SHA1_3=$(git --git-dir=testrepo/.git rev-parse HEAD) &&
                (
                        cd testrepo &&
                        git reset --hard $SHA1_2 &&
-                       git cat-file commit $SHA1_3 >/dev/null &&
-                       git cat-file commit $SHA1_3 >/dev/null
+                       git cat-file commit $SHA1_1 &&
+                       git cat-file commit $SHA1_3
                ) &&
                mk_empty shallow &&
                (
@@ -1166,10 +1166,10 @@ do
                        test_must_fail git fetch ../testrepo/.git $SHA1_1 &&
                        git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
                        git fetch ../testrepo/.git $SHA1_1 &&
-                       git cat-file commit $SHA1_1 >/dev/null &&
-                       test_must_fail git cat-file commit $SHA1_2 >/dev/null &&
+                       git cat-file commit $SHA1_1 &&
+                       test_must_fail git cat-file commit $SHA1_2 &&
                        git fetch ../testrepo/.git $SHA1_2 &&
-                       git cat-file commit $SHA1_2 >/dev/null &&
+                       git cat-file commit $SHA1_2 &&
                        test_must_fail git fetch ../testrepo/.git $SHA1_3
                )
        '
--
2.4.1-439-gcfa393f
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help