* [PATCH 2/3] upload-pack: Prepare to extend allow-tip-sha1-in-want
2015-05-12 21:14 ` [PATCH 1/3] config.txt: Clarify allowTipSHA1InWant with camelCase Fredrik Medley
@ 2015-05-12 21:14 ` Fredrik Medley
2015-05-12 21:37 ` Eric Sunshine
2015-05-12 21:39 ` Junio C Hamano
2015-05-12 21:14 ` [PATCH 3/3] upload-pack: Optionally allow fetching reachable sha1 Fredrik Medley
` (2 subsequent siblings)
3 siblings, 2 replies; 34+ messages in thread
From: Fredrik Medley @ 2015-05-12 21:14 UTC (permalink / raw)
To: git
Cc: Fredrik Medley, Christian Halstrick, Dan Johnson, Jeff King,
Jonathan Nieder, Duy Nguyen, Junio C Hamano
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
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 2/3] upload-pack: Prepare to extend allow-tip-sha1-in-want
2015-05-12 21:14 ` [PATCH 2/3] upload-pack: Prepare to extend allow-tip-sha1-in-want Fredrik Medley
@ 2015-05-12 21:37 ` Eric Sunshine
2015-05-12 21:39 ` Junio C Hamano
1 sibling, 0 replies; 34+ messages in thread
From: Eric Sunshine @ 2015-05-12 21:37 UTC (permalink / raw)
To: Fredrik Medley
Cc: Git List, Christian Halstrick, Dan Johnson, Jeff King,
Jonathan Nieder, Duy Nguyen, Junio C Hamano
On Tue, May 12, 2015 at 5:14 PM, Fredrik Medley
<fredrik.medley@gmail.com> 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.
> ---
> 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.
> 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
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 2/3] upload-pack: Prepare to extend allow-tip-sha1-in-want
2015-05-12 21:14 ` [PATCH 2/3] upload-pack: Prepare to extend allow-tip-sha1-in-want Fredrik Medley
2015-05-12 21:37 ` Eric Sunshine
@ 2015-05-12 21:39 ` Junio C Hamano
2015-05-19 20:19 ` Fredrik Medley
1 sibling, 1 reply; 34+ messages in thread
From: Junio C Hamano @ 2015-05-12 21:39 UTC (permalink / raw)
To: Fredrik Medley
Cc: git, Christian Halstrick, Dan Johnson, Jeff King,
Jonathan Nieder, Duy Nguyen
Fredrik Medley <fredrik.medley@gmail.com> 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?
> 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.
> 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?
> 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;
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 2/3] upload-pack: Prepare to extend allow-tip-sha1-in-want
2015-05-12 21:39 ` Junio C Hamano
@ 2015-05-19 20:19 ` Fredrik Medley
0 siblings, 0 replies; 34+ messages in thread
From: Fredrik Medley @ 2015-05-19 20:19 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Christian Halstrick, Dan Johnson, Jeff King,
Jonathan Nieder, Duy Nguyen
2015-05-12 23:39 GMT+02:00 Junio C Hamano <gitster@pobox.com>:
> Fredrik Medley <fredrik.medley@gmail.com> writes:
>
>> 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.
>
>> 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.
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 3/3] upload-pack: Optionally allow fetching reachable sha1
2015-05-12 21:14 ` [PATCH 1/3] config.txt: Clarify allowTipSHA1InWant with camelCase Fredrik Medley
2015-05-12 21:14 ` [PATCH 2/3] upload-pack: Prepare to extend allow-tip-sha1-in-want Fredrik Medley
@ 2015-05-12 21:14 ` Fredrik Medley
2015-05-12 22:01 ` Eric Sunshine
2015-05-12 21:24 ` [PATCH 1/3] config.txt: Clarify allowTipSHA1InWant with camelCase Eric Sunshine
2015-05-12 21:35 ` Junio C Hamano
3 siblings, 1 reply; 34+ messages in thread
From: Fredrik Medley @ 2015-05-12 21:14 UTC (permalink / raw)
To: git
Cc: Fredrik Medley, Christian Halstrick, Dan Johnson, Jeff King,
Jonathan Nieder, Duy Nguyen, Junio C Hamano
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 <fredrik.medley@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 3/3] upload-pack: Optionally allow fetching reachable sha1
2015-05-12 21:14 ` [PATCH 3/3] upload-pack: Optionally allow fetching reachable sha1 Fredrik Medley
@ 2015-05-12 22:01 ` Eric Sunshine
2015-05-19 20:44 ` [PATCH 1/3] config.txt: clarify allowTipSHA1InWant with camelCase Fredrik Medley
0 siblings, 1 reply; 34+ messages in thread
From: Eric Sunshine @ 2015-05-12 22:01 UTC (permalink / raw)
To: Fredrik Medley
Cc: Git List, Christian Halstrick, Dan Johnson, Jeff King,
Jonathan Nieder, Duy Nguyen, Junio C Hamano
On Tue, May 12, 2015 at 5:14 PM, Fredrik Medley
<fredrik.medley@gmail.com> 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/
> 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 <fredrik.medley@gmail.com>
> ---
> 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+/ /
> + 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
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 1/3] config.txt: clarify allowTipSHA1InWant with camelCase
2015-05-12 22:01 ` Eric Sunshine
@ 2015-05-19 20:44 ` Fredrik Medley
2015-05-19 20:44 ` [PATCH 2/3] upload-pack: prepare to extend allow-tip-sha1-in-want Fredrik Medley
` (2 more replies)
0 siblings, 3 replies; 34+ messages in thread
From: Fredrik Medley @ 2015-05-19 20:44 UTC (permalink / raw)
To: git
Cc: Fredrik Medley, Christian Halstrick, Dan Johnson, Jeff King,
Jonathan Nieder, Duy Nguyen, Junio C Hamano
Most of the options in config.txt are camelCase. Improve the readability
for allowtipsha1inwant by changing to allowTipSHA1InWant.
Signed-off-by: Fredrik Medley <fredrik.medley@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 2/3] upload-pack: prepare to extend allow-tip-sha1-in-want
2015-05-19 20:44 ` [PATCH 1/3] config.txt: clarify allowTipSHA1InWant with camelCase Fredrik Medley
@ 2015-05-19 20:44 ` Fredrik Medley
2015-05-19 22:00 ` Junio C Hamano
2015-05-19 20:44 ` [PATCH 3/3] upload-pack: optionally allow fetching reachable sha1 Fredrik Medley
2015-05-21 20:23 ` [PATCH v5 1/3] config.txt: clarify allowTipSHA1InWant with camelCase Fredrik Medley
2 siblings, 1 reply; 34+ messages in thread
From: Fredrik Medley @ 2015-05-19 20:44 UTC (permalink / raw)
To: git
Cc: Fredrik Medley, Christian Halstrick, Dan Johnson, Jeff King,
Jonathan Nieder, Duy Nguyen, Junio C Hamano
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 <fredrik.medley@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 2/3] upload-pack: prepare to extend allow-tip-sha1-in-want
2015-05-19 20:44 ` [PATCH 2/3] upload-pack: prepare to extend allow-tip-sha1-in-want Fredrik Medley
@ 2015-05-19 22:00 ` Junio C Hamano
2015-05-20 19:31 ` Fredrik Medley
0 siblings, 1 reply; 34+ messages in thread
From: Junio C Hamano @ 2015-05-19 22:00 UTC (permalink / raw)
To: Fredrik Medley
Cc: git, Christian Halstrick, Dan Johnson, Jeff King,
Jonathan Nieder, Duy Nguyen
Fredrik Medley <fredrik.medley@gmail.com> writes:
> 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;
Doesn't this change break the behaviour?
Shouldn't you be able to say
[uploadpack]
allowTipSHA1InWant = false
in a higher-precedence configuration file to override the same
variable in other files in the configuration chain that may set it
to true?
> + } else if (!strcmp("uploadpack.keepalive", var)) {
> keepalive = git_config_int(var, value);
> if (!keepalive)
> keepalive = -1;
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 2/3] upload-pack: prepare to extend allow-tip-sha1-in-want
2015-05-19 22:00 ` Junio C Hamano
@ 2015-05-20 19:31 ` Fredrik Medley
0 siblings, 0 replies; 34+ messages in thread
From: Fredrik Medley @ 2015-05-20 19:31 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Christian Halstrick, Dan Johnson, Jeff King,
Jonathan Nieder, Duy Nguyen
2015-05-20 0:00 GMT+02:00 Junio C Hamano <gitster@pobox.com>:
> Fredrik Medley <fredrik.medley@gmail.com> writes:
>
>> 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;
>
> Doesn't this change break the behaviour?
>
> Shouldn't you be able to say
>
> [uploadpack]
> allowTipSHA1InWant = false
>
> in a higher-precedence configuration file to override the same
> variable in other files in the configuration chain that may set it
> to true?
Of course, thought it work differently. Should I add tests with
test_config_global
to check that loading the config works as well?
>
>> + } else if (!strcmp("uploadpack.keepalive", var)) {
>> keepalive = git_config_int(var, value);
>> if (!keepalive)
>> keepalive = -1;
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 3/3] upload-pack: optionally allow fetching reachable sha1
2015-05-19 20:44 ` [PATCH 1/3] config.txt: clarify allowTipSHA1InWant with camelCase Fredrik Medley
2015-05-19 20:44 ` [PATCH 2/3] upload-pack: prepare to extend allow-tip-sha1-in-want Fredrik Medley
@ 2015-05-19 20:44 ` Fredrik Medley
2015-05-19 22:06 ` Junio C Hamano
2015-05-21 20:23 ` [PATCH v5 1/3] config.txt: clarify allowTipSHA1InWant with camelCase Fredrik Medley
2 siblings, 1 reply; 34+ messages in thread
From: Fredrik Medley @ 2015-05-19 20:44 UTC (permalink / raw)
To: git
Cc: Fredrik Medley, Christian Halstrick, Dan Johnson, Jeff King,
Jonathan Nieder, Duy Nguyen, Junio C Hamano
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 <fredrik.medley@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 3/3] upload-pack: optionally allow fetching reachable sha1
2015-05-19 20:44 ` [PATCH 3/3] upload-pack: optionally allow fetching reachable sha1 Fredrik Medley
@ 2015-05-19 22:06 ` Junio C Hamano
0 siblings, 0 replies; 34+ messages in thread
From: Junio C Hamano @ 2015-05-19 22:06 UTC (permalink / raw)
To: Fredrik Medley
Cc: git, Christian Halstrick, Dan Johnson, Jeff King,
Jonathan Nieder, Duy Nguyen
Fredrik Medley <fredrik.medley@gmail.com> writes:
> 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
What is this change about? You do not have to align value columns.
> - if ((allow_unadvertised_object_request & ALLOW_TIP_SHA1)) {
> + if ((allow_unadvertised_object_request & (ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1))) {
Perhaps wrap it like this?
if ((allow_unadvertised_object_request &
(ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1))) {
> @@ -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));
Good, but push that to even more right?
> @@ -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)
Same issue as [2/3]
...
} else if (!strcmp("configVariable", var)) {
if (git_config_bool(var, value))
value |= ALLOW_THIS;
else
value &= ~ALLOW_THIS;
} else if ...
should be OK.
Thanks.
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v5 1/3] config.txt: clarify allowTipSHA1InWant with camelCase
2015-05-19 20:44 ` [PATCH 1/3] config.txt: clarify allowTipSHA1InWant with camelCase Fredrik Medley
2015-05-19 20:44 ` [PATCH 2/3] upload-pack: prepare to extend allow-tip-sha1-in-want Fredrik Medley
2015-05-19 20:44 ` [PATCH 3/3] upload-pack: optionally allow fetching reachable sha1 Fredrik Medley
@ 2015-05-21 20:23 ` Fredrik Medley
2015-05-21 20:23 ` [PATCH v5 2/3] upload-pack: prepare to extend allow-tip-sha1-in-want Fredrik Medley
2015-05-21 20:23 ` [PATCH v5 3/3] upload-pack: optionally allow fetching reachable sha1 Fredrik Medley
2 siblings, 2 replies; 34+ messages in thread
From: Fredrik Medley @ 2015-05-21 20:23 UTC (permalink / raw)
To: git
Cc: Fredrik Medley, Christian Halstrick, Dan Johnson, Jeff King,
Jonathan Nieder, Duy Nguyen, Junio C Hamano
Most of the options in config.txt are camelCase. Improve the readability
for allowtipsha1inwant by changing to allowTipSHA1InWant.
Signed-off-by: Fredrik Medley <fredrik.medley@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH v5 2/3] upload-pack: prepare to extend allow-tip-sha1-in-want
2015-05-21 20:23 ` [PATCH v5 1/3] config.txt: clarify allowTipSHA1InWant with camelCase Fredrik Medley
@ 2015-05-21 20:23 ` Fredrik Medley
2015-05-21 22:07 ` Junio C Hamano
2015-05-21 20:23 ` [PATCH v5 3/3] upload-pack: optionally allow fetching reachable sha1 Fredrik Medley
1 sibling, 1 reply; 34+ messages in thread
From: Fredrik Medley @ 2015-05-21 20:23 UTC (permalink / raw)
To: git
Cc: Fredrik Medley, Christian Halstrick, Dan Johnson, Jeff King,
Jonathan Nieder, Duy Nguyen, Junio C Hamano
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 <fredrik.medley@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH v5 2/3] upload-pack: prepare to extend allow-tip-sha1-in-want
2015-05-21 20:23 ` [PATCH v5 2/3] upload-pack: prepare to extend allow-tip-sha1-in-want Fredrik Medley
@ 2015-05-21 22:07 ` Junio C Hamano
2015-05-22 23:47 ` Fredrik Medley
0 siblings, 1 reply; 34+ messages in thread
From: Junio C Hamano @ 2015-05-21 22:07 UTC (permalink / raw)
To: Fredrik Medley
Cc: git, Christian Halstrick, Dan Johnson, Jeff King,
Jonathan Nieder, Duy Nguyen
Fredrik Medley <fredrik.medley@gmail.com> writes:
> 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 <fredrik.medley@gmail.com>
> ---
> 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.
> 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;
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v5 2/3] upload-pack: prepare to extend allow-tip-sha1-in-want
2015-05-21 22:07 ` Junio C Hamano
@ 2015-05-22 23:47 ` Fredrik Medley
2015-05-23 0:53 ` Junio C Hamano
0 siblings, 1 reply; 34+ messages in thread
From: Fredrik Medley @ 2015-05-22 23:47 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Christian Halstrick, Dan Johnson, Jeff King,
Jonathan Nieder, Duy Nguyen
2015-05-22 0:07 GMT+02:00 Junio C Hamano <gitster@pobox.com>:
> Fredrik Medley <fredrik.medley@gmail.com> writes:
>
>> 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 <fredrik.medley@gmail.com>
>> ---
>> 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.
>
>> 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;
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v5 2/3] upload-pack: prepare to extend allow-tip-sha1-in-want
2015-05-22 23:47 ` Fredrik Medley
@ 2015-05-23 0:53 ` Junio C Hamano
0 siblings, 0 replies; 34+ messages in thread
From: Junio C Hamano @ 2015-05-23 0:53 UTC (permalink / raw)
To: Fredrik Medley
Cc: git, Christian Halstrick, Dan Johnson, Jeff King,
Jonathan Nieder, Duy Nguyen
Fredrik Medley <fredrik.medley@gmail.com> writes:
>>> +#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.
Alright, will do.
Thanks.
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v5 3/3] upload-pack: optionally allow fetching reachable sha1
2015-05-21 20:23 ` [PATCH v5 1/3] config.txt: clarify allowTipSHA1InWant with camelCase Fredrik Medley
2015-05-21 20:23 ` [PATCH v5 2/3] upload-pack: prepare to extend allow-tip-sha1-in-want Fredrik Medley
@ 2015-05-21 20:23 ` Fredrik Medley
2015-05-21 22:15 ` Junio C Hamano
1 sibling, 1 reply; 34+ messages in thread
From: Fredrik Medley @ 2015-05-21 20:23 UTC (permalink / raw)
To: git
Cc: Fredrik Medley, Christian Halstrick, Dan Johnson, Jeff King,
Jonathan Nieder, Duy Nguyen, Junio C Hamano
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 <fredrik.medley@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH v5 3/3] upload-pack: optionally allow fetching reachable sha1
2015-05-21 20:23 ` [PATCH v5 3/3] upload-pack: optionally allow fetching reachable sha1 Fredrik Medley
@ 2015-05-21 22:15 ` Junio C Hamano
2015-05-22 23:55 ` Fredrik Medley
0 siblings, 1 reply; 34+ messages in thread
From: Junio C Hamano @ 2015-05-21 22:15 UTC (permalink / raw)
To: Fredrik Medley
Cc: git, Christian Halstrick, Dan Johnson, Jeff King,
Jonathan Nieder, Duy Nguyen
Fredrik Medley <fredrik.medley@gmail.com> writes:
> --- 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.
> 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
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH v5 3/3] upload-pack: optionally allow fetching reachable sha1
2015-05-21 22:15 ` Junio C Hamano
@ 2015-05-22 23:55 ` Fredrik Medley
2015-05-23 1:01 ` Junio C Hamano
0 siblings, 1 reply; 34+ messages in thread
From: Fredrik Medley @ 2015-05-22 23:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Christian Halstrick, Dan Johnson, Jeff King,
Jonathan Nieder, Duy Nguyen
2015-05-22 0:15 GMT+02:00 Junio C Hamano <gitster@pobox.com>:
> Fredrik Medley <fredrik.medley@gmail.com> writes:
>
>> --- 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.
>
>> 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?
>
>> +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.
Yes, that was intended.
>
>> + ) &&
>> + 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
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v5 3/3] upload-pack: optionally allow fetching reachable sha1
2015-05-22 23:55 ` Fredrik Medley
@ 2015-05-23 1:01 ` Junio C Hamano
0 siblings, 0 replies; 34+ messages in thread
From: Junio C Hamano @ 2015-05-23 1:01 UTC (permalink / raw)
To: Fredrik Medley
Cc: git, Christian Halstrick, Dan Johnson, Jeff King,
Jonathan Nieder, Duy Nguyen
Fredrik Medley <fredrik.medley@gmail.com> writes:
> 2015-05-22 0:15 GMT+02:00 Junio C Hamano <gitster@pobox.com>:
>
>> 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.
Thanks.
> Should the test code contain the explanations you've written in this email?
No, I don't think so.
I was just showing that thinking aloud while reviewing would be a
good way to do a review. The practice
(1) makes sure that the reviewer actually understood what the patch
wanted to do (and the reviewee can point out misunderstandings
if there are any); and
(2) shows others that the reviewer actually read the patch ;-).
The latter is primarily meant for other people who review the
patches. I want to see people get in the habit of responding with
something more than just a single-liner "Reviewed-by:", which I
often have hard time guessing if the reviewer really read the patch,
or just skimmed without spending effort to spot issues, and this
message was my attempt to lead with an example.
Will squash in the fix-up in the message you are responding to.
Let's move the topic to 'next' after that.
Thanks.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 1/3] config.txt: Clarify allowTipSHA1InWant with camelCase
2015-05-12 21:14 ` [PATCH 1/3] config.txt: Clarify allowTipSHA1InWant with camelCase Fredrik Medley
2015-05-12 21:14 ` [PATCH 2/3] upload-pack: Prepare to extend allow-tip-sha1-in-want Fredrik Medley
2015-05-12 21:14 ` [PATCH 3/3] upload-pack: Optionally allow fetching reachable sha1 Fredrik Medley
@ 2015-05-12 21:24 ` Eric Sunshine
2015-05-12 21:33 ` Junio C Hamano
2015-05-12 21:35 ` Junio C Hamano
3 siblings, 1 reply; 34+ messages in thread
From: Eric Sunshine @ 2015-05-12 21:24 UTC (permalink / raw)
To: Fredrik Medley
Cc: Git List, Christian Halstrick, Dan Johnson, Jeff King,
Jonathan Nieder, Duy Nguyen, Junio C Hamano
On Tue, May 12, 2015 at 5:14 PM, Fredrik Medley
<fredrik.medley@gmail.com> wrote:
> Most of the options in config.txt are camelCase. Improve the readability
> for allowtipsha1inwant by changing to allowTipSHA1InWant.
Missing sign-off. More below.
> ---
> 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
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 1/3] config.txt: Clarify allowTipSHA1InWant with camelCase
2015-05-12 21:14 ` [PATCH 1/3] config.txt: Clarify allowTipSHA1InWant with camelCase Fredrik Medley
` (2 preceding siblings ...)
2015-05-12 21:24 ` [PATCH 1/3] config.txt: Clarify allowTipSHA1InWant with camelCase Eric Sunshine
@ 2015-05-12 21:35 ` Junio C Hamano
3 siblings, 0 replies; 34+ messages in thread
From: Junio C Hamano @ 2015-05-12 21:35 UTC (permalink / raw)
To: Fredrik Medley
Cc: git, Christian Halstrick, Dan Johnson, Jeff King,
Jonathan Nieder, Duy Nguyen
Fredrik Medley <fredrik.medley@gmail.com> writes:
> Subject: Re: [PATCH 1/3] config.txt: Clarify allowTipSHA1InWant with camelCase
s/: Clarify/: clarify/;
> Most of the options in config.txt are camelCase. Improve the readability
> for allowtipsha1inwant by changing to allowTipSHA1InWant.
Sign-off?
> ---
> 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).
^ permalink raw reply [flat|nested] 34+ messages in thread