{"thread":{"id":"39224","subject":"[PATCH] upload-pack: Optionally allow fetching reachable sha1","startedAt":"2015-05-02T22:01:15Z","lastAt":"2015-05-23T01:01:08Z","messageCount":34,"participants":["Fredrik Medley","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"260438","messageId":"1430604075-5951-1-git-send-email-fredrik.medley@gmail.com","threadId":"39224","inReplyTo":null,"subject":"[PATCH] upload-pack: Optionally allow fetching reachable sha1","fromName":"Fredrik Medley","fromEmail":"fredrik.medley@gmail.com","sentAt":"2015-05-02T22:01:15Z","receivedAt":"2015-05-02T22:01:15Z","isPatch":true,"sender":{"key":"fredrik.medley@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5401799?v=4"},"body":"For you who don't remember the email discussion, look through the references.\n\nSince before, it is allowed to ask for a sha1, but most git servers refuse\nthe request[1]. This patch is based on a suggested location for\nimplementation[2] which has been updated according to ideas in the reply[3].\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/257809\n[2] http://thread.gmane.org/gmane.comp.version-control.git/258002\n[3] http://thread.gmane.org/gmane.comp.version-control.git/258080\n\n\nWith uploadpack.allowreachablesha1inwant configuration option set,\n\"git fetch\" can make a request with a \"want\" line that names an object\nthat has not been advertised (likely to have been obtained out of band or\nfrom a submodule pointer). Only objects reachable from the branch tips,\nhidden by transfer.hiderefs or not, will be processed.\n\nSigned-off-by: Fredrik Medley <fredrik.medley@gmail.com>\n---\n Documentation/config.txt |  6 ++++++\n fetch-pack.c             |  9 ++++++--\n t/t5516-fetch-push.sh    | 55 ++++++++++++++++++++++++++++++++++++++++++++++++\n upload-pack.c            | 13 +++++++++---\n 4 files changed, 78 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 2e5ceaf..76cd713 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2538,6 +2538,12 @@ uploadpack.allowtipsha1inwant::\n \tof a hidden ref (by default, such a request is rejected).\n \tsee also `uploadpack.hideRefs`.\n \n+uploadpack.allowreachablesha1inwant::\n+\tAllow `upload-pack` to accept a fetch request that asks for an\n+\tobject that is reachable from any ref tip. However, note that\n+\tcalculating\tobject reachability is computationally expensive.\n+\tDefaults to `false`.\n+\n uploadpack.keepAlive::\n \tWhen `upload-pack` has started `pack-objects`, there may be a\n \tquiet period while `pack-objects` prepares the pack. Normally\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 48526aa..fb01b6c 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -43,7 +43,7 @@ static int marked;\n #define MAX_IN_VAIN 256\n \n static struct prio_queue rev_list = { compare_commits_by_commit_date };\n-static int non_common_revs, multi_ack, use_sideband, allow_tip_sha1_in_want;\n+static int non_common_revs, multi_ack, use_sideband, allow_tip_sha1_in_want, allow_reachable_sha1_in_want;\n \n static void rev_list_push(struct commit *commit, int mark)\n {\n@@ -542,7 +542,7 @@ static void filter_refs(struct fetch_pack_args *args,\n \t}\n \n \t/* Append unmatched requests to the list */\n-\tif (allow_tip_sha1_in_want) {\n+\tif (allow_tip_sha1_in_want || allow_reachable_sha1_in_want) {\n \t\tfor (i = 0; i < nr_sought; i++) {\n \t\t\tunsigned char sha1[20];\n \n@@ -823,6 +823,11 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,\n \t\t\tfprintf(stderr, \"Server supports allow-tip-sha1-in-want\\n\");\n \t\tallow_tip_sha1_in_want = 1;\n \t}\n+\tif (server_supports(\"allow-reachable-sha1-in-want\")) {\n+\t\tif (args->verbose)\n+\t\t\tfprintf(stderr, \"Server supports allow-reachable-sha1-in-want\\n\");\n+\t\tallow_reachable_sha1_in_want = 1;\n+\t}\n \tif (!server_supports(\"thin-pack\"))\n \t\targs->use_thin_pack = 0;\n \tif (!server_supports(\"no-progress\"))\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 8a5f236..01fabfb 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1120,6 +1120,61 @@ test_expect_success 'fetch exact SHA1' '\n \t)\n '\n \n+for configallowtipsha1inwant in true false\n+do\n+\ttest_expect_success \"shallow fetch reachable SHA1 (but not a ref), allowtipsha1inwant=$configallowtipsha1inwant\" '\n+\t\tmk_empty testrepo &&\n+\t\t(\n+\t\t\tcd testrepo &&\n+\t\t\tgit config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&\n+\t\t\tgit commit --allow-empty -m foo &&\n+\t\t\tgit commit --allow-empty -m bar\n+\t\t) &&\n+\t\tSHA1=`git --git-dir=testrepo/.git rev-parse HEAD^` &&\n+\t\tmk_empty shallow &&\n+\t\t(\n+\t\t\tcd shallow &&\n+\t\t\ttest_must_fail git fetch --depth=1 ../testrepo/.git $SHA1 &&\n+\t\t\tgit --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&\n+\t\t\tgit fetch --depth=1 ../testrepo/.git $SHA1 &&\n+\t\t\tgit cat-file commit $SHA1 >/dev/null\n+\t\t)\n+\t'\n+\n+\ttest_expect_success \"deny fetch unreachable SHA1, allowtipsha1inwant=$configallowtipsha1inwant\" '\n+\t\tmk_empty testrepo &&\n+\t\t(\n+\t\t\tcd testrepo &&\n+\t\t\tgit config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&\n+\t\t\tgit commit --allow-empty -m foo &&\n+\t\t\tgit commit --allow-empty -m bar &&\n+\t\t\tgit commit --allow-empty -m xyz\n+\t\t)\n+\t\tSHA1_1=`git --git-dir=testrepo/.git rev-parse HEAD^^` &&\n+\t\tSHA1_2=`git --git-dir=testrepo/.git rev-parse HEAD^` &&\n+\t\tSHA1_3=`git --git-dir=testrepo/.git rev-parse HEAD` &&\n+\t\t(\n+\t\t\tcd testrepo &&\n+\t\t\tgit reset --hard $SHA1_2 &&\n+\t\t\tgit cat-file commit $SHA1_3 >/dev/null &&\n+\t\t\tgit cat-file commit $SHA1_3 >/dev/null\n+\t\t) &&\n+\t\tmk_empty shallow &&\n+\t\t(\n+\t\t\tcd shallow &&\n+\t\t\ttest_must_fail git fetch ../testrepo/.git $SHA1_3 &&\n+\t\t\ttest_must_fail git fetch ../testrepo/.git $SHA1_1 &&\n+\t\t\tgit --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&\n+\t\t\tgit fetch ../testrepo/.git $SHA1_1 &&\n+\t\t\tgit cat-file commit $SHA1_1 >/dev/null &&\n+\t\t\ttest_must_fail git cat-file commit $SHA1_2 >/dev/null &&\n+\t\t\tgit fetch ../testrepo/.git $SHA1_2 &&\n+\t\t\tgit cat-file commit $SHA1_2 >/dev/null &&\n+\t\t\ttest_must_fail git fetch ../testrepo/.git $SHA1_3\n+\t\t)\n+\t'\n+done\n+\n test_expect_success 'fetch follows tags by default' '\n \tmk_test testrepo heads/master &&\n \trm -fr src dst &&\ndiff --git a/upload-pack.c b/upload-pack.c\nindex aa84576..2e704d6 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -36,6 +36,7 @@ static int no_done;\n static int use_thin_pack, use_ofs_delta, use_include_tag;\n static int no_progress, daemon_mode;\n static int allow_tip_sha1_in_want;\n+static int allow_reachable_sha1_in_want;\n static int shallow_nr;\n static struct object_array have_obj;\n static struct object_array want_obj;\n@@ -443,7 +444,7 @@ static int get_common_commits(void)\n static int is_our_ref(struct object *o)\n {\n \treturn o->flags &\n-\t\t((allow_tip_sha1_in_want ? HIDDEN_REF : 0) | OUR_REF);\n+\t\t(((allow_tip_sha1_in_want || allow_reachable_sha1_in_want) ? HIDDEN_REF : 0) | OUR_REF);\n }\n \n static void check_non_tip(void)\n@@ -456,8 +457,11 @@ static void check_non_tip(void)\n \tchar namebuf[42]; /* ^ + SHA-1 + LF */\n \tint i;\n \n-\t/* In the normal in-process case non-tip request can never happen */\n-\tif (!stateless_rpc)\n+\t/*\n+\t * In the normal in-process case without\n+\t * uploadpack.allowreachablesha1inwant, non-tip requests can never happen.\n+\t */\n+\tif (!stateless_rpc && !allow_reachable_sha1_in_want)\n \t\tgoto error;\n \n \tcmd.argv = argv;\n@@ -728,6 +732,7 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo\n \t\t\t     sha1_to_hex(sha1), refname_nons,\n \t\t\t     0, capabilities,\n \t\t\t     allow_tip_sha1_in_want ? \" allow-tip-sha1-in-want\" : \"\",\n+\t\t\t     allow_reachable_sha1_in_want ? \" allow-reachable-sha1-in-want\" : \"\",\n \t\t\t     stateless_rpc ? \" no-done\" : \"\",\n \t\t\t     symref_info.buf,\n \t\t\t     git_user_agent_sanitized());\n@@ -789,6 +794,8 @@ static int upload_pack_config(const char *var, const char *value, void *unused)\n {\n \tif (!strcmp(\"uploadpack.allowtipsha1inwant\", var))\n \t\tallow_tip_sha1_in_want = git_config_bool(var, value);\n+\telse if (!strcmp(\"uploadpack.allowreachablesha1inwant\", var))\n+\t\tallow_reachable_sha1_in_want = git_config_bool(var, value);\n \telse if (!strcmp(\"uploadpack.keepalive\", var)) {\n \t\tkeepalive = git_config_int(var, value);\n \t\tif (!keepalive)\n-- \n1.9.1\n"},{"id":"260474","messageId":"xmqqy4l5v9tm.fsf@gitster.dls.corp.google.com","threadId":"39224","inReplyTo":"1430604075-5951-1-git-send-email-fredrik.medley@gmail.com","subject":"Re: [PATCH] upload-pack: Optionally allow fetching reachable sha1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-03T17:40:21Z","receivedAt":"2015-05-03T17:40:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Fredrik Medley <fredrik.medley@gmail.com> writes:\n\n> For you who don't remember the email discussion, look through the references.\n\nPlease don't do this.  Always describe the background yourself in\nthe log message so that \"git log\" can be read offline.  \"describe\nyourself\" can be done by summarizing earlier discussion, borrowing\nothers' words, of course.  And it is a very good idea to give\nreferences like you did after your summary to optionally allow\npeople to verify your summary is correct.\n\nI do not quite understand the early part of the following:\n\n> Since before, it is allowed to ask for a sha1, but most git servers refuse\n> the request[1].\n\nThat makes it sound like \"then talk to the server operator to\nreconfigure and your problem is solved\", no?  I do not think the\nexchange [2] and [3] are about that---your wish will not be granted,\neven if we extended the system to allow non-tip to be fetched, if\nthe server is configured not use that feature.\n\n> This patch is based on a suggested location for\n> implementation[2] which has been updated according to ideas in the reply[3].\n\nI quite am not sure what the suggested \"location\" for implemenation\nis.\n\n> [1] http://thread.gmane.org/gmane.comp.version-control.git/257809\n> [2] http://thread.gmane.org/gmane.comp.version-control.git/258002\n> [3] http://thread.gmane.org/gmane.comp.version-control.git/258080\n>\n> With uploadpack.allowreachablesha1inwant configuration option set,\n\n\"option set\" on the server side, I presume?\n\n> \"git fetch\" can make a request with a \"want\" line that names an object\n> that has not been advertised (likely to have been obtained out of band or\n> from a submodule pointer). Only objects reachable from the branch tips,\n> hidden by transfer.hiderefs or not, will be processed.\n\nI am not sure if I am reading the last sentence correctly.  If there\nare two branches, 'master' (exposed) and 'occult' (hidden),\n\n    ---o---o---2---1---x master\n        \\\n         o---2---1---y occult\n\nyou can ask for the 40-hex object name of occult~2, but you cannot\nask for the 40-hex object name of master~2?  Why such a restriction?\n\n ... wait for an answer that justfies the restriction ...\n\nDoes that justification applies for allowing occult~4 but not master~4?\n\n> @@ -456,8 +457,11 @@ static void check_non_tip(void)\n>  \tchar namebuf[42]; /* ^ + SHA-1 + LF */\n>  \tint i;\n>  \n> -\t/* In the normal in-process case non-tip request can never happen */\n> -\tif (!stateless_rpc)\n> +\t/*\n> +\t * In the normal in-process case without\n> +\t * uploadpack.allowreachablesha1inwant, non-tip requests can never happen.\n> +\t */\n\nThat's quite an overlong line; if you switched it to multi-line,\nthen fold the line once more, perhaps before \"non-tip\"?\n\n> @@ -728,6 +732,7 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo\n>  \t\t\t     sha1_to_hex(sha1), refname_nons,\n>  \t\t\t     0, capabilities,\n>  \t\t\t     allow_tip_sha1_in_want ? \" allow-tip-sha1-in-want\" : \"\",\n> +\t\t\t     allow_reachable_sha1_in_want ? \" allow-reachable-sha1-in-want\" : \"\",\n\nWhich printf format specifier does this new element correspond to?\n"},{"id":"260488","messageId":"CABA5-znFVPunBxET-42jDTC00gH8VMkG280Ptyr8FUU6vfuiCA@mail.gmail.com","threadId":"39224","inReplyTo":"xmqqy4l5v9tm.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] upload-pack: Optionally allow fetching reachable sha1","fromName":"Fredrik Medley","fromEmail":"fredrik.medley@gmail.com","sentAt":"2015-05-03T20:13:31Z","receivedAt":"2015-05-03T20:13:31Z","isPatch":true,"sender":{"key":"fredrik.medley@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5401799?v=4"},"body":"2015-05-03 19:40 GMT+02:00 Junio C Hamano <gitster@pobox.com>:\n> Fredrik Medley <fredrik.medley@gmail.com> writes:\n>\n>> For you who don't remember the email discussion, look through the references.\n>\n> Please don't do this.  Always describe the background yourself in\n> the log message so that \"git log\" can be read offline.  \"describe\n> yourself\" can be done by summarizing earlier discussion, borrowing\n> others' words, of course.  And it is a very good idea to give\n> references like you did after your summary to optionally allow\n> people to verify your summary is correct.\n\nOkay, I understand. My intention was to recapture the old thread, but\nto keep the part under the references for the commit message. When\nI get this answer, I do see that this is impossible to understand and I\nshould probably have added a cover-letter instead. (This is my first\ntime every I've supplied patches to such an open-source project.)\n\nIs the text under the reference enough describing or should there\nbe added some more background text?\n\nUnless someone asks for more answers about my attempt to recap\nthe old mail thread, I skip commenting on this part.\n\n>\n> ...\n>\n>> With uploadpack.allowreachablesha1inwant configuration option set,\n>\n> \"option set\" on the server side, I presume?\nYes, can fix that.\n\"...configuration option set on the server side, \"git fetch\" can...\"\n\n>\n>> \"git fetch\" can make a request with a \"want\" line that names an object\n>> that has not been advertised (likely to have been obtained out of band or\n>> from a submodule pointer). Only objects reachable from the branch tips,\n>> hidden by transfer.hiderefs or not, will be processed.\n>\n> I am not sure if I am reading the last sentence correctly.  If there\n> are two branches, 'master' (exposed) and 'occult' (hidden),\n>\n>     ---o---o---2---1---x master\n>         \\\n>          o---2---1---y occult\n>\n> you can ask for the 40-hex object name of occult~2, but you cannot\n> ask for the 40-hex object name of master~2?  Why such a restriction?\n>\n>  ... wait for an answer that justfies the restriction ...\n>\n> Does that justification applies for allowing occult~4 but not master~4?\nI thought I wrote that both visible and hidden branches will be counted as\nbranch tips. Obviously need to reformulate. Maybe:\n\"Only objects reachable from the branch tips, i.e. the union of advertised\nbranches and branches hidden by transfer.hiderefs, will be processed.\"\n\n>\n>> @@ -456,8 +457,11 @@ static void check_non_tip(void)\n>>       char namebuf[42]; /* ^ + SHA-1 + LF */\n>>       int i;\n>>\n>> -     /* In the normal in-process case non-tip request can never happen */\n>> -     if (!stateless_rpc)\n>> +     /*\n>> +      * In the normal in-process case without\n>> +      * uploadpack.allowreachablesha1inwant, non-tip requests can never happen.\n>> +      */\n>\n> That's quite an overlong line; if you switched it to multi-line,\n> then fold the line once more, perhaps before \"non-tip\"?\n\nYes, can change that. This was to avoid more than 80 characters per line.\nShould I split to three lines then?\n\n>\n>> @@ -728,6 +732,7 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo\n>>                            sha1_to_hex(sha1), refname_nons,\n>>                            0, capabilities,\n>>                            allow_tip_sha1_in_want ? \" allow-tip-sha1-in-want\" : \"\",\n>> +                          allow_reachable_sha1_in_want ? \" allow-reachable-sha1-in-want\" : \"\",\n>\n> Which printf format specifier does this new element correspond to?\n\nOuch! How could I miss that?! Fixing.\n"},{"id":"260494","messageId":"CAPig+cShtk7Be_miSfgzXTPdWxfYmFjJ+kEg5xr+2yLyEhfLvQ@mail.gmail.com","threadId":"39224","inReplyTo":"CABA5-znFVPunBxET-42jDTC00gH8VMkG280Ptyr8FUU6vfuiCA@mail.gmail.com","subject":"Re: [PATCH] upload-pack: Optionally allow fetching reachable sha1","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-05-03T21:35:45Z","receivedAt":"2015-05-03T21:35:45Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, May 3, 2015 at 4:13 PM, Fredrik Medley <fredrik.medley@gmail.com> wrote:\n> 2015-05-03 19:40 GMT+02:00 Junio C Hamano <gitster@pobox.com>:\n>> Fredrik Medley <fredrik.medley@gmail.com> writes:\n>>> For you who don't remember the email discussion, look through the references.\n>>\n>> Please don't do this.  Always describe the background yourself in\n>> the log message so that \"git log\" can be read offline.  \"describe\n>> yourself\" can be done by summarizing earlier discussion, borrowing\n>> others' words, of course.  And it is a very good idea to give\n>> references like you did after your summary to optionally allow\n>> people to verify your summary is correct.\n>\n> Okay, I understand. My intention was to recapture the old thread, but\n> to keep the part under the references for the commit message. When\n> I get this answer, I do see that this is impossible to understand and I\n> should probably have added a cover-letter instead. (This is my first\n> time every I've supplied patches to such an open-source project.)\n\nWelcome to open-source and to git.\n\n> Is the text under the reference enough describing or should there\n> be added some more background text?\n> Unless someone asks for more answers about my attempt to recap\n> the old mail thread, I skip commenting on this part.\n\nWhat Junio meant was that the commit message should be sufficiently\nself-contained such that someone reading it should be able to\nunderstand the problem you're solving without being familiar with past\ndiscussions (and without having to chase down links), and why the\nsolution you've chosen is desirable.\n\nIn this case, the paragraph following the references in your patch,\nwhich you had intended as the full commit message, may not be\nsufficient. You might come closer to a good commit message by fleshing\nout the paragraph preceding the references, and following that by the\nparagraph after the references. By \"fleshing out\", I mean providing\nsalient information from the cited discussions at the points where you\nreferenced them via [n] notation.\n\nYour commit message should explain the problem in the current\nimplementation (possibly justifying why you consider it a problem),\nand an explanation of the solution. You will want to keep the\nreferences (but move them to the bottom of the commit message), and\ncite them when appropriate in your explanation.\n\nFor a single patch like this, you don't really need a cover letter.\nJust make sure the commit message provides sufficient explanation and\nis self-contained. If you want to provide extra commentary not\nintended for the commit message, place it below the \"---\" line after\nyour sign-off and before the diffstat.\n"},{"id":"260621","messageId":"1430860888-6146-1-git-send-email-fredrik.medley@gmail.com","threadId":"39224","inReplyTo":"1430604075-5951-1-git-send-email-fredrik.medley@gmail.com","subject":"[PATCH v2] upload-pack: Optionally allow fetching reachable sha1","fromName":"Fredrik Medley","fromEmail":"fredrik.medley@gmail.com","sentAt":"2015-05-05T21:21:28Z","receivedAt":"2015-05-05T21:21:28Z","isPatch":true,"sender":{"key":"fredrik.medley@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5401799?v=4"},"body":"With uploadpack.allowreachablesha1inwant configuration option set on the\nserver side, \"git fetch\" can make a request with a \"want\" line that names\nan object that has not been advertised (likely to have been obtained out\nof band or from a submodule pointer). Only objects reachable from the\nbranch tips, i.e. the union of advertised branches and branches hidden by\ntransfer.hiderefs, will be processed. Note that there is an associated\ncost of having to walk back the hstory to check the reachability.\n\nThis feature can be used when obtaining the content of a certain commit,\nfor which the sha1 is known, without the need of cloning the whole\nrepository, especially if a shallow fetch is used. Useful cases are e.g.\nrepositories containing large files in the history, fetching only the\nneeded data for a submodule checkout, when sharing a sha1 without telling\nwhich exact branch it belongs to and in Gerrit, if you think in terms of\ncommits instead of change numbers. (The Gerrit case has already been\nsolved through allow-tip-sha1-in-want as every change has a ref.)\n\nSigned-off-by: Fredrik Medley <fredrik.medley@gmail.com>\n---\n Documentation/config.txt |  6 ++++++\n fetch-pack.c             |  9 ++++++--\n t/t5516-fetch-push.sh    | 55 ++++++++++++++++++++++++++++++++++++++++++++++++\n upload-pack.c            | 19 ++++++++++++-----\n 4 files changed, 82 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 2e5ceaf..76cd713 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2538,6 +2538,12 @@ uploadpack.allowtipsha1inwant::\n \tof a hidden ref (by default, such a request is rejected).\n \tsee also `uploadpack.hideRefs`.\n \n+uploadpack.allowreachablesha1inwant::\n+\tAllow `upload-pack` to accept a fetch request that asks for an\n+\tobject that is reachable from any ref tip. However, note that\n+\tcalculating\tobject reachability is computationally expensive.\n+\tDefaults to `false`.\n+\n uploadpack.keepAlive::\n \tWhen `upload-pack` has started `pack-objects`, there may be a\n \tquiet period while `pack-objects` prepares the pack. Normally\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 48526aa..fb01b6c 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -43,7 +43,7 @@ static int marked;\n #define MAX_IN_VAIN 256\n \n static struct prio_queue rev_list = { compare_commits_by_commit_date };\n-static int non_common_revs, multi_ack, use_sideband, allow_tip_sha1_in_want;\n+static int non_common_revs, multi_ack, use_sideband, allow_tip_sha1_in_want, allow_reachable_sha1_in_want;\n \n static void rev_list_push(struct commit *commit, int mark)\n {\n@@ -542,7 +542,7 @@ static void filter_refs(struct fetch_pack_args *args,\n \t}\n \n \t/* Append unmatched requests to the list */\n-\tif (allow_tip_sha1_in_want) {\n+\tif (allow_tip_sha1_in_want || allow_reachable_sha1_in_want) {\n \t\tfor (i = 0; i < nr_sought; i++) {\n \t\t\tunsigned char sha1[20];\n \n@@ -823,6 +823,11 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,\n \t\t\tfprintf(stderr, \"Server supports allow-tip-sha1-in-want\\n\");\n \t\tallow_tip_sha1_in_want = 1;\n \t}\n+\tif (server_supports(\"allow-reachable-sha1-in-want\")) {\n+\t\tif (args->verbose)\n+\t\t\tfprintf(stderr, \"Server supports allow-reachable-sha1-in-want\\n\");\n+\t\tallow_reachable_sha1_in_want = 1;\n+\t}\n \tif (!server_supports(\"thin-pack\"))\n \t\targs->use_thin_pack = 0;\n \tif (!server_supports(\"no-progress\"))\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 8a5f236..01fabfb 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1120,6 +1120,61 @@ test_expect_success 'fetch exact SHA1' '\n \t)\n '\n \n+for configallowtipsha1inwant in true false\n+do\n+\ttest_expect_success \"shallow fetch reachable SHA1 (but not a ref), allowtipsha1inwant=$configallowtipsha1inwant\" '\n+\t\tmk_empty testrepo &&\n+\t\t(\n+\t\t\tcd testrepo &&\n+\t\t\tgit config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&\n+\t\t\tgit commit --allow-empty -m foo &&\n+\t\t\tgit commit --allow-empty -m bar\n+\t\t) &&\n+\t\tSHA1=`git --git-dir=testrepo/.git rev-parse HEAD^` &&\n+\t\tmk_empty shallow &&\n+\t\t(\n+\t\t\tcd shallow &&\n+\t\t\ttest_must_fail git fetch --depth=1 ../testrepo/.git $SHA1 &&\n+\t\t\tgit --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&\n+\t\t\tgit fetch --depth=1 ../testrepo/.git $SHA1 &&\n+\t\t\tgit cat-file commit $SHA1 >/dev/null\n+\t\t)\n+\t'\n+\n+\ttest_expect_success \"deny fetch unreachable SHA1, allowtipsha1inwant=$configallowtipsha1inwant\" '\n+\t\tmk_empty testrepo &&\n+\t\t(\n+\t\t\tcd testrepo &&\n+\t\t\tgit config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&\n+\t\t\tgit commit --allow-empty -m foo &&\n+\t\t\tgit commit --allow-empty -m bar &&\n+\t\t\tgit commit --allow-empty -m xyz\n+\t\t)\n+\t\tSHA1_1=`git --git-dir=testrepo/.git rev-parse HEAD^^` &&\n+\t\tSHA1_2=`git --git-dir=testrepo/.git rev-parse HEAD^` &&\n+\t\tSHA1_3=`git --git-dir=testrepo/.git rev-parse HEAD` &&\n+\t\t(\n+\t\t\tcd testrepo &&\n+\t\t\tgit reset --hard $SHA1_2 &&\n+\t\t\tgit cat-file commit $SHA1_3 >/dev/null &&\n+\t\t\tgit cat-file commit $SHA1_3 >/dev/null\n+\t\t) &&\n+\t\tmk_empty shallow &&\n+\t\t(\n+\t\t\tcd shallow &&\n+\t\t\ttest_must_fail git fetch ../testrepo/.git $SHA1_3 &&\n+\t\t\ttest_must_fail git fetch ../testrepo/.git $SHA1_1 &&\n+\t\t\tgit --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&\n+\t\t\tgit fetch ../testrepo/.git $SHA1_1 &&\n+\t\t\tgit cat-file commit $SHA1_1 >/dev/null &&\n+\t\t\ttest_must_fail git cat-file commit $SHA1_2 >/dev/null &&\n+\t\t\tgit fetch ../testrepo/.git $SHA1_2 &&\n+\t\t\tgit cat-file commit $SHA1_2 >/dev/null &&\n+\t\t\ttest_must_fail git fetch ../testrepo/.git $SHA1_3\n+\t\t)\n+\t'\n+done\n+\n test_expect_success 'fetch follows tags by default' '\n \tmk_test testrepo heads/master &&\n \trm -fr src dst &&\ndiff --git a/upload-pack.c b/upload-pack.c\nindex aa84576..cea4c57 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -36,6 +36,7 @@ static int no_done;\n static int use_thin_pack, use_ofs_delta, use_include_tag;\n static int no_progress, daemon_mode;\n static int allow_tip_sha1_in_want;\n+static int allow_reachable_sha1_in_want;\n static int shallow_nr;\n static struct object_array have_obj;\n static struct object_array want_obj;\n@@ -442,8 +443,9 @@ static int get_common_commits(void)\n \n static int is_our_ref(struct object *o)\n {\n-\treturn o->flags &\n-\t\t((allow_tip_sha1_in_want ? HIDDEN_REF : 0) | OUR_REF);\n+\tint allow_hidden_ref =\n+\t\tallow_tip_sha1_in_want || allow_reachable_sha1_in_want;\n+\treturn o->flags & ((allow_hidden_ref ? HIDDEN_REF : 0) | OUR_REF);\n }\n \n static void check_non_tip(void)\n@@ -456,8 +458,12 @@ static void check_non_tip(void)\n \tchar namebuf[42]; /* ^ + SHA-1 + LF */\n \tint i;\n \n-\t/* In the normal in-process case non-tip request can never happen */\n-\tif (!stateless_rpc)\n+\t/*\n+\t * In the normal in-process case without\n+\t * uploadpack.allowreachablesha1inwant,\n+\t * non-tip requests can never happen.\n+\t */\n+\tif (!stateless_rpc && !allow_reachable_sha1_in_want)\n \t\tgoto error;\n \n \tcmd.argv = argv;\n@@ -724,10 +730,11 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo\n \t\tstruct strbuf symref_info = STRBUF_INIT;\n \n \t\tformat_symref_info(&symref_info, cb_data);\n-\t\tpacket_write(1, \"%s %s%c%s%s%s%s agent=%s\\n\",\n+\t\tpacket_write(1, \"%s %s%c%s%s%s%s%s agent=%s\\n\",\n \t\t\t     sha1_to_hex(sha1), refname_nons,\n \t\t\t     0, capabilities,\n \t\t\t     allow_tip_sha1_in_want ? \" allow-tip-sha1-in-want\" : \"\",\n+\t\t\t     allow_reachable_sha1_in_want ? \" allow-reachable-sha1-in-want\" : \"\",\n \t\t\t     stateless_rpc ? \" no-done\" : \"\",\n \t\t\t     symref_info.buf,\n \t\t\t     git_user_agent_sanitized());\n@@ -789,6 +796,8 @@ static int upload_pack_config(const char *var, const char *value, void *unused)\n {\n \tif (!strcmp(\"uploadpack.allowtipsha1inwant\", var))\n \t\tallow_tip_sha1_in_want = git_config_bool(var, value);\n+\telse if (!strcmp(\"uploadpack.allowreachablesha1inwant\", var))\n+\t\tallow_reachable_sha1_in_want = git_config_bool(var, value);\n \telse if (!strcmp(\"uploadpack.keepalive\", var)) {\n \t\tkeepalive = git_config_int(var, value);\n \t\tif (!keepalive)\n-- \n1.9.1\n"},{"id":"260625","messageId":"xmqqmw1id610.fsf@gitster.dls.corp.google.com","threadId":"39224","inReplyTo":"1430860888-6146-1-git-send-email-fredrik.medley@gmail.com","subject":"Re: [PATCH v2] upload-pack: Optionally allow fetching reachable sha1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-05T22:16:27Z","receivedAt":"2015-05-05T22:16:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Fredrik Medley <fredrik.medley@gmail.com> writes:\n\n> With uploadpack.allowreachablesha1inwant configuration option set on the\n> server side, \"git fetch\" can make a request with a \"want\" line that names\n> an object that has not been advertised (likely to have been obtained out\n> of band or from a submodule pointer). Only objects reachable from the\n> branch tips, i.e. the union of advertised branches and branches hidden by\n> transfer.hiderefs, will be processed. Note that there is an associated\n> cost of having to walk back the hstory to check the reachability.\n\ns/hstory/history/\n\n>\n> This feature can be used when obtaining the content of a certain commit,\n> for which the sha1 is known, without the need of cloning the whole\n> repository, especially if a shallow fetch is used. Useful cases are e.g.\n> repositories containing large files in the history, fetching only the\n> needed data for a submodule checkout, when sharing a sha1 without telling\n> which exact branch it belongs to and in Gerrit, if you think in terms of\n> commits instead of change numbers. (The Gerrit case has already been\n> solved through allow-tip-sha1-in-want as every change has a ref.)\n>\n> Signed-off-by: Fredrik Medley <fredrik.medley@gmail.com>\n> ---\n\nMuch easier to read and understand.\n\n>  Documentation/config.txt |  6 ++++++\n>  fetch-pack.c             |  9 ++++++--\n>  t/t5516-fetch-push.sh    | 55 ++++++++++++++++++++++++++++++++++++++++++++++++\n>  upload-pack.c            | 19 ++++++++++++-----\n>  4 files changed, 82 insertions(+), 7 deletions(-)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 2e5ceaf..76cd713 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -2538,6 +2538,12 @@ uploadpack.allowtipsha1inwant::\n>  \tof a hidden ref (by default, such a request is rejected).\n>  \tsee also `uploadpack.hideRefs`.\n>  \n> +uploadpack.allowreachablesha1inwant::\n\nI know that the existing allowtipsha1inwant is spelled that way, and\nit may be better done as a separate clean-up patch (either before or\nafter this step), but the documentation and the first line of the\nlog message would be easier to read with\n\n\tuploadpack.allowReachableSHA1InWant\n\nI'd think.\n\n> +\tAllow `upload-pack` to accept a fetch request that asks for an\n> +\tobject that is reachable from any ref tip. However, note that\n> +\tcalculating\tobject reachability is computationally expensive.\n> +\tDefaults to `false`.\n\ns/<TAB>object/<SPACE>object/\n\n> diff --git a/fetch-pack.c b/fetch-pack.c\n> index 48526aa..fb01b6c 100644\n> --- a/fetch-pack.c\n> +++ b/fetch-pack.c\n> @@ -43,7 +43,7 @@ static int marked;\n>  #define MAX_IN_VAIN 256\n>  \n>  static struct prio_queue rev_list = { compare_commits_by_commit_date };\n> -static int non_common_revs, multi_ack, use_sideband, allow_tip_sha1_in_want;\n> +static int non_common_revs, multi_ack, use_sideband, allow_tip_sha1_in_want, allow_reachable_sha1_in_want;\n\nDo we anticipate need for other variations of \"allowing bare SHA-1\nthat they did not advertise\" in the future?\n\nThat is a trick question.  We didn't anticipate it, and that is why\nthe existing feature squats on a whole integer variable.  And we are\npaying the price of that lack of foresight by having to enhance with\nthis patch.  So the only sensible answer to that question is \"we\nmight need to keep this extensible\".\n\nHow about renaming the existing allow_tip_sha1_in_want to something\nmore generic to cover all such needs, e.g.\n\n    #define ALLOW_TIP          01\n    #define ALLOW_REACHABLE    02\n    static int allow_request_with_bare_object_name;\n\nThen you do not have to write (tip || reachable), and more\nimportantly, you do not have to force the next person to update that\nto (tip || reachable || his_new_kind), I would think.\n\n> @@ -789,6 +796,8 @@ static int upload_pack_config(const char *var, const char *value, void *unused)\n>  {\n>  \tif (!strcmp(\"uploadpack.allowtipsha1inwant\", var))\n>  \t\tallow_tip_sha1_in_want = git_config_bool(var, value);\n> +\telse if (!strcmp(\"uploadpack.allowreachablesha1inwant\", var))\n> +\t\tallow_reachable_sha1_in_want = git_config_bool(var, value);\n\nUsing all lowercase is correct here, even though the \"camelCase in\ndocumentation and log message for humans\" suggestion above still\nstands.\n\nThanks.\n"},{"id":"260627","messageId":"CAPig+cRqDdj0fHdt10x_YhZcEdJ9WN_bAjUH5MjQxKhdVaEKGg@mail.gmail.com","threadId":"39224","inReplyTo":"1430860888-6146-1-git-send-email-fredrik.medley@gmail.com","subject":"Re: [PATCH v2] upload-pack: Optionally allow fetching reachable sha1","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-05-05T22:29:00Z","receivedAt":"2015-05-05T22:29:00Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, May 5, 2015 at 5:21 PM, Fredrik Medley <fredrik.medley@gmail.com> wrote:\n> With uploadpack.allowreachablesha1inwant configuration option set on the\n> server side, \"git fetch\" can make a request with a \"want\" line that names\n> an object that has not been advertised (likely to have been obtained out\n> of band or from a submodule pointer). Only objects reachable from the\n> branch tips, i.e. the union of advertised branches and branches hidden by\n> transfer.hiderefs, will be processed. Note that there is an associated\n> cost of having to walk back the hstory to check the reachability.\n> [...]\n> Signed-off-by: Fredrik Medley <fredrik.medley@gmail.com>\n> ---\n> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\n> index 8a5f236..01fabfb 100755\n> --- a/t/t5516-fetch-push.sh\n> +++ b/t/t5516-fetch-push.sh\n> @@ -1120,6 +1120,61 @@ test_expect_success 'fetch exact SHA1' '\n>         )\n>  '\n>\n> +for configallowtipsha1inwant in true false\n> +do\n> +       test_expect_success \"shallow fetch reachable SHA1 (but not a ref), allowtipsha1inwant=$configallowtipsha1inwant\" '\n> +               mk_empty testrepo &&\n> +               (\n> +                       cd testrepo &&\n> +                       git config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&\n> +                       git commit --allow-empty -m foo &&\n> +                       git commit --allow-empty -m bar\n> +               ) &&\n> +               SHA1=`git --git-dir=testrepo/.git rev-parse HEAD^` &&\n\nUse $(..) here and elsewhere rather than `...`.\n\n    SHA1=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&\n\n> +               mk_empty shallow &&\n> +               (\n> +                       cd shallow &&\n> +                       test_must_fail git fetch --depth=1 ../testrepo/.git $SHA1 &&\n> +                       git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&\n> +                       git fetch --depth=1 ../testrepo/.git $SHA1 &&\n> +                       git cat-file commit $SHA1 >/dev/null\n> +               )\n> +       '\n> +\n> +       test_expect_success \"deny fetch unreachable SHA1, allowtipsha1inwant=$configallowtipsha1inwant\" '\n> +               mk_empty testrepo &&\n> +               (\n> +                       cd testrepo &&\n> +                       git config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&\n> +                       git commit --allow-empty -m foo &&\n> +                       git commit --allow-empty -m bar &&\n> +                       git commit --allow-empty -m xyz\n> +               )\n> +               SHA1_1=`git --git-dir=testrepo/.git rev-parse HEAD^^` &&\n> +               SHA1_2=`git --git-dir=testrepo/.git rev-parse HEAD^` &&\n> +               SHA1_3=`git --git-dir=testrepo/.git rev-parse HEAD` &&\n> +               (\n> +                       cd testrepo &&\n> +                       git reset --hard $SHA1_2 &&\n> +                       git cat-file commit $SHA1_3 >/dev/null &&\n> +                       git cat-file commit $SHA1_3 >/dev/null\n> +               ) &&\n> +               mk_empty shallow &&\n> +               (\n> +                       cd shallow &&\n> +                       test_must_fail git fetch ../testrepo/.git $SHA1_3 &&\n> +                       test_must_fail git fetch ../testrepo/.git $SHA1_1 &&\n> +                       git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&\n> +                       git fetch ../testrepo/.git $SHA1_1 &&\n> +                       git cat-file commit $SHA1_1 >/dev/null &&\n> +                       test_must_fail git cat-file commit $SHA1_2 >/dev/null &&\n> +                       git fetch ../testrepo/.git $SHA1_2 &&\n> +                       git cat-file commit $SHA1_2 >/dev/null &&\n> +                       test_must_fail git fetch ../testrepo/.git $SHA1_3\n> +               )\n> +       '\n> +done\n> +\n>  test_expect_success 'fetch follows tags by default' '\n>         mk_test testrepo heads/master &&\n>         rm -fr src dst &&\n"},{"id":"260688","messageId":"CABA5-zm-PxX-WWtoDnLhtJVZFhO2G1pOTApoz4DtmmUQ61iC6w@mail.gmail.com","threadId":"39224","inReplyTo":"xmqqmw1id610.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] upload-pack: Optionally allow fetching reachable sha1","fromName":"Fredrik Medley","fromEmail":"fredrik.medley@gmail.com","sentAt":"2015-05-06T20:10:11Z","receivedAt":"2015-05-06T20:10:11Z","isPatch":true,"sender":{"key":"fredrik.medley@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5401799?v=4"},"body":"2015-05-06 0:16 GMT+02:00 Junio C Hamano <gitster@pobox.com>:\n> Fredrik Medley <fredrik.medley@gmail.com> writes:\n>>\n>> diff --git a/Documentation/config.txt b/Documentation/config.txt\n>> index 2e5ceaf..76cd713 100644\n>> --- a/Documentation/config.txt\n>> +++ b/Documentation/config.txt\n>> @@ -2538,6 +2538,12 @@ uploadpack.allowtipsha1inwant::\n>>       of a hidden ref (by default, such a request is rejected).\n>>       see also `uploadpack.hideRefs`.\n>>\n>> +uploadpack.allowreachablesha1inwant::\n>\n> I know that the existing allowtipsha1inwant is spelled that way, and\n> it may be better done as a separate clean-up patch (either before or\n> after this step), but the documentation and the first line of the\n> log message would be easier to read with\n>\n>         uploadpack.allowReachableSHA1InWant\n>\n> I'd think.\n>\n\nI would prefer using allowReachableSha1InWant. Please tell\nme if I should use SHA1InWant instead of Sha1InWant.\n(I cannot find anything similar in the repository.)\n\n>\n>> diff --git a/fetch-pack.c b/fetch-pack.c\n>> index 48526aa..fb01b6c 100644\n>> --- a/fetch-pack.c\n>> +++ b/fetch-pack.c\n>> @@ -43,7 +43,7 @@ static int marked;\n>>  #define MAX_IN_VAIN 256\n>>\n>>  static struct prio_queue rev_list = { compare_commits_by_commit_date };\n>> -static int non_common_revs, multi_ack, use_sideband, allow_tip_sha1_in_want;\n>> +static int non_common_revs, multi_ack, use_sideband, allow_tip_sha1_in_want, allow_reachable_sha1_in_want;\n>\n> Do we anticipate need for other variations of \"allowing bare SHA-1\n> that they did not advertise\" in the future?\n>\n> That is a trick question.  We didn't anticipate it, and that is why\n> the existing feature squats on a whole integer variable.  And we are\n> paying the price of that lack of foresight by having to enhance with\n> this patch.  So the only sensible answer to that question is \"we\n> might need to keep this extensible\".\n>\n> How about renaming the existing allow_tip_sha1_in_want to something\n> more generic to cover all such needs, e.g.\n>\n>     #define ALLOW_TIP          01\n>     #define ALLOW_REACHABLE    02\n>     static int allow_request_with_bare_object_name;\n>\n> Then you do not have to write (tip || reachable), and more\n> importantly, you do not have to force the next person to update that\n> to (tip || reachable || his_new_kind), I would think.\n>\n\nI think the reasoning is sensible and I can see a possibility to allow\nnon-reachable SHA1 in the future, even I would very much recommend\nagainst the use of such option for security reasons.\n\nWhat I can understand, the capability protocol will still need both options\nas separate variables. (I forgot to update the technical documentation\nbefore will do that as well.)\n\nThank you all for quick replies.\n"},{"id":"260689","messageId":"xmqqy4l1sblq.fsf@gitster.dls.corp.google.com","threadId":"39224","inReplyTo":"CABA5-zm-PxX-WWtoDnLhtJVZFhO2G1pOTApoz4DtmmUQ61iC6w@mail.gmail.com","subject":"Re: [PATCH v2] upload-pack: Optionally allow fetching reachable sha1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-06T20:19:13Z","receivedAt":"2015-05-06T20:19:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Fredrik Medley <fredrik.medley@gmail.com> writes:\n\n> 2015-05-06 0:16 GMT+02:00 Junio C Hamano <gitster@pobox.com>:\n>> Fredrik Medley <fredrik.medley@gmail.com> writes:\n>>>\n>>> diff --git a/Documentation/config.txt b/Documentation/config.txt\n>>> index 2e5ceaf..76cd713 100644\n>>> --- a/Documentation/config.txt\n>>> +++ b/Documentation/config.txt\n>>> @@ -2538,6 +2538,12 @@ uploadpack.allowtipsha1inwant::\n>>>       of a hidden ref (by default, such a request is rejected).\n>>>       see also `uploadpack.hideRefs`.\n>>>\n>>> +uploadpack.allowreachablesha1inwant::\n>>\n>> I know that the existing allowtipsha1inwant is spelled that way, and\n>> it may be better done as a separate clean-up patch (either before or\n>> after this step), but the documentation and the first line of the\n>> log message would be easier to read with\n>>\n>>         uploadpack.allowReachableSHA1InWant\n>>\n>> I'd think.\n>>\n>\n> I would prefer using allowReachableSha1InWant. Please tell\n> me if I should use SHA1InWant instead of Sha1InWant.\n> (I cannot find anything similar in the repository.)\n\nKeep in mind what was discussed recently:\n\n  http://thread.gmane.org/gmane.comp.version-control.git/265225/focus=265322\n\nI would think SHA1 should be upcased (so should SSL, SMTP, etc.)\neven in the existing ones when we do the \"clean-up\" pass.  Even\nthough this patch is not about cleaning up existing mess, there\nis no point adding more cruft that we need to clean up later ;-)\n"},{"id":"261080","messageId":"1431465265-18486-1-git-send-email-fredrik.medley@gmail.com","threadId":"39224","inReplyTo":"xmqqy4l1sblq.fsf@gitster.dls.corp.google.com","subject":"[PATCH 1/3] config.txt: Clarify allowTipSHA1InWant with camelCase","fromName":"Fredrik Medley","fromEmail":"fredrik.medley@gmail.com","sentAt":"2015-05-12T21:14:23Z","receivedAt":"2015-05-12T21:14:23Z","isPatch":true,"sender":{"key":"fredrik.medley@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5401799?v=4"},"body":"Most of the options in config.txt are camelCase. Improve the readability\nfor allowtipsha1inwant by changing to allowTipSHA1InWant.\n---\n Documentation/config.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 2e5ceaf..2b86fe6 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2530,9 +2530,9 @@ uploadpack.hideRefs::\n \tare under the hierarchies listed on the value of this\n \tvariable is excluded, and is hidden from `git ls-remote`,\n \t`git fetch`, etc.  An attempt to fetch a hidden ref by `git\n-\tfetch` will fail.  See also `uploadpack.allowtipsha1inwant`.\n+\tfetch` will fail.  See also `uploadpack.allowTipSHA1InWant`.\n \n-uploadpack.allowtipsha1inwant::\n+uploadpack.allowTipSHA1InWant::\n \tWhen `uploadpack.hideRefs` is in effect, allow `upload-pack`\n \tto accept a fetch request that asks for an object at the tip\n \tof a hidden ref (by default, such a request is rejected).\n-- \n1.9.1\n"},{"id":"261081","messageId":"1431465265-18486-2-git-send-email-fredrik.medley@gmail.com","threadId":"39224","inReplyTo":"1431465265-18486-1-git-send-email-fredrik.medley@gmail.com","subject":"[PATCH 2/3] upload-pack: Prepare to extend allow-tip-sha1-in-want","fromName":"Fredrik Medley","fromEmail":"fredrik.medley@gmail.com","sentAt":"2015-05-12T21:14:24Z","receivedAt":"2015-05-12T21:14:24Z","isPatch":true,"sender":{"key":"fredrik.medley@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5401799?v=4"},"body":"Rename the allow_tip_sha1_in_want variable to\nallow_request_with_bare_object_name to allow for future extensions, e.g.\nallowing non-tip sha1.\n---\n fetch-pack.c  |  9 ++++++---\n upload-pack.c | 18 +++++++++++-------\n 2 files changed, 17 insertions(+), 10 deletions(-)\n\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 48526aa..77174f9 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -43,7 +43,10 @@ static int marked;\n #define MAX_IN_VAIN 256\n \n static struct prio_queue rev_list = { compare_commits_by_commit_date };\n-static int non_common_revs, multi_ack, use_sideband, allow_tip_sha1_in_want;\n+static int non_common_revs, multi_ack, use_sideband;\n+/* Allow specifying sha1 if it is a ref tip. */\n+#define ALLOW_TIP\t01\n+static int allow_request_with_bare_object_name;\n \n static void rev_list_push(struct commit *commit, int mark)\n {\n@@ -542,7 +545,7 @@ static void filter_refs(struct fetch_pack_args *args,\n \t}\n \n \t/* Append unmatched requests to the list */\n-\tif (allow_tip_sha1_in_want) {\n+\tif (allow_request_with_bare_object_name & ALLOW_TIP) {\n \t\tfor (i = 0; i < nr_sought; i++) {\n \t\t\tunsigned char sha1[20];\n \n@@ -821,7 +824,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,\n \tif (server_supports(\"allow-tip-sha1-in-want\")) {\n \t\tif (args->verbose)\n \t\t\tfprintf(stderr, \"Server supports allow-tip-sha1-in-want\\n\");\n-\t\tallow_tip_sha1_in_want = 1;\n+\t\tallow_request_with_bare_object_name |= ALLOW_TIP;\n \t}\n \tif (!server_supports(\"thin-pack\"))\n \t\targs->use_thin_pack = 0;\ndiff --git a/upload-pack.c b/upload-pack.c\nindex aa84576..708a502 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -35,7 +35,9 @@ static int multi_ack;\n static int no_done;\n static int use_thin_pack, use_ofs_delta, use_include_tag;\n static int no_progress, daemon_mode;\n-static int allow_tip_sha1_in_want;\n+/* Allow specifying sha1 if it is a ref tip. */\n+#define ALLOW_TIP\t01\n+static int allow_request_with_bare_object_name;\n static int shallow_nr;\n static struct object_array have_obj;\n static struct object_array want_obj;\n@@ -442,8 +444,8 @@ static int get_common_commits(void)\n \n static int is_our_ref(struct object *o)\n {\n-\treturn o->flags &\n-\t\t((allow_tip_sha1_in_want ? HIDDEN_REF : 0) | OUR_REF);\n+\tint allow_hidden_ref = (allow_request_with_bare_object_name & ALLOW_TIP);\n+\treturn o->flags & ((allow_hidden_ref ? HIDDEN_REF : 0) | OUR_REF);\n }\n \n static void check_non_tip(void)\n@@ -727,7 +729,8 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo\n \t\tpacket_write(1, \"%s %s%c%s%s%s%s agent=%s\\n\",\n \t\t\t     sha1_to_hex(sha1), refname_nons,\n \t\t\t     0, capabilities,\n-\t\t\t     allow_tip_sha1_in_want ? \" allow-tip-sha1-in-want\" : \"\",\n+\t\t\t     (allow_request_with_bare_object_name & ALLOW_TIP) ?\n+\t\t\t\t     \" allow-tip-sha1-in-want\" : \"\",\n \t\t\t     stateless_rpc ? \" no-done\" : \"\",\n \t\t\t     symref_info.buf,\n \t\t\t     git_user_agent_sanitized());\n@@ -787,9 +790,10 @@ static void upload_pack(void)\n \n static int upload_pack_config(const char *var, const char *value, void *unused)\n {\n-\tif (!strcmp(\"uploadpack.allowtipsha1inwant\", var))\n-\t\tallow_tip_sha1_in_want = git_config_bool(var, value);\n-\telse if (!strcmp(\"uploadpack.keepalive\", var)) {\n+\tif (!strcmp(\"uploadpack.allowtipsha1inwant\", var)) {\n+\t\tif (git_config_bool(var, value))\n+\t\t\tallow_request_with_bare_object_name |= ALLOW_TIP;\n+\t} else if (!strcmp(\"uploadpack.keepalive\", var)) {\n \t\tkeepalive = git_config_int(var, value);\n \t\tif (!keepalive)\n \t\t\tkeepalive = -1;\n-- \n1.9.1\n"},{"id":"261082","messageId":"1431465265-18486-3-git-send-email-fredrik.medley@gmail.com","threadId":"39224","inReplyTo":"1431465265-18486-1-git-send-email-fredrik.medley@gmail.com","subject":"[PATCH 3/3] upload-pack: Optionally allow fetching reachable sha1","fromName":"Fredrik Medley","fromEmail":"fredrik.medley@gmail.com","sentAt":"2015-05-12T21:14:25Z","receivedAt":"2015-05-12T21:14:25Z","isPatch":true,"sender":{"key":"fredrik.medley@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5401799?v=4"},"body":"With uploadpack.allowReachableSHA1InWant configuration option set on the\nserver side, \"git fetch\" can make a request with a \"want\" line that names\nan object that has not been advertised (likely to have been obtained out\nof band or from a submodule pointer). Only objects reachable from the\nbranch tips, i.e. the union of advertised branches and branches hidden by\ntransfer.hideRefs, will be processed. Note that there is an associated\ncost of having to walk back the hstory to check the reachability.\n\nThis feature can be used when obtaining the content of a certain commit,\nfor which the sha1 is known, without the need of cloning the whole\nrepository, especially if a shallow fetch is used. Useful cases are e.g.\nrepositories containing large files in the history, fetching only the\nneeded data for a submodule checkout, when sharing a sha1 without telling\nwhich exact branch it belongs to and in Gerrit, if you think in terms of\ncommits instead of change numbers. (The Gerrit case has already been\nsolved through allowTipSHA1InWant as every Gerrit change has a ref.)\n\nSigned-off-by: Fredrik Medley <fredrik.medley@gmail.com>\n---\n Documentation/config.txt                          |  6 +++\n Documentation/technical/http-protocol.txt         |  3 +-\n Documentation/technical/protocol-capabilities.txt |  7 +++\n fetch-pack.c                                      |  9 +++-\n t/t5516-fetch-push.sh                             | 55 +++++++++++++++++++++++\n upload-pack.c                                     | 20 +++++++--\n 6 files changed, 94 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 2b86fe6..ffc4cf4 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2538,6 +2538,12 @@ uploadpack.allowTipSHA1InWant::\n \tof a hidden ref (by default, such a request is rejected).\n \tsee also `uploadpack.hideRefs`.\n \n+uploadpack.allowReachableSHA1InWant::\n+\tAllow `upload-pack` to accept a fetch request that asks for an\n+\tobject that is reachable from any ref tip. However, note that\n+\tcalculating\tobject reachability is computationally expensive.\n+\tDefaults to `false`.\n+\n uploadpack.keepAlive::\n \tWhen `upload-pack` has started `pack-objects`, there may be a\n \tquiet period while `pack-objects` prepares the pack. Normally\ndiff --git a/Documentation/technical/http-protocol.txt b/Documentation/technical/http-protocol.txt\nindex 229f845..1c561bd 100644\n--- a/Documentation/technical/http-protocol.txt\n+++ b/Documentation/technical/http-protocol.txt\n@@ -319,7 +319,8 @@ Servers SHOULD support all capabilities defined here.\n Clients MUST send at least one \"want\" command in the request body.\n Clients MUST NOT reference an id in a \"want\" command which did not\n appear in the response obtained through ref discovery unless the\n-server advertises capability `allow-tip-sha1-in-want`.\n+server advertises capability `allow-tip-sha1-in-want` or\n+`allow-reachable-sha1-in-want`.\n \n   compute_request   =  want_list\n \t\t       have_list\ndiff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt\nindex 4f8a7bf..265fcab 100644\n--- a/Documentation/technical/protocol-capabilities.txt\n+++ b/Documentation/technical/protocol-capabilities.txt\n@@ -260,6 +260,13 @@ If the upload-pack server advertises this capability, fetch-pack may\n send \"want\" lines with SHA-1s that exist at the server but are not\n advertised by upload-pack.\n \n+allow-reachable-sha1-in-want\n+----------------------\n+\n+If the upload-pack server advertises this capability, fetch-pack may\n+send \"want\" lines with SHA-1s that exist at the server but are not\n+advertised by upload-pack.\n+\n push-cert=<nonce>\n -----------------\n \ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 77174f9..6c63d8e 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -46,6 +46,8 @@ static struct prio_queue rev_list = { compare_commits_by_commit_date };\n static int non_common_revs, multi_ack, use_sideband;\n /* Allow specifying sha1 if it is a ref tip. */\n #define ALLOW_TIP\t01\n+/* Allow request of a sha1 if it is reachable from a ref (possibly hidden ref). */\n+#define ALLOW_REACHABLE\t02\n static int allow_request_with_bare_object_name;\n \n static void rev_list_push(struct commit *commit, int mark)\n@@ -545,7 +547,7 @@ static void filter_refs(struct fetch_pack_args *args,\n \t}\n \n \t/* Append unmatched requests to the list */\n-\tif (allow_request_with_bare_object_name & ALLOW_TIP) {\n+\tif (allow_request_with_bare_object_name & (ALLOW_TIP | ALLOW_REACHABLE)) {\n \t\tfor (i = 0; i < nr_sought; i++) {\n \t\t\tunsigned char sha1[20];\n \n@@ -826,6 +828,11 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,\n \t\t\tfprintf(stderr, \"Server supports allow-tip-sha1-in-want\\n\");\n \t\tallow_request_with_bare_object_name |= ALLOW_TIP;\n \t}\n+\tif (server_supports(\"allow-reachable-sha1-in-want\")) {\n+\t\tif (args->verbose)\n+\t\t\tfprintf(stderr, \"Server supports allow-reachable-sha1-in-want\\n\");\n+\t\tallow_request_with_bare_object_name |= ALLOW_REACHABLE;\n+\t}\n \tif (!server_supports(\"thin-pack\"))\n \t\targs->use_thin_pack = 0;\n \tif (!server_supports(\"no-progress\"))\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 8a5f236..fdcc114 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1120,6 +1120,61 @@ test_expect_success 'fetch exact SHA1' '\n \t)\n '\n \n+for configallowtipsha1inwant in true false\n+do\n+\ttest_expect_success \"shallow fetch reachable SHA1 (but not a ref), allowtipsha1inwant=$configallowtipsha1inwant\" '\n+\t\tmk_empty testrepo &&\n+\t\t(\n+\t\t\tcd testrepo &&\n+\t\t\tgit config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&\n+\t\t\tgit commit --allow-empty -m foo &&\n+\t\t\tgit commit --allow-empty -m bar\n+\t\t) &&\n+\t\tSHA1=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&\n+\t\tmk_empty shallow &&\n+\t\t(\n+\t\t\tcd shallow &&\n+\t\t\ttest_must_fail git fetch --depth=1 ../testrepo/.git $SHA1 &&\n+\t\t\tgit --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&\n+\t\t\tgit fetch --depth=1 ../testrepo/.git $SHA1 &&\n+\t\t\tgit cat-file commit $SHA1 >/dev/null\n+\t\t)\n+\t'\n+\n+\ttest_expect_success \"deny fetch unreachable SHA1, allowtipsha1inwant=$configallowtipsha1inwant\" '\n+\t\tmk_empty testrepo &&\n+\t\t(\n+\t\t\tcd testrepo &&\n+\t\t\tgit config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&\n+\t\t\tgit commit --allow-empty -m foo &&\n+\t\t\tgit commit --allow-empty -m bar &&\n+\t\t\tgit commit --allow-empty -m xyz\n+\t\t)\n+\t\tSHA1_1=$(git --git-dir=testrepo/.git rev-parse HEAD^^) &&\n+\t\tSHA1_2=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&\n+\t\tSHA1_3=$(git --git-dir=testrepo/.git rev-parse HEAD) &&\n+\t\t(\n+\t\t\tcd testrepo &&\n+\t\t\tgit reset --hard $SHA1_2 &&\n+\t\t\tgit cat-file commit $SHA1_3 >/dev/null &&\n+\t\t\tgit cat-file commit $SHA1_3 >/dev/null\n+\t\t) &&\n+\t\tmk_empty shallow &&\n+\t\t(\n+\t\t\tcd shallow &&\n+\t\t\ttest_must_fail git fetch ../testrepo/.git $SHA1_3 &&\n+\t\t\ttest_must_fail git fetch ../testrepo/.git $SHA1_1 &&\n+\t\t\tgit --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&\n+\t\t\tgit fetch ../testrepo/.git $SHA1_1 &&\n+\t\t\tgit cat-file commit $SHA1_1 >/dev/null &&\n+\t\t\ttest_must_fail git cat-file commit $SHA1_2 >/dev/null &&\n+\t\t\tgit fetch ../testrepo/.git $SHA1_2 &&\n+\t\t\tgit cat-file commit $SHA1_2 >/dev/null &&\n+\t\t\ttest_must_fail git fetch ../testrepo/.git $SHA1_3\n+\t\t)\n+\t'\n+done\n+\n test_expect_success 'fetch follows tags by default' '\n \tmk_test testrepo heads/master &&\n \trm -fr src dst &&\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 708a502..bd14a81 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -37,6 +37,8 @@ static int use_thin_pack, use_ofs_delta, use_include_tag;\n static int no_progress, daemon_mode;\n /* Allow specifying sha1 if it is a ref tip. */\n #define ALLOW_TIP\t01\n+/* Allow request of a sha1 if it is reachable from a ref (possibly hidden ref). */\n+#define ALLOW_REACHABLE\t02\n static int allow_request_with_bare_object_name;\n static int shallow_nr;\n static struct object_array have_obj;\n@@ -444,7 +446,8 @@ static int get_common_commits(void)\n \n static int is_our_ref(struct object *o)\n {\n-\tint allow_hidden_ref = (allow_request_with_bare_object_name & ALLOW_TIP);\n+\tint allow_hidden_ref = (allow_request_with_bare_object_name &\n+\t\t(ALLOW_TIP | ALLOW_REACHABLE));\n \treturn o->flags & ((allow_hidden_ref ? HIDDEN_REF : 0) | OUR_REF);\n }\n \n@@ -458,8 +461,12 @@ static void check_non_tip(void)\n \tchar namebuf[42]; /* ^ + SHA-1 + LF */\n \tint i;\n \n-\t/* In the normal in-process case non-tip request can never happen */\n-\tif (!stateless_rpc)\n+\t/*\n+\t * In the normal in-process case without\n+\t * uploadpack.allowReachableSHA1InWant,\n+\t * non-tip requests can never happen.\n+\t */\n+\tif (!stateless_rpc && !(allow_request_with_bare_object_name & ALLOW_REACHABLE))\n \t\tgoto error;\n \n \tcmd.argv = argv;\n@@ -726,11 +733,13 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo\n \t\tstruct strbuf symref_info = STRBUF_INIT;\n \n \t\tformat_symref_info(&symref_info, cb_data);\n-\t\tpacket_write(1, \"%s %s%c%s%s%s%s agent=%s\\n\",\n+\t\tpacket_write(1, \"%s %s%c%s%s%s%s%s agent=%s\\n\",\n \t\t\t     sha1_to_hex(sha1), refname_nons,\n \t\t\t     0, capabilities,\n \t\t\t     (allow_request_with_bare_object_name & ALLOW_TIP) ?\n \t\t\t\t     \" allow-tip-sha1-in-want\" : \"\",\n+\t\t\t     (allow_request_with_bare_object_name & ALLOW_REACHABLE) ?\n+\t\t\t\t     \" allow-reachable-sha1-in-want\" : \"\",\n \t\t\t     stateless_rpc ? \" no-done\" : \"\",\n \t\t\t     symref_info.buf,\n \t\t\t     git_user_agent_sanitized());\n@@ -793,6 +802,9 @@ static int upload_pack_config(const char *var, const char *value, void *unused)\n \tif (!strcmp(\"uploadpack.allowtipsha1inwant\", var)) {\n \t\tif (git_config_bool(var, value))\n \t\t\tallow_request_with_bare_object_name |= ALLOW_TIP;\n+\t} else if (!strcmp(\"uploadpack.allowreachablesha1inwant\", var)) {\n+\t\tif (git_config_bool(var, value))\n+\t\t\tallow_request_with_bare_object_name |= ALLOW_REACHABLE;\n \t} else if (!strcmp(\"uploadpack.keepalive\", var)) {\n \t\tkeepalive = git_config_int(var, value);\n \t\tif (!keepalive)\n-- \n1.9.1\n"},{"id":"261084","messageId":"CAPig+cTc7qE_tD2-DtpnMGrSFthwaGakFnOWAtkq=a5aOQNMbQ@mail.gmail.com","threadId":"39224","inReplyTo":"1431465265-18486-1-git-send-email-fredrik.medley@gmail.com","subject":"Re: [PATCH 1/3] config.txt: Clarify allowTipSHA1InWant with camelCase","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-05-12T21:24:06Z","receivedAt":"2015-05-12T21:24:06Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, May 12, 2015 at 5:14 PM, Fredrik Medley\n<fredrik.medley@gmail.com> wrote:\n> Most of the options in config.txt are camelCase. Improve the readability\n> for allowtipsha1inwant by changing to allowTipSHA1InWant.\n\nMissing sign-off. More below.\n\n> ---\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 2e5ceaf..2b86fe6 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -2530,9 +2530,9 @@ uploadpack.hideRefs::\n>         are under the hierarchies listed on the value of this\n>         variable is excluded, and is hidden from `git ls-remote`,\n>         `git fetch`, etc.  An attempt to fetch a hidden ref by `git\n> -       fetch` will fail.  See also `uploadpack.allowtipsha1inwant`.\n> +       fetch` will fail.  See also `uploadpack.allowTipSHA1InWant`.\n>\n> -uploadpack.allowtipsha1inwant::\n> +uploadpack.allowTipSHA1InWant::\n\nThis was already attempted here[1]; some inconsistencies with acronyms\nand abbreviations pointed out here[2]; and delayed here[3]. Perhaps\nit's now time to revisit the acronyms/abbreviations issue?\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/265225/focus=265225\n[2]: http://thread.gmane.org/gmane.comp.version-control.git/265225/focus=265262\n[3]: http://thread.gmane.org/gmane.comp.version-control.git/265225/focus=265282\n\n>         When `uploadpack.hideRefs` is in effect, allow `upload-pack`\n>         to accept a fetch request that asks for an object at the tip\n>         of a hidden ref (by default, such a request is rejected).\n> --\n> 1.9.1\n"},{"id":"261085","messageId":"xmqqvbfxeb0n.fsf@gitster.dls.corp.google.com","threadId":"39224","inReplyTo":"CAPig+cTc7qE_tD2-DtpnMGrSFthwaGakFnOWAtkq=a5aOQNMbQ@mail.gmail.com","subject":"Re: [PATCH 1/3] config.txt: Clarify allowTipSHA1InWant with camelCase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-12T21:33:44Z","receivedAt":"2015-05-12T21:33:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n>> -uploadpack.allowtipsha1inwant::\n>> +uploadpack.allowTipSHA1InWant::\n>\n> This was already attempted here[1]; some inconsistencies with acronyms\n> and abbreviations pointed out here[2]; and delayed here[3].\n\nThis was entirely my \"fault\"; see $gmane/268493.\n"},{"id":"261086","messageId":"xmqqr3qleax4.fsf@gitster.dls.corp.google.com","threadId":"39224","inReplyTo":"1431465265-18486-1-git-send-email-fredrik.medley@gmail.com","subject":"Re: [PATCH 1/3] config.txt: Clarify allowTipSHA1InWant with camelCase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-12T21:35:51Z","receivedAt":"2015-05-12T21:35:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Fredrik Medley <fredrik.medley@gmail.com> writes:\n\n> Subject: Re: [PATCH 1/3] config.txt: Clarify allowTipSHA1InWant with camelCase\n\ns/: Clarify/: clarify/;\n\n> Most of the options in config.txt are camelCase. Improve the readability\n> for allowtipsha1inwant by changing to allowTipSHA1InWant.\n\nSign-off?\n\n> ---\n>  Documentation/config.txt | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 2e5ceaf..2b86fe6 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -2530,9 +2530,9 @@ uploadpack.hideRefs::\n>  \tare under the hierarchies listed on the value of this\n>  \tvariable is excluded, and is hidden from `git ls-remote`,\n>  \t`git fetch`, etc.  An attempt to fetch a hidden ref by `git\n> -\tfetch` will fail.  See also `uploadpack.allowtipsha1inwant`.\n> +\tfetch` will fail.  See also `uploadpack.allowTipSHA1InWant`.\n>  \n> -uploadpack.allowtipsha1inwant::\n> +uploadpack.allowTipSHA1InWant::\n>  \tWhen `uploadpack.hideRefs` is in effect, allow `upload-pack`\n>  \tto accept a fetch request that asks for an object at the tip\n>  \tof a hidden ref (by default, such a request is rejected).\n"},{"id":"261088","messageId":"CAPig+cQ8Ndf791b0g+UGZcVsP75Nsd9TQBiUgXv9fhuE+9P4-w@mail.gmail.com","threadId":"39224","inReplyTo":"1431465265-18486-2-git-send-email-fredrik.medley@gmail.com","subject":"Re: [PATCH 2/3] upload-pack: Prepare to extend allow-tip-sha1-in-want","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-05-12T21:37:54Z","receivedAt":"2015-05-12T21:37:54Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, May 12, 2015 at 5:14 PM, Fredrik Medley\n<fredrik.medley@gmail.com> wrote:\n> Rename the allow_tip_sha1_in_want variable to\n> allow_request_with_bare_object_name to allow for future extensions, e.g.\n> allowing non-tip sha1.\n\n'allow_request_with_bare_object_name' is quite a mouthful. Does it\nneed to be this long?\n\nRegarding the commit message: Isn't this preparatory step really about\nchanging the variable from a simple boolean to a \"flag\"-style so that\nit can hold bits for multiple options? Perhaps the commit message\nshould mention something about that?\n\nMissing sign-off.\n\nMore below.\n\n> ---\n> diff --git a/fetch-pack.c b/fetch-pack.c\n> index 48526aa..77174f9 100644\n> --- a/fetch-pack.c\n> +++ b/fetch-pack.c\n> @@ -43,7 +43,10 @@ static int marked;\n>  #define MAX_IN_VAIN 256\n>\n>  static struct prio_queue rev_list = { compare_commits_by_commit_date };\n> -static int non_common_revs, multi_ack, use_sideband, allow_tip_sha1_in_want;\n> +static int non_common_revs, multi_ack, use_sideband;\n> +/* Allow specifying sha1 if it is a ref tip. */\n> +#define ALLOW_TIP      01\n> +static int allow_request_with_bare_object_name;\n>\n>  static void rev_list_push(struct commit *commit, int mark)\n>  {\n> @@ -542,7 +545,7 @@ static void filter_refs(struct fetch_pack_args *args,\n>         }\n>\n>         /* Append unmatched requests to the list */\n> -       if (allow_tip_sha1_in_want) {\n> +       if (allow_request_with_bare_object_name & ALLOW_TIP) {\n\nSome compilers are going to warn about this (warning: did you mean\n\"&&\" rather than \"&\"?). Wrap it in an extra set of parentheses to\navoid the warning.\n\n>                 for (i = 0; i < nr_sought; i++) {\n>                         unsigned char sha1[20];\n>\n> @@ -821,7 +824,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,\n>         if (server_supports(\"allow-tip-sha1-in-want\")) {\n>                 if (args->verbose)\n>                         fprintf(stderr, \"Server supports allow-tip-sha1-in-want\\n\");\n> -               allow_tip_sha1_in_want = 1;\n> +               allow_request_with_bare_object_name |= ALLOW_TIP;\n>         }\n>         if (!server_supports(\"thin-pack\"))\n>                 args->use_thin_pack = 0;\n> diff --git a/upload-pack.c b/upload-pack.c\n> index aa84576..708a502 100644\n> --- a/upload-pack.c\n> +++ b/upload-pack.c\n> @@ -35,7 +35,9 @@ static int multi_ack;\n>  static int no_done;\n>  static int use_thin_pack, use_ofs_delta, use_include_tag;\n>  static int no_progress, daemon_mode;\n> -static int allow_tip_sha1_in_want;\n> +/* Allow specifying sha1 if it is a ref tip. */\n> +#define ALLOW_TIP      01\n> +static int allow_request_with_bare_object_name;\n>  static int shallow_nr;\n>  static struct object_array have_obj;\n>  static struct object_array want_obj;\n> @@ -442,8 +444,8 @@ static int get_common_commits(void)\n>\n>  static int is_our_ref(struct object *o)\n>  {\n> -       return o->flags &\n> -               ((allow_tip_sha1_in_want ? HIDDEN_REF : 0) | OUR_REF);\n> +       int allow_hidden_ref = (allow_request_with_bare_object_name & ALLOW_TIP);\n> +       return o->flags & ((allow_hidden_ref ? HIDDEN_REF : 0) | OUR_REF);\n>  }\n>\n>  static void check_non_tip(void)\n> @@ -727,7 +729,8 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo\n>                 packet_write(1, \"%s %s%c%s%s%s%s agent=%s\\n\",\n>                              sha1_to_hex(sha1), refname_nons,\n>                              0, capabilities,\n> -                            allow_tip_sha1_in_want ? \" allow-tip-sha1-in-want\" : \"\",\n> +                            (allow_request_with_bare_object_name & ALLOW_TIP) ?\n> +                                    \" allow-tip-sha1-in-want\" : \"\",\n>                              stateless_rpc ? \" no-done\" : \"\",\n>                              symref_info.buf,\n>                              git_user_agent_sanitized());\n> @@ -787,9 +790,10 @@ static void upload_pack(void)\n>\n>  static int upload_pack_config(const char *var, const char *value, void *unused)\n>  {\n> -       if (!strcmp(\"uploadpack.allowtipsha1inwant\", var))\n> -               allow_tip_sha1_in_want = git_config_bool(var, value);\n> -       else if (!strcmp(\"uploadpack.keepalive\", var)) {\n> +       if (!strcmp(\"uploadpack.allowtipsha1inwant\", var)) {\n> +               if (git_config_bool(var, value))\n> +                       allow_request_with_bare_object_name |= ALLOW_TIP;\n> +       } else if (!strcmp(\"uploadpack.keepalive\", var)) {\n>                 keepalive = git_config_int(var, value);\n>                 if (!keepalive)\n>                         keepalive = -1;\n> --\n> 1.9.1\n"},{"id":"261089","messageId":"xmqqmw19earu.fsf@gitster.dls.corp.google.com","threadId":"39224","inReplyTo":"1431465265-18486-2-git-send-email-fredrik.medley@gmail.com","subject":"Re: [PATCH 2/3] upload-pack: Prepare to extend allow-tip-sha1-in-want","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-12T21:39:01Z","receivedAt":"2015-05-12T21:39:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Fredrik Medley <fredrik.medley@gmail.com> writes:\n\n> Subject: Re: [PATCH 2/3] upload-pack: Prepare to extend allow-tip-sha1-in-want\n\ns/: Prepare/: prepare/;\n\n> Rename the allow_tip_sha1_in_want variable to\n> allow_request_with_bare_object_name to allow for future extensions, e.g.\n> allowing non-tip sha1.\n> ---\n\nSign-off?\n\n>  fetch-pack.c  |  9 ++++++---\n>  upload-pack.c | 18 +++++++++++-------\n>  2 files changed, 17 insertions(+), 10 deletions(-)\n>\n> diff --git a/fetch-pack.c b/fetch-pack.c\n> index 48526aa..77174f9 100644\n> --- a/fetch-pack.c\n> +++ b/fetch-pack.c\n> @@ -43,7 +43,10 @@ static int marked;\n>  #define MAX_IN_VAIN 256\n>  \n>  static struct prio_queue rev_list = { compare_commits_by_commit_date };\n> -static int non_common_revs, multi_ack, use_sideband, allow_tip_sha1_in_want;\n> +static int non_common_revs, multi_ack, use_sideband;\n> +/* Allow specifying sha1 if it is a ref tip. */\n> +#define ALLOW_TIP\t01\n> +static int allow_request_with_bare_object_name;\n\nThis side is OK, as \"request\" is by the end user giving the object\nname from its command line.\n\n> diff --git a/upload-pack.c b/upload-pack.c\n> index aa84576..708a502 100644\n> --- a/upload-pack.c\n> +++ b/upload-pack.c\n> @@ -35,7 +35,9 @@ static int multi_ack;\n>  static int no_done;\n>  static int use_thin_pack, use_ofs_delta, use_include_tag;\n>  static int no_progress, daemon_mode;\n> -static int allow_tip_sha1_in_want;\n> +/* Allow specifying sha1 if it is a ref tip. */\n> +#define ALLOW_TIP\t01\n> +static int allow_request_with_bare_object_name;\n\nThis side is not quite good, as the request coming over wire is\nalways 40-hex bare object name.  We are allowing requests against\nwhat we did not advertise (either the tip of hidden refs, or\nsomewhere deep in the history from some tip that may or may not have\nbeen advertised).\n\nallow-unadvertised-object-request or something, perhaps?\n\n>  static int shallow_nr;\n>  static struct object_array have_obj;\n>  static struct object_array want_obj;\n> @@ -442,8 +444,8 @@ static int get_common_commits(void)\n>  \n>  static int is_our_ref(struct object *o)\n>  {\n> -\treturn o->flags &\n> -\t\t((allow_tip_sha1_in_want ? HIDDEN_REF : 0) | OUR_REF);\n> +\tint allow_hidden_ref = (allow_request_with_bare_object_name & ALLOW_TIP);\n> +\treturn o->flags & ((allow_hidden_ref ? HIDDEN_REF : 0) | OUR_REF);\n>  }\n>  \n>  static void check_non_tip(void)\n> @@ -727,7 +729,8 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo\n>  \t\tpacket_write(1, \"%s %s%c%s%s%s%s agent=%s\\n\",\n>  \t\t\t     sha1_to_hex(sha1), refname_nons,\n>  \t\t\t     0, capabilities,\n> -\t\t\t     allow_tip_sha1_in_want ? \" allow-tip-sha1-in-want\" : \"\",\n> +\t\t\t     (allow_request_with_bare_object_name & ALLOW_TIP) ?\n> +\t\t\t\t     \" allow-tip-sha1-in-want\" : \"\",\n>  \t\t\t     stateless_rpc ? \" no-done\" : \"\",\n>  \t\t\t     symref_info.buf,\n>  \t\t\t     git_user_agent_sanitized());\n> @@ -787,9 +790,10 @@ static void upload_pack(void)\n>  \n>  static int upload_pack_config(const char *var, const char *value, void *unused)\n>  {\n> -\tif (!strcmp(\"uploadpack.allowtipsha1inwant\", var))\n> -\t\tallow_tip_sha1_in_want = git_config_bool(var, value);\n> -\telse if (!strcmp(\"uploadpack.keepalive\", var)) {\n> +\tif (!strcmp(\"uploadpack.allowtipsha1inwant\", var)) {\n> +\t\tif (git_config_bool(var, value))\n> +\t\t\tallow_request_with_bare_object_name |= ALLOW_TIP;\n> +\t} else if (!strcmp(\"uploadpack.keepalive\", var)) {\n>  \t\tkeepalive = git_config_int(var, value);\n>  \t\tif (!keepalive)\n>  \t\t\tkeepalive = -1;\n"},{"id":"261094","messageId":"CAPig+cQe3=Y15+RBZCxry7-78kbNL6H0PfdYs=OV_DMifOWxaA@mail.gmail.com","threadId":"39224","inReplyTo":"1431465265-18486-3-git-send-email-fredrik.medley@gmail.com","subject":"Re: [PATCH 3/3] upload-pack: Optionally allow fetching reachable sha1","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-05-12T22:01:10Z","receivedAt":"2015-05-12T22:01:10Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, May 12, 2015 at 5:14 PM, Fredrik Medley\n<fredrik.medley@gmail.com> wrote:\n> With uploadpack.allowReachableSHA1InWant configuration option set on the\n> server side, \"git fetch\" can make a request with a \"want\" line that names\n> an object that has not been advertised (likely to have been obtained out\n> of band or from a submodule pointer). Only objects reachable from the\n> branch tips, i.e. the union of advertised branches and branches hidden by\n> transfer.hideRefs, will be processed. Note that there is an associated\n> cost of having to walk back the hstory to check the reachability.\n\nMentioned previously[1]: s/hstory/history/\n\n> This feature can be used when obtaining the content of a certain commit,\n> for which the sha1 is known, without the need of cloning the whole\n> repository, especially if a shallow fetch is used. Useful cases are e.g.\n> repositories containing large files in the history, fetching only the\n> needed data for a submodule checkout, when sharing a sha1 without telling\n> which exact branch it belongs to and in Gerrit, if you think in terms of\n> commits instead of change numbers. (The Gerrit case has already been\n> solved through allowTipSHA1InWant as every Gerrit change has a ref.)\n>\n> Signed-off-by: Fredrik Medley <fredrik.medley@gmail.com>\n> ---\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 2b86fe6..ffc4cf4 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -2538,6 +2538,12 @@ uploadpack.allowTipSHA1InWant::\n>         of a hidden ref (by default, such a request is rejected).\n>         see also `uploadpack.hideRefs`.\n>\n> +uploadpack.allowReachableSHA1InWant::\n> +       Allow `upload-pack` to accept a fetch request that asks for an\n> +       object that is reachable from any ref tip. However, note that\n> +       calculating     object reachability is computationally expensive.\n\nMentioned previously[1]: s/\\s+/ /\n\n> +       Defaults to `false`.\n> +\n> diff --git a/fetch-pack.c b/fetch-pack.c\n> index 77174f9..6c63d8e 100644\n> --- a/fetch-pack.c\n> +++ b/fetch-pack.c\n> @@ -545,7 +547,7 @@ static void filter_refs(struct fetch_pack_args *args,\n>         }\n>\n>         /* Append unmatched requests to the list */\n> -       if (allow_request_with_bare_object_name & ALLOW_TIP) {\n> +       if (allow_request_with_bare_object_name & (ALLOW_TIP | ALLOW_REACHABLE)) {\n\nWrap this in extra parentheses to avoid compilation warnings:\n\n    if ((foo & (bar | baz))) {\n\n>                 for (i = 0; i < nr_sought; i++) {\n>                         unsigned char sha1[20];\n>\n\n[1]: http://article.gmane.org/gmane.comp.version-control.git/268428\n"},{"id":"261590","messageId":"CABA5-zkLY9vnZnAffn7=NYOmaoXeK8LJiNBVmyvRmc803mfGVg@mail.gmail.com","threadId":"39224","inReplyTo":"xmqqmw19earu.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/3] upload-pack: Prepare to extend allow-tip-sha1-in-want","fromName":"Fredrik Medley","fromEmail":"fredrik.medley@gmail.com","sentAt":"2015-05-19T20:19:14Z","receivedAt":"2015-05-19T20:19:14Z","isPatch":true,"sender":{"key":"fredrik.medley@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5401799?v=4"},"body":"2015-05-12 23:39 GMT+02:00 Junio C Hamano <gitster@pobox.com>:\n> Fredrik Medley <fredrik.medley@gmail.com> writes:\n>\n>> Subject: Re: [PATCH 2/3] upload-pack: Prepare to extend allow-tip-sha1-in-want\n>>\n>> diff --git a/fetch-pack.c b/fetch-pack.c\n>> index 48526aa..77174f9 100644\n>> --- a/fetch-pack.c\n>> +++ b/fetch-pack.c\n>> @@ -43,7 +43,10 @@ static int marked;\n>>  #define MAX_IN_VAIN 256\n>>\n>>  static struct prio_queue rev_list = { compare_commits_by_commit_date };\n>> -static int non_common_revs, multi_ack, use_sideband, allow_tip_sha1_in_want;\n>> +static int non_common_revs, multi_ack, use_sideband;\n>> +/* Allow specifying sha1 if it is a ref tip. */\n>> +#define ALLOW_TIP    01\n>> +static int allow_request_with_bare_object_name;\n>\n> This side is OK, as \"request\" is by the end user giving the object\n> name from its command line.\n>\n>> diff --git a/upload-pack.c b/upload-pack.c\n>> index aa84576..708a502 100644\n>> --- a/upload-pack.c\n>> +++ b/upload-pack.c\n>> @@ -35,7 +35,9 @@ static int multi_ack;\n>>  static int no_done;\n>>  static int use_thin_pack, use_ofs_delta, use_include_tag;\n>>  static int no_progress, daemon_mode;\n>> -static int allow_tip_sha1_in_want;\n>> +/* Allow specifying sha1 if it is a ref tip. */\n>> +#define ALLOW_TIP    01\n>> +static int allow_request_with_bare_object_name;\n>\n> This side is not quite good, as the request coming over wire is\n> always 40-hex bare object name.  We are allowing requests against\n> what we did not advertise (either the tip of hidden refs, or\n> somewhere deep in the history from some tip that may or may not have\n> been advertised).\n>\n> allow-unadvertised-object-request or something, perhaps?\n>\n\nMy imagination is not any better, so I will stick with\nallow_unadvertised_object_request. I do think that that name\nfits okay in fetch-pack.c as well. The name will also allow for a\npossibly future option of requesting (unadvertised) hidden refs by\nname, but it would of course require larger changes of the protocol.\n"},{"id":"261593","messageId":"1432068269-14895-1-git-send-email-fredrik.medley@gmail.com","threadId":"39224","inReplyTo":"CAPig+cQe3=Y15+RBZCxry7-78kbNL6H0PfdYs=OV_DMifOWxaA@mail.gmail.com","subject":"[PATCH 1/3] config.txt: clarify allowTipSHA1InWant with camelCase","fromName":"Fredrik Medley","fromEmail":"fredrik.medley@gmail.com","sentAt":"2015-05-19T20:44:27Z","receivedAt":"2015-05-19T20:44:27Z","isPatch":true,"sender":{"key":"fredrik.medley@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5401799?v=4"},"body":"Most of the options in config.txt are camelCase. Improve the readability\nfor allowtipsha1inwant by changing to allowTipSHA1InWant.\n\nSigned-off-by: Fredrik Medley <fredrik.medley@gmail.com>\n---\nThis patch is optional. There has been work on fixing the whole\nDocumentation/config.txt which has not been merged yet. When adding\nallowReachableSHA1InWant later, it would be good to already have changed\nto allowTipSHA1InWant.\n\n Documentation/config.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 2e5ceaf..2b86fe6 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2530,9 +2530,9 @@ uploadpack.hideRefs::\n \tare under the hierarchies listed on the value of this\n \tvariable is excluded, and is hidden from `git ls-remote`,\n \t`git fetch`, etc.  An attempt to fetch a hidden ref by `git\n-\tfetch` will fail.  See also `uploadpack.allowtipsha1inwant`.\n+\tfetch` will fail.  See also `uploadpack.allowTipSHA1InWant`.\n \n-uploadpack.allowtipsha1inwant::\n+uploadpack.allowTipSHA1InWant::\n \tWhen `uploadpack.hideRefs` is in effect, allow `upload-pack`\n \tto accept a fetch request that asks for an object at the tip\n \tof a hidden ref (by default, such a request is rejected).\n-- \n1.9.1\n"},{"id":"261594","messageId":"1432068269-14895-2-git-send-email-fredrik.medley@gmail.com","threadId":"39224","inReplyTo":"1432068269-14895-1-git-send-email-fredrik.medley@gmail.com","subject":"[PATCH 2/3] upload-pack: prepare to extend allow-tip-sha1-in-want","fromName":"Fredrik Medley","fromEmail":"fredrik.medley@gmail.com","sentAt":"2015-05-19T20:44:28Z","receivedAt":"2015-05-19T20:44:28Z","isPatch":true,"sender":{"key":"fredrik.medley@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5401799?v=4"},"body":"To allow future extensions, e.g. allowing non-tip sha1, replace the\nboolean allow_tip_sha1_in_want variable with the flag-style\nallow_request_with_bare_object_name variable.\n\nSigned-off-by: Fredrik Medley <fredrik.medley@gmail.com>\n---\n fetch-pack.c  |  9 ++++++---\n upload-pack.c | 18 +++++++++++-------\n 2 files changed, 17 insertions(+), 10 deletions(-)\n\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 48526aa..699f586 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -43,7 +43,10 @@ static int marked;\n #define MAX_IN_VAIN 256\n \n static struct prio_queue rev_list = { compare_commits_by_commit_date };\n-static int non_common_revs, multi_ack, use_sideband, allow_tip_sha1_in_want;\n+static int non_common_revs, multi_ack, use_sideband;\n+/* Allow specifying sha1 if it is a ref tip. */\n+#define ALLOW_TIP_SHA1\t01\n+static int allow_unadvertised_object_request;\n \n static void rev_list_push(struct commit *commit, int mark)\n {\n@@ -542,7 +545,7 @@ static void filter_refs(struct fetch_pack_args *args,\n \t}\n \n \t/* Append unmatched requests to the list */\n-\tif (allow_tip_sha1_in_want) {\n+\tif ((allow_unadvertised_object_request & ALLOW_TIP_SHA1)) {\n \t\tfor (i = 0; i < nr_sought; i++) {\n \t\t\tunsigned char sha1[20];\n \n@@ -821,7 +824,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,\n \tif (server_supports(\"allow-tip-sha1-in-want\")) {\n \t\tif (args->verbose)\n \t\t\tfprintf(stderr, \"Server supports allow-tip-sha1-in-want\\n\");\n-\t\tallow_tip_sha1_in_want = 1;\n+\t\tallow_unadvertised_object_request |= ALLOW_TIP_SHA1;\n \t}\n \tif (!server_supports(\"thin-pack\"))\n \t\targs->use_thin_pack = 0;\ndiff --git a/upload-pack.c b/upload-pack.c\nindex aa84576..d443e58 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -35,7 +35,9 @@ static int multi_ack;\n static int no_done;\n static int use_thin_pack, use_ofs_delta, use_include_tag;\n static int no_progress, daemon_mode;\n-static int allow_tip_sha1_in_want;\n+/* Allow specifying sha1 if it is a ref tip. */\n+#define ALLOW_TIP_SHA1\t01\n+static int allow_unadvertised_object_request;\n static int shallow_nr;\n static struct object_array have_obj;\n static struct object_array want_obj;\n@@ -442,8 +444,8 @@ static int get_common_commits(void)\n \n static int is_our_ref(struct object *o)\n {\n-\treturn o->flags &\n-\t\t((allow_tip_sha1_in_want ? HIDDEN_REF : 0) | OUR_REF);\n+\tint allow_hidden_ref = (allow_unadvertised_object_request & ALLOW_TIP_SHA1);\n+\treturn o->flags & ((allow_hidden_ref ? HIDDEN_REF : 0) | OUR_REF);\n }\n \n static void check_non_tip(void)\n@@ -727,7 +729,8 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo\n \t\tpacket_write(1, \"%s %s%c%s%s%s%s agent=%s\\n\",\n \t\t\t     sha1_to_hex(sha1), refname_nons,\n \t\t\t     0, capabilities,\n-\t\t\t     allow_tip_sha1_in_want ? \" allow-tip-sha1-in-want\" : \"\",\n+\t\t\t     (allow_unadvertised_object_request & ALLOW_TIP_SHA1) ?\n+\t\t\t\t     \" allow-tip-sha1-in-want\" : \"\",\n \t\t\t     stateless_rpc ? \" no-done\" : \"\",\n \t\t\t     symref_info.buf,\n \t\t\t     git_user_agent_sanitized());\n@@ -787,9 +790,10 @@ static void upload_pack(void)\n \n static int upload_pack_config(const char *var, const char *value, void *unused)\n {\n-\tif (!strcmp(\"uploadpack.allowtipsha1inwant\", var))\n-\t\tallow_tip_sha1_in_want = git_config_bool(var, value);\n-\telse if (!strcmp(\"uploadpack.keepalive\", var)) {\n+\tif (!strcmp(\"uploadpack.allowtipsha1inwant\", var)) {\n+\t\tif (git_config_bool(var, value))\n+\t\t\tallow_unadvertised_object_request |= ALLOW_TIP_SHA1;\n+\t} else if (!strcmp(\"uploadpack.keepalive\", var)) {\n \t\tkeepalive = git_config_int(var, value);\n \t\tif (!keepalive)\n \t\t\tkeepalive = -1;\n-- \n1.9.1\n"},{"id":"261595","messageId":"1432068269-14895-3-git-send-email-fredrik.medley@gmail.com","threadId":"39224","inReplyTo":"1432068269-14895-1-git-send-email-fredrik.medley@gmail.com","subject":"[PATCH 3/3] upload-pack: optionally allow fetching reachable sha1","fromName":"Fredrik Medley","fromEmail":"fredrik.medley@gmail.com","sentAt":"2015-05-19T20:44:29Z","receivedAt":"2015-05-19T20:44:29Z","isPatch":true,"sender":{"key":"fredrik.medley@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5401799?v=4"},"body":"With uploadpack.allowReachableSHA1InWant configuration option set on the\nserver side, \"git fetch\" can make a request with a \"want\" line that names\nan object that has not been advertised (likely to have been obtained out\nof band or from a submodule pointer). Only objects reachable from the\nbranch tips, i.e. the union of advertised branches and branches hidden by\ntransfer.hideRefs, will be processed. Note that there is an associated\ncost of having to walk back the history to check the reachability.\n\nThis feature can be used when obtaining the content of a certain commit,\nfor which the sha1 is known, without the need of cloning the whole\nrepository, especially if a shallow fetch is used. Useful cases are e.g.\nrepositories containing large files in the history, fetching only the\nneeded data for a submodule checkout, when sharing a sha1 without telling\nwhich exact branch it belongs to and in Gerrit, if you think in terms of\ncommits instead of change numbers. (The Gerrit case has already been\nsolved through allowTipSHA1InWant as every Gerrit change has a ref.)\n\nSigned-off-by: Fredrik Medley <fredrik.medley@gmail.com>\n---\n Documentation/config.txt                          |  6 +++\n Documentation/technical/http-protocol.txt         |  3 +-\n Documentation/technical/protocol-capabilities.txt |  7 +++\n fetch-pack.c                                      | 11 ++++-\n t/t5516-fetch-push.sh                             | 55 +++++++++++++++++++++++\n upload-pack.c                                     | 22 ++++++---\n 6 files changed, 96 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 2b86fe6..980520b 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2538,6 +2538,12 @@ uploadpack.allowTipSHA1InWant::\n \tof a hidden ref (by default, such a request is rejected).\n \tsee also `uploadpack.hideRefs`.\n \n+uploadpack.allowReachableSHA1InWant::\n+\tAllow `upload-pack` to accept a fetch request that asks for an\n+\tobject that is reachable from any ref tip. However, note that\n+\tcalculating object reachability is computationally expensive.\n+\tDefaults to `false`.\n+\n uploadpack.keepAlive::\n \tWhen `upload-pack` has started `pack-objects`, there may be a\n \tquiet period while `pack-objects` prepares the pack. Normally\ndiff --git a/Documentation/technical/http-protocol.txt b/Documentation/technical/http-protocol.txt\nindex 229f845..1c561bd 100644\n--- a/Documentation/technical/http-protocol.txt\n+++ b/Documentation/technical/http-protocol.txt\n@@ -319,7 +319,8 @@ Servers SHOULD support all capabilities defined here.\n Clients MUST send at least one \"want\" command in the request body.\n Clients MUST NOT reference an id in a \"want\" command which did not\n appear in the response obtained through ref discovery unless the\n-server advertises capability `allow-tip-sha1-in-want`.\n+server advertises capability `allow-tip-sha1-in-want` or\n+`allow-reachable-sha1-in-want`.\n \n   compute_request   =  want_list\n \t\t       have_list\ndiff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt\nindex 4f8a7bf..265fcab 100644\n--- a/Documentation/technical/protocol-capabilities.txt\n+++ b/Documentation/technical/protocol-capabilities.txt\n@@ -260,6 +260,13 @@ If the upload-pack server advertises this capability, fetch-pack may\n send \"want\" lines with SHA-1s that exist at the server but are not\n advertised by upload-pack.\n \n+allow-reachable-sha1-in-want\n+----------------------\n+\n+If the upload-pack server advertises this capability, fetch-pack may\n+send \"want\" lines with SHA-1s that exist at the server but are not\n+advertised by upload-pack.\n+\n push-cert=<nonce>\n -----------------\n \ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 699f586..875b688 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -45,7 +45,9 @@ static int marked;\n static struct prio_queue rev_list = { compare_commits_by_commit_date };\n static int non_common_revs, multi_ack, use_sideband;\n /* Allow specifying sha1 if it is a ref tip. */\n-#define ALLOW_TIP_SHA1\t01\n+#define ALLOW_TIP_SHA1\t\t01\n+/* Allow request of a sha1 if it is reachable from a ref (possibly hidden ref). */\n+#define ALLOW_REACHABLE_SHA1\t02\n static int allow_unadvertised_object_request;\n \n static void rev_list_push(struct commit *commit, int mark)\n@@ -545,7 +547,7 @@ static void filter_refs(struct fetch_pack_args *args,\n \t}\n \n \t/* Append unmatched requests to the list */\n-\tif ((allow_unadvertised_object_request & ALLOW_TIP_SHA1)) {\n+\tif ((allow_unadvertised_object_request & (ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1))) {\n \t\tfor (i = 0; i < nr_sought; i++) {\n \t\t\tunsigned char sha1[20];\n \n@@ -826,6 +828,11 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,\n \t\t\tfprintf(stderr, \"Server supports allow-tip-sha1-in-want\\n\");\n \t\tallow_unadvertised_object_request |= ALLOW_TIP_SHA1;\n \t}\n+\tif (server_supports(\"allow-reachable-sha1-in-want\")) {\n+\t\tif (args->verbose)\n+\t\t\tfprintf(stderr, \"Server supports allow-reachable-sha1-in-want\\n\");\n+\t\tallow_unadvertised_object_request |= ALLOW_REACHABLE_SHA1;\n+\t}\n \tif (!server_supports(\"thin-pack\"))\n \t\targs->use_thin_pack = 0;\n \tif (!server_supports(\"no-progress\"))\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 8a5f236..fdcc114 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1120,6 +1120,61 @@ test_expect_success 'fetch exact SHA1' '\n \t)\n '\n \n+for configallowtipsha1inwant in true false\n+do\n+\ttest_expect_success \"shallow fetch reachable SHA1 (but not a ref), allowtipsha1inwant=$configallowtipsha1inwant\" '\n+\t\tmk_empty testrepo &&\n+\t\t(\n+\t\t\tcd testrepo &&\n+\t\t\tgit config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&\n+\t\t\tgit commit --allow-empty -m foo &&\n+\t\t\tgit commit --allow-empty -m bar\n+\t\t) &&\n+\t\tSHA1=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&\n+\t\tmk_empty shallow &&\n+\t\t(\n+\t\t\tcd shallow &&\n+\t\t\ttest_must_fail git fetch --depth=1 ../testrepo/.git $SHA1 &&\n+\t\t\tgit --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&\n+\t\t\tgit fetch --depth=1 ../testrepo/.git $SHA1 &&\n+\t\t\tgit cat-file commit $SHA1 >/dev/null\n+\t\t)\n+\t'\n+\n+\ttest_expect_success \"deny fetch unreachable SHA1, allowtipsha1inwant=$configallowtipsha1inwant\" '\n+\t\tmk_empty testrepo &&\n+\t\t(\n+\t\t\tcd testrepo &&\n+\t\t\tgit config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&\n+\t\t\tgit commit --allow-empty -m foo &&\n+\t\t\tgit commit --allow-empty -m bar &&\n+\t\t\tgit commit --allow-empty -m xyz\n+\t\t)\n+\t\tSHA1_1=$(git --git-dir=testrepo/.git rev-parse HEAD^^) &&\n+\t\tSHA1_2=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&\n+\t\tSHA1_3=$(git --git-dir=testrepo/.git rev-parse HEAD) &&\n+\t\t(\n+\t\t\tcd testrepo &&\n+\t\t\tgit reset --hard $SHA1_2 &&\n+\t\t\tgit cat-file commit $SHA1_3 >/dev/null &&\n+\t\t\tgit cat-file commit $SHA1_3 >/dev/null\n+\t\t) &&\n+\t\tmk_empty shallow &&\n+\t\t(\n+\t\t\tcd shallow &&\n+\t\t\ttest_must_fail git fetch ../testrepo/.git $SHA1_3 &&\n+\t\t\ttest_must_fail git fetch ../testrepo/.git $SHA1_1 &&\n+\t\t\tgit --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&\n+\t\t\tgit fetch ../testrepo/.git $SHA1_1 &&\n+\t\t\tgit cat-file commit $SHA1_1 >/dev/null &&\n+\t\t\ttest_must_fail git cat-file commit $SHA1_2 >/dev/null &&\n+\t\t\tgit fetch ../testrepo/.git $SHA1_2 &&\n+\t\t\tgit cat-file commit $SHA1_2 >/dev/null &&\n+\t\t\ttest_must_fail git fetch ../testrepo/.git $SHA1_3\n+\t\t)\n+\t'\n+done\n+\n test_expect_success 'fetch follows tags by default' '\n \tmk_test testrepo heads/master &&\n \trm -fr src dst &&\ndiff --git a/upload-pack.c b/upload-pack.c\nindex d443e58..f99eb99 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -36,7 +36,9 @@ static int no_done;\n static int use_thin_pack, use_ofs_delta, use_include_tag;\n static int no_progress, daemon_mode;\n /* Allow specifying sha1 if it is a ref tip. */\n-#define ALLOW_TIP_SHA1\t01\n+#define ALLOW_TIP_SHA1\t\t01\n+/* Allow request of a sha1 if it is reachable from a ref (possibly hidden ref). */\n+#define ALLOW_REACHABLE_SHA1\t02\n static int allow_unadvertised_object_request;\n static int shallow_nr;\n static struct object_array have_obj;\n@@ -444,7 +446,8 @@ static int get_common_commits(void)\n \n static int is_our_ref(struct object *o)\n {\n-\tint allow_hidden_ref = (allow_unadvertised_object_request & ALLOW_TIP_SHA1);\n+\tint allow_hidden_ref = (allow_unadvertised_object_request &\n+\t\t(ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1));\n \treturn o->flags & ((allow_hidden_ref ? HIDDEN_REF : 0) | OUR_REF);\n }\n \n@@ -458,8 +461,12 @@ static void check_non_tip(void)\n \tchar namebuf[42]; /* ^ + SHA-1 + LF */\n \tint i;\n \n-\t/* In the normal in-process case non-tip request can never happen */\n-\tif (!stateless_rpc)\n+\t/*\n+\t * In the normal in-process case without\n+\t * uploadpack.allowReachableSHA1InWant,\n+\t * non-tip requests can never happen.\n+\t */\n+\tif (!stateless_rpc && !(allow_unadvertised_object_request & ALLOW_REACHABLE_SHA1))\n \t\tgoto error;\n \n \tcmd.argv = argv;\n@@ -726,11 +733,13 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo\n \t\tstruct strbuf symref_info = STRBUF_INIT;\n \n \t\tformat_symref_info(&symref_info, cb_data);\n-\t\tpacket_write(1, \"%s %s%c%s%s%s%s agent=%s\\n\",\n+\t\tpacket_write(1, \"%s %s%c%s%s%s%s%s agent=%s\\n\",\n \t\t\t     sha1_to_hex(sha1), refname_nons,\n \t\t\t     0, capabilities,\n \t\t\t     (allow_unadvertised_object_request & ALLOW_TIP_SHA1) ?\n \t\t\t\t     \" allow-tip-sha1-in-want\" : \"\",\n+\t\t\t     (allow_unadvertised_object_request & ALLOW_REACHABLE_SHA1) ?\n+\t\t\t\t     \" allow-reachable-sha1-in-want\" : \"\",\n \t\t\t     stateless_rpc ? \" no-done\" : \"\",\n \t\t\t     symref_info.buf,\n \t\t\t     git_user_agent_sanitized());\n@@ -793,6 +802,9 @@ static int upload_pack_config(const char *var, const char *value, void *unused)\n \tif (!strcmp(\"uploadpack.allowtipsha1inwant\", var)) {\n \t\tif (git_config_bool(var, value))\n \t\t\tallow_unadvertised_object_request |= ALLOW_TIP_SHA1;\n+\t} else if (!strcmp(\"uploadpack.allowreachablesha1inwant\", var)) {\n+\t\tif (git_config_bool(var, value))\n+\t\t\tallow_unadvertised_object_request |= ALLOW_REACHABLE_SHA1;\n \t} else if (!strcmp(\"uploadpack.keepalive\", var)) {\n \t\tkeepalive = git_config_int(var, value);\n \t\tif (!keepalive)\n-- \n1.9.1\n"},{"id":"261607","messageId":"xmqqzj505isz.fsf@gitster.dls.corp.google.com","threadId":"39224","inReplyTo":"1432068269-14895-2-git-send-email-fredrik.medley@gmail.com","subject":"Re: [PATCH 2/3] upload-pack: prepare to extend allow-tip-sha1-in-want","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-19T22:00:44Z","receivedAt":"2015-05-19T22:00:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Fredrik Medley <fredrik.medley@gmail.com> writes:\n\n>  static int upload_pack_config(const char *var, const char *value, void *unused)\n>  {\n> -\tif (!strcmp(\"uploadpack.allowtipsha1inwant\", var))\n> -\t\tallow_tip_sha1_in_want = git_config_bool(var, value);\n> -\telse if (!strcmp(\"uploadpack.keepalive\", var)) {\n> +\tif (!strcmp(\"uploadpack.allowtipsha1inwant\", var)) {\n> +\t\tif (git_config_bool(var, value))\n> +\t\t\tallow_unadvertised_object_request |= ALLOW_TIP_SHA1;\n\nDoesn't this change break the behaviour?\n\nShouldn't you be able to say\n\n\t[uploadpack]\n        \tallowTipSHA1InWant = false\n\nin a higher-precedence configuration file to override the same\nvariable in other files in the configuration chain that may set it\nto true?\n\n> +\t} else if (!strcmp(\"uploadpack.keepalive\", var)) {\n>  \t\tkeepalive = git_config_int(var, value);\n>  \t\tif (!keepalive)\n>  \t\t\tkeepalive = -1;\n"},{"id":"261608","messageId":"xmqqvbfo5ijs.fsf@gitster.dls.corp.google.com","threadId":"39224","inReplyTo":"1432068269-14895-3-git-send-email-fredrik.medley@gmail.com","subject":"Re: [PATCH 3/3] upload-pack: optionally allow fetching reachable sha1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-19T22:06:15Z","receivedAt":"2015-05-19T22:06:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Fredrik Medley <fredrik.medley@gmail.com> writes:\n\n> diff --git a/fetch-pack.c b/fetch-pack.c\n> index 699f586..875b688 100644\n> --- a/fetch-pack.c\n> +++ b/fetch-pack.c\n> @@ -45,7 +45,9 @@ static int marked;\n>  static struct prio_queue rev_list = { compare_commits_by_commit_date };\n>  static int non_common_revs, multi_ack, use_sideband;\n>  /* Allow specifying sha1 if it is a ref tip. */\n> -#define ALLOW_TIP_SHA1\t01\n> +#define ALLOW_TIP_SHA1\t\t01\n\nWhat is this change about?  You do not have to align value columns.\n\n\n> -\tif ((allow_unadvertised_object_request & ALLOW_TIP_SHA1)) {\n> +\tif ((allow_unadvertised_object_request & (ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1))) {\n\nPerhaps wrap it like this?\n\n\tif ((allow_unadvertised_object_request &\n            (ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1))) {\n\n> @@ -444,7 +446,8 @@ static int get_common_commits(void)\n>  \n>  static int is_our_ref(struct object *o)\n>  {\n> -\tint allow_hidden_ref = (allow_unadvertised_object_request & ALLOW_TIP_SHA1);\n> +\tint allow_hidden_ref = (allow_unadvertised_object_request &\n> +\t\t(ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1));\n\nGood, but push that to even more right?\n\n> @@ -793,6 +802,9 @@ static int upload_pack_config(const char *var, const char *value, void *unused)\n>  \tif (!strcmp(\"uploadpack.allowtipsha1inwant\", var)) {\n>  \t\tif (git_config_bool(var, value))\n>  \t\t\tallow_unadvertised_object_request |= ALLOW_TIP_SHA1;\n> +\t} else if (!strcmp(\"uploadpack.allowreachablesha1inwant\", var)) {\n> +\t\tif (git_config_bool(var, value))\n> +\t\t\tallow_unadvertised_object_request |= ALLOW_REACHABLE_SHA1;\n>  \t} else if (!strcmp(\"uploadpack.keepalive\", var)) {\n>  \t\tkeepalive = git_config_int(var, value);\n>  \t\tif (!keepalive)\n\nSame issue as [2/3]\n\n\t\t...\n\t} else if (!strcmp(\"configVariable\", var)) {\n        \tif (git_config_bool(var, value))\n\t\t\tvalue |= ALLOW_THIS;\n\t\telse\n                \tvalue &= ~ALLOW_THIS;\n\t} else if ...\n\nshould be OK.\n\nThanks.\n"},{"id":"261697","messageId":"CABA5-zm77sbEvBV2LyFh7hweTjKenZvWWYtwEVZ2XAkDdV3WNg@mail.gmail.com","threadId":"39224","inReplyTo":"xmqqzj505isz.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/3] upload-pack: prepare to extend allow-tip-sha1-in-want","fromName":"Fredrik Medley","fromEmail":"fredrik.medley@gmail.com","sentAt":"2015-05-20T19:31:32Z","receivedAt":"2015-05-20T19:31:32Z","isPatch":true,"sender":{"key":"fredrik.medley@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5401799?v=4"},"body":"2015-05-20 0:00 GMT+02:00 Junio C Hamano <gitster@pobox.com>:\n> Fredrik Medley <fredrik.medley@gmail.com> writes:\n>\n>>  static int upload_pack_config(const char *var, const char *value, void *unused)\n>>  {\n>> -     if (!strcmp(\"uploadpack.allowtipsha1inwant\", var))\n>> -             allow_tip_sha1_in_want = git_config_bool(var, value);\n>> -     else if (!strcmp(\"uploadpack.keepalive\", var)) {\n>> +     if (!strcmp(\"uploadpack.allowtipsha1inwant\", var)) {\n>> +             if (git_config_bool(var, value))\n>> +                     allow_unadvertised_object_request |= ALLOW_TIP_SHA1;\n>\n> Doesn't this change break the behaviour?\n>\n> Shouldn't you be able to say\n>\n>         [uploadpack]\n>                 allowTipSHA1InWant = false\n>\n> in a higher-precedence configuration file to override the same\n> variable in other files in the configuration chain that may set it\n> to true?\n\nOf course, thought it work differently. Should I add tests with\ntest_config_global\nto check that loading the config works as well?\n\n>\n>> +     } else if (!strcmp(\"uploadpack.keepalive\", var)) {\n>>               keepalive = git_config_int(var, value);\n>>               if (!keepalive)\n>>                       keepalive = -1;\n>\n"},{"id":"261847","messageId":"1432239819-21794-1-git-send-email-fredrik.medley@gmail.com","threadId":"39224","inReplyTo":"1432068269-14895-1-git-send-email-fredrik.medley@gmail.com","subject":"[PATCH v5 1/3] config.txt: clarify allowTipSHA1InWant with camelCase","fromName":"Fredrik Medley","fromEmail":"fredrik.medley@gmail.com","sentAt":"2015-05-21T20:23:37Z","receivedAt":"2015-05-21T20:23:37Z","isPatch":true,"sender":{"key":"fredrik.medley@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5401799?v=4"},"body":"Most of the options in config.txt are camelCase. Improve the readability\nfor allowtipsha1inwant by changing to allowTipSHA1InWant.\n\nSigned-off-by: Fredrik Medley <fredrik.medley@gmail.com>\n---\nThis patch is optional. There has been work on fixing the whole\nDocumentation/config.txt which has not been merged yet. When adding\nallowReachableSHA1InWant later, it would be good to already have changed\nto allowTipSHA1InWant.\n\n Documentation/config.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 948b8b0..1311383 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2539,9 +2539,9 @@ uploadpack.hideRefs::\n \tare under the hierarchies listed on the value of this\n \tvariable is excluded, and is hidden from `git ls-remote`,\n \t`git fetch`, etc.  An attempt to fetch a hidden ref by `git\n-\tfetch` will fail.  See also `uploadpack.allowtipsha1inwant`.\n+\tfetch` will fail.  See also `uploadpack.allowTipSHA1InWant`.\n \n-uploadpack.allowtipsha1inwant::\n+uploadpack.allowTipSHA1InWant::\n \tWhen `uploadpack.hideRefs` is in effect, allow `upload-pack`\n \tto accept a fetch request that asks for an object at the tip\n \tof a hidden ref (by default, such a request is rejected).\n-- \n1.9.1\n"},{"id":"261845","messageId":"1432239819-21794-2-git-send-email-fredrik.medley@gmail.com","threadId":"39224","inReplyTo":"1432239819-21794-1-git-send-email-fredrik.medley@gmail.com","subject":"[PATCH v5 2/3] upload-pack: prepare to extend allow-tip-sha1-in-want","fromName":"Fredrik Medley","fromEmail":"fredrik.medley@gmail.com","sentAt":"2015-05-21T20:23:38Z","receivedAt":"2015-05-21T20:23:38Z","isPatch":true,"sender":{"key":"fredrik.medley@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5401799?v=4"},"body":"To allow future extensions, e.g. allowing non-tip sha1, replace the\nboolean allow_tip_sha1_in_want variable with the flag-style\nallow_request_with_bare_object_name variable.\n\nSigned-off-by: Fredrik Medley <fredrik.medley@gmail.com>\n---\n fetch-pack.c  |  9 ++++++---\n upload-pack.c | 20 +++++++++++++-------\n 2 files changed, 19 insertions(+), 10 deletions(-)\n\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 48526aa..699f586 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -43,7 +43,10 @@ static int marked;\n #define MAX_IN_VAIN 256\n \n static struct prio_queue rev_list = { compare_commits_by_commit_date };\n-static int non_common_revs, multi_ack, use_sideband, allow_tip_sha1_in_want;\n+static int non_common_revs, multi_ack, use_sideband;\n+/* Allow specifying sha1 if it is a ref tip. */\n+#define ALLOW_TIP_SHA1\t01\n+static int allow_unadvertised_object_request;\n \n static void rev_list_push(struct commit *commit, int mark)\n {\n@@ -542,7 +545,7 @@ static void filter_refs(struct fetch_pack_args *args,\n \t}\n \n \t/* Append unmatched requests to the list */\n-\tif (allow_tip_sha1_in_want) {\n+\tif ((allow_unadvertised_object_request & ALLOW_TIP_SHA1)) {\n \t\tfor (i = 0; i < nr_sought; i++) {\n \t\t\tunsigned char sha1[20];\n \n@@ -821,7 +824,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,\n \tif (server_supports(\"allow-tip-sha1-in-want\")) {\n \t\tif (args->verbose)\n \t\t\tfprintf(stderr, \"Server supports allow-tip-sha1-in-want\\n\");\n-\t\tallow_tip_sha1_in_want = 1;\n+\t\tallow_unadvertised_object_request |= ALLOW_TIP_SHA1;\n \t}\n \tif (!server_supports(\"thin-pack\"))\n \t\targs->use_thin_pack = 0;\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 745fda8..726486b 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -35,7 +35,9 @@ static int multi_ack;\n static int no_done;\n static int use_thin_pack, use_ofs_delta, use_include_tag;\n static int no_progress, daemon_mode;\n-static int allow_tip_sha1_in_want;\n+/* Allow specifying sha1 if it is a ref tip. */\n+#define ALLOW_TIP_SHA1\t01\n+static int allow_unadvertised_object_request;\n static int shallow_nr;\n static struct object_array have_obj;\n static struct object_array want_obj;\n@@ -442,8 +444,8 @@ static int get_common_commits(void)\n \n static int is_our_ref(struct object *o)\n {\n-\treturn o->flags &\n-\t\t((allow_tip_sha1_in_want ? HIDDEN_REF : 0) | OUR_REF);\n+\tint allow_hidden_ref = (allow_unadvertised_object_request & ALLOW_TIP_SHA1);\n+\treturn o->flags & ((allow_hidden_ref ? HIDDEN_REF : 0) | OUR_REF);\n }\n \n static void check_non_tip(void)\n@@ -727,7 +729,8 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo\n \t\tpacket_write(1, \"%s %s%c%s%s%s%s agent=%s\\n\",\n \t\t\t     sha1_to_hex(sha1), refname_nons,\n \t\t\t     0, capabilities,\n-\t\t\t     allow_tip_sha1_in_want ? \" allow-tip-sha1-in-want\" : \"\",\n+\t\t\t     (allow_unadvertised_object_request & ALLOW_TIP_SHA1) ?\n+\t\t\t\t     \" allow-tip-sha1-in-want\" : \"\",\n \t\t\t     stateless_rpc ? \" no-done\" : \"\",\n \t\t\t     symref_info.buf,\n \t\t\t     git_user_agent_sanitized());\n@@ -787,9 +790,12 @@ static void upload_pack(void)\n \n static int upload_pack_config(const char *var, const char *value, void *unused)\n {\n-\tif (!strcmp(\"uploadpack.allowtipsha1inwant\", var))\n-\t\tallow_tip_sha1_in_want = git_config_bool(var, value);\n-\telse if (!strcmp(\"uploadpack.keepalive\", var)) {\n+\tif (!strcmp(\"uploadpack.allowtipsha1inwant\", var)) {\n+\t\tif (git_config_bool(var, value))\n+\t\t\tallow_unadvertised_object_request |= ALLOW_TIP_SHA1;\n+\t\telse\n+\t\t\tallow_unadvertised_object_request &= ~ALLOW_TIP_SHA1;\n+\t} else if (!strcmp(\"uploadpack.keepalive\", var)) {\n \t\tkeepalive = git_config_int(var, value);\n \t\tif (!keepalive)\n \t\t\tkeepalive = -1;\n-- \n1.9.1\n"},{"id":"261846","messageId":"1432239819-21794-3-git-send-email-fredrik.medley@gmail.com","threadId":"39224","inReplyTo":"1432239819-21794-1-git-send-email-fredrik.medley@gmail.com","subject":"[PATCH v5 3/3] upload-pack: optionally allow fetching reachable sha1","fromName":"Fredrik Medley","fromEmail":"fredrik.medley@gmail.com","sentAt":"2015-05-21T20:23:39Z","receivedAt":"2015-05-21T20:23:39Z","isPatch":true,"sender":{"key":"fredrik.medley@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5401799?v=4"},"body":"With uploadpack.allowReachableSHA1InWant configuration option set on the\nserver side, \"git fetch\" can make a request with a \"want\" line that names\nan object that has not been advertised (likely to have been obtained out\nof band or from a submodule pointer). Only objects reachable from the\nbranch tips, i.e. the union of advertised branches and branches hidden by\ntransfer.hideRefs, will be processed. Note that there is an associated\ncost of having to walk back the history to check the reachability.\n\nThis feature can be used when obtaining the content of a certain commit,\nfor which the sha1 is known, without the need of cloning the whole\nrepository, especially if a shallow fetch is used. Useful cases are e.g.\nrepositories containing large files in the history, fetching only the\nneeded data for a submodule checkout, when sharing a sha1 without telling\nwhich exact branch it belongs to and in Gerrit, if you think in terms of\ncommits instead of change numbers. (The Gerrit case has already been\nsolved through allowTipSHA1InWant as every Gerrit change has a ref.)\n\nSigned-off-by: Fredrik Medley <fredrik.medley@gmail.com>\n---\n Documentation/config.txt                          |  6 +++\n Documentation/technical/http-protocol.txt         |  3 +-\n Documentation/technical/protocol-capabilities.txt |  7 +++\n fetch-pack.c                                      | 10 ++++-\n t/t5516-fetch-push.sh                             | 55 +++++++++++++++++++++++\n upload-pack.c                                     | 22 +++++++--\n 6 files changed, 97 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 1311383..b8215ba 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2547,6 +2547,12 @@ uploadpack.allowTipSHA1InWant::\n \tof a hidden ref (by default, such a request is rejected).\n \tsee also `uploadpack.hideRefs`.\n \n+uploadpack.allowReachableSHA1InWant::\n+\tAllow `upload-pack` to accept a fetch request that asks for an\n+\tobject that is reachable from any ref tip. However, note that\n+\tcalculating object reachability is computationally expensive.\n+\tDefaults to `false`.\n+\n uploadpack.keepAlive::\n \tWhen `upload-pack` has started `pack-objects`, there may be a\n \tquiet period while `pack-objects` prepares the pack. Normally\ndiff --git a/Documentation/technical/http-protocol.txt b/Documentation/technical/http-protocol.txt\nindex 229f845..1c561bd 100644\n--- a/Documentation/technical/http-protocol.txt\n+++ b/Documentation/technical/http-protocol.txt\n@@ -319,7 +319,8 @@ Servers SHOULD support all capabilities defined here.\n Clients MUST send at least one \"want\" command in the request body.\n Clients MUST NOT reference an id in a \"want\" command which did not\n appear in the response obtained through ref discovery unless the\n-server advertises capability `allow-tip-sha1-in-want`.\n+server advertises capability `allow-tip-sha1-in-want` or\n+`allow-reachable-sha1-in-want`.\n \n   compute_request   =  want_list\n \t\t       have_list\ndiff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt\nindex 4f8a7bf..265fcab 100644\n--- a/Documentation/technical/protocol-capabilities.txt\n+++ b/Documentation/technical/protocol-capabilities.txt\n@@ -260,6 +260,13 @@ If the upload-pack server advertises this capability, fetch-pack may\n send \"want\" lines with SHA-1s that exist at the server but are not\n advertised by upload-pack.\n \n+allow-reachable-sha1-in-want\n+----------------------\n+\n+If the upload-pack server advertises this capability, fetch-pack may\n+send \"want\" lines with SHA-1s that exist at the server but are not\n+advertised by upload-pack.\n+\n push-cert=<nonce>\n -----------------\n \ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 699f586..0d83d47 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -46,6 +46,8 @@ static struct prio_queue rev_list = { compare_commits_by_commit_date };\n static int non_common_revs, multi_ack, use_sideband;\n /* Allow specifying sha1 if it is a ref tip. */\n #define ALLOW_TIP_SHA1\t01\n+/* Allow request of a sha1 if it is reachable from a ref (possibly hidden ref). */\n+#define ALLOW_REACHABLE_SHA1\t02\n static int allow_unadvertised_object_request;\n \n static void rev_list_push(struct commit *commit, int mark)\n@@ -545,7 +547,8 @@ static void filter_refs(struct fetch_pack_args *args,\n \t}\n \n \t/* Append unmatched requests to the list */\n-\tif ((allow_unadvertised_object_request & ALLOW_TIP_SHA1)) {\n+\tif ((allow_unadvertised_object_request &\n+\t    (ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1))) {\n \t\tfor (i = 0; i < nr_sought; i++) {\n \t\t\tunsigned char sha1[20];\n \n@@ -826,6 +829,11 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,\n \t\t\tfprintf(stderr, \"Server supports allow-tip-sha1-in-want\\n\");\n \t\tallow_unadvertised_object_request |= ALLOW_TIP_SHA1;\n \t}\n+\tif (server_supports(\"allow-reachable-sha1-in-want\")) {\n+\t\tif (args->verbose)\n+\t\t\tfprintf(stderr, \"Server supports allow-reachable-sha1-in-want\\n\");\n+\t\tallow_unadvertised_object_request |= ALLOW_REACHABLE_SHA1;\n+\t}\n \tif (!server_supports(\"thin-pack\"))\n \t\targs->use_thin_pack = 0;\n \tif (!server_supports(\"no-progress\"))\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 8a5f236..fdcc114 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1120,6 +1120,61 @@ test_expect_success 'fetch exact SHA1' '\n \t)\n '\n \n+for configallowtipsha1inwant in true false\n+do\n+\ttest_expect_success \"shallow fetch reachable SHA1 (but not a ref), allowtipsha1inwant=$configallowtipsha1inwant\" '\n+\t\tmk_empty testrepo &&\n+\t\t(\n+\t\t\tcd testrepo &&\n+\t\t\tgit config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&\n+\t\t\tgit commit --allow-empty -m foo &&\n+\t\t\tgit commit --allow-empty -m bar\n+\t\t) &&\n+\t\tSHA1=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&\n+\t\tmk_empty shallow &&\n+\t\t(\n+\t\t\tcd shallow &&\n+\t\t\ttest_must_fail git fetch --depth=1 ../testrepo/.git $SHA1 &&\n+\t\t\tgit --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&\n+\t\t\tgit fetch --depth=1 ../testrepo/.git $SHA1 &&\n+\t\t\tgit cat-file commit $SHA1 >/dev/null\n+\t\t)\n+\t'\n+\n+\ttest_expect_success \"deny fetch unreachable SHA1, allowtipsha1inwant=$configallowtipsha1inwant\" '\n+\t\tmk_empty testrepo &&\n+\t\t(\n+\t\t\tcd testrepo &&\n+\t\t\tgit config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&\n+\t\t\tgit commit --allow-empty -m foo &&\n+\t\t\tgit commit --allow-empty -m bar &&\n+\t\t\tgit commit --allow-empty -m xyz\n+\t\t)\n+\t\tSHA1_1=$(git --git-dir=testrepo/.git rev-parse HEAD^^) &&\n+\t\tSHA1_2=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&\n+\t\tSHA1_3=$(git --git-dir=testrepo/.git rev-parse HEAD) &&\n+\t\t(\n+\t\t\tcd testrepo &&\n+\t\t\tgit reset --hard $SHA1_2 &&\n+\t\t\tgit cat-file commit $SHA1_3 >/dev/null &&\n+\t\t\tgit cat-file commit $SHA1_3 >/dev/null\n+\t\t) &&\n+\t\tmk_empty shallow &&\n+\t\t(\n+\t\t\tcd shallow &&\n+\t\t\ttest_must_fail git fetch ../testrepo/.git $SHA1_3 &&\n+\t\t\ttest_must_fail git fetch ../testrepo/.git $SHA1_1 &&\n+\t\t\tgit --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&\n+\t\t\tgit fetch ../testrepo/.git $SHA1_1 &&\n+\t\t\tgit cat-file commit $SHA1_1 >/dev/null &&\n+\t\t\ttest_must_fail git cat-file commit $SHA1_2 >/dev/null &&\n+\t\t\tgit fetch ../testrepo/.git $SHA1_2 &&\n+\t\t\tgit cat-file commit $SHA1_2 >/dev/null &&\n+\t\t\ttest_must_fail git fetch ../testrepo/.git $SHA1_3\n+\t\t)\n+\t'\n+done\n+\n test_expect_success 'fetch follows tags by default' '\n \tmk_test testrepo heads/master &&\n \trm -fr src dst &&\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 726486b..15571d7 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -37,6 +37,8 @@ static int use_thin_pack, use_ofs_delta, use_include_tag;\n static int no_progress, daemon_mode;\n /* Allow specifying sha1 if it is a ref tip. */\n #define ALLOW_TIP_SHA1\t01\n+/* Allow request of a sha1 if it is reachable from a ref (possibly hidden ref). */\n+#define ALLOW_REACHABLE_SHA1\t02\n static int allow_unadvertised_object_request;\n static int shallow_nr;\n static struct object_array have_obj;\n@@ -444,7 +446,8 @@ static int get_common_commits(void)\n \n static int is_our_ref(struct object *o)\n {\n-\tint allow_hidden_ref = (allow_unadvertised_object_request & ALLOW_TIP_SHA1);\n+\tint allow_hidden_ref = (allow_unadvertised_object_request &\n+\t\t\t(ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1));\n \treturn o->flags & ((allow_hidden_ref ? HIDDEN_REF : 0) | OUR_REF);\n }\n \n@@ -458,8 +461,12 @@ static void check_non_tip(void)\n \tchar namebuf[42]; /* ^ + SHA-1 + LF */\n \tint i;\n \n-\t/* In the normal in-process case non-tip request can never happen */\n-\tif (!stateless_rpc)\n+\t/*\n+\t * In the normal in-process case without\n+\t * uploadpack.allowReachableSHA1InWant,\n+\t * non-tip requests can never happen.\n+\t */\n+\tif (!stateless_rpc && !(allow_unadvertised_object_request & ALLOW_REACHABLE_SHA1))\n \t\tgoto error;\n \n \tcmd.argv = argv;\n@@ -726,11 +733,13 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo\n \t\tstruct strbuf symref_info = STRBUF_INIT;\n \n \t\tformat_symref_info(&symref_info, cb_data);\n-\t\tpacket_write(1, \"%s %s%c%s%s%s%s agent=%s\\n\",\n+\t\tpacket_write(1, \"%s %s%c%s%s%s%s%s agent=%s\\n\",\n \t\t\t     sha1_to_hex(sha1), refname_nons,\n \t\t\t     0, capabilities,\n \t\t\t     (allow_unadvertised_object_request & ALLOW_TIP_SHA1) ?\n \t\t\t\t     \" allow-tip-sha1-in-want\" : \"\",\n+\t\t\t     (allow_unadvertised_object_request & ALLOW_REACHABLE_SHA1) ?\n+\t\t\t\t     \" allow-reachable-sha1-in-want\" : \"\",\n \t\t\t     stateless_rpc ? \" no-done\" : \"\",\n \t\t\t     symref_info.buf,\n \t\t\t     git_user_agent_sanitized());\n@@ -795,6 +804,11 @@ static int upload_pack_config(const char *var, const char *value, void *unused)\n \t\t\tallow_unadvertised_object_request |= ALLOW_TIP_SHA1;\n \t\telse\n \t\t\tallow_unadvertised_object_request &= ~ALLOW_TIP_SHA1;\n+\t} else if (!strcmp(\"uploadpack.allowreachablesha1inwant\", var)) {\n+\t\tif (git_config_bool(var, value))\n+\t\t\tallow_unadvertised_object_request |= ALLOW_REACHABLE_SHA1;\n+\t\telse\n+\t\t\tallow_unadvertised_object_request &= ~ALLOW_REACHABLE_SHA1;\n \t} else if (!strcmp(\"uploadpack.keepalive\", var)) {\n \t\tkeepalive = git_config_int(var, value);\n \t\tif (!keepalive)\n-- \n1.9.1\n"},{"id":"261859","messageId":"xmqqpp5th9dy.fsf@gitster.dls.corp.google.com","threadId":"39224","inReplyTo":"1432239819-21794-2-git-send-email-fredrik.medley@gmail.com","subject":"Re: [PATCH v5 2/3] upload-pack: prepare to extend allow-tip-sha1-in-want","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-21T22:07:53Z","receivedAt":"2015-05-21T22:07:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Fredrik Medley <fredrik.medley@gmail.com> writes:\n\n> To allow future extensions, e.g. allowing non-tip sha1, replace the\n> boolean allow_tip_sha1_in_want variable with the flag-style\n> allow_request_with_bare_object_name variable.\n>\n> Signed-off-by: Fredrik Medley <fredrik.medley@gmail.com>\n> ---\n>  fetch-pack.c  |  9 ++++++---\n>  upload-pack.c | 20 +++++++++++++-------\n>  2 files changed, 19 insertions(+), 10 deletions(-)\n>\n> diff --git a/fetch-pack.c b/fetch-pack.c\n> index 48526aa..699f586 100644\n> --- a/fetch-pack.c\n> +++ b/fetch-pack.c\n> @@ -43,7 +43,10 @@ static int marked;\n>  #define MAX_IN_VAIN 256\n>  \n>  static struct prio_queue rev_list = { compare_commits_by_commit_date };\n> -static int non_common_revs, multi_ack, use_sideband, allow_tip_sha1_in_want;\n> +static int non_common_revs, multi_ack, use_sideband;\n> +/* Allow specifying sha1 if it is a ref tip. */\n> +#define ALLOW_TIP_SHA1\t01\n> +static int allow_unadvertised_object_request;\n\nIt is better to use \"unsigned int\" for these bit masks, as we are\nnot interested in the top-most bit getting special-cased by using a\nsigned type.  I'll amend this (and the one in upload-pack.c) while\napplying, so no need to resend only to correct these two, unless you\nhave other reasons to reroll.\n\nThanks.\n\n> diff --git a/upload-pack.c b/upload-pack.c\n> index 745fda8..726486b 100644\n> --- a/upload-pack.c\n> +++ b/upload-pack.c\n> @@ -35,7 +35,9 @@ static int multi_ack;\n>  static int no_done;\n>  static int use_thin_pack, use_ofs_delta, use_include_tag;\n>  static int no_progress, daemon_mode;\n> -static int allow_tip_sha1_in_want;\n> +/* Allow specifying sha1 if it is a ref tip. */\n> +#define ALLOW_TIP_SHA1\t01\n> +static int allow_unadvertised_object_request;\n"},{"id":"261861","messageId":"xmqqioblh91w.fsf@gitster.dls.corp.google.com","threadId":"39224","inReplyTo":"1432239819-21794-3-git-send-email-fredrik.medley@gmail.com","subject":"Re: [PATCH v5 3/3] upload-pack: optionally allow fetching reachable sha1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-21T22:15:07Z","receivedAt":"2015-05-21T22:15:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Fredrik Medley <fredrik.medley@gmail.com> writes:\n\n> --- a/Documentation/technical/protocol-capabilities.txt\n> +++ b/Documentation/technical/protocol-capabilities.txt\n> @@ -260,6 +260,13 @@ If the upload-pack server advertises this capability, fetch-pack may\n>  send \"want\" lines with SHA-1s that exist at the server but are not\n>  advertised by upload-pack.\n>  \n> +allow-reachable-sha1-in-want\n> +----------------------\n\nThis is an underline applied to one line prior, and their length\nmust match.  I'll amend while applying (attached at end), so there\nis no need to resend with correction unless you have other reasons\nto do so.\n\n> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\n> index 8a5f236..fdcc114 100755\n> --- a/t/t5516-fetch-push.sh\n> +++ b/t/t5516-fetch-push.sh\n> @@ -1120,6 +1120,61 @@ test_expect_success 'fetch exact SHA1' '\n>  \t)\n>  '\n\nIt looks like this new set of tests are well thought out; good job.\n\nI spotted a few minor nits, though.  All I'll amend while applying\nso there is no need to resend only to correct them.\n\n> +for configallowtipsha1inwant in true false\n> +do\n> +\ttest_expect_success \"shallow fetch reachable SHA1 (but not a ref), allowtipsha1inwant=$configallowtipsha1inwant\" '\n> +\t\tmk_empty testrepo &&\n> +\t\t(\n> +\t\t\tcd testrepo &&\n> +\t\t\tgit config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&\n> +\t\t\tgit commit --allow-empty -m foo &&\n> +\t\t\tgit commit --allow-empty -m bar\n> +\t\t) &&\n> +\t\tSHA1=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&\n> +\t\tmk_empty shallow &&\n> +\t\t(\n> +\t\t\tcd shallow &&\n> +\t\t\ttest_must_fail git fetch --depth=1 ../testrepo/.git $SHA1 &&\n\nThis tries to fetch one before the tip with \"allowTipSHA1InWant\" set\nto true or false; either should fail.  Good.\n\n> +\t\t\tgit --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&\n> +\t\t\tgit fetch --depth=1 ../testrepo/.git $SHA1 &&\n\nAnd regardless of allowTip setting, with allowReachable set to true,\nfetching the reachable HEAD^ would succeed.  Good.\n\n> +\t\t\tgit cat-file commit $SHA1 >/dev/null\n\nMinor nit; drop \">/dev/null\", as test framework will squelch the\noutput by default, and when the test is run with \"-v\" option, the\noutput would help debugging the script.\n\n> +\t\t)\n> +\t'\n> +\n> +\ttest_expect_success \"deny fetch unreachable SHA1, allowtipsha1inwant=$configallowtipsha1inwant\" '\n> +\t\tmk_empty testrepo &&\n> +\t\t(\n> +\t\t\tcd testrepo &&\n> +\t\t\tgit config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&\n> +\t\t\tgit commit --allow-empty -m foo &&\n> +\t\t\tgit commit --allow-empty -m bar &&\n> +\t\t\tgit commit --allow-empty -m xyz\n> +\t\t)\n\nBroken && chain\n\n> +\t\tSHA1_1=$(git --git-dir=testrepo/.git rev-parse HEAD^^) &&\n> +\t\tSHA1_2=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&\n> +\t\tSHA1_3=$(git --git-dir=testrepo/.git rev-parse HEAD) &&\n> +\t\t(\n> +\t\t\tcd testrepo &&\n> +\t\t\tgit reset --hard $SHA1_2 &&\n\nWe have one before the tip (SHA1_1), one after the tip and no longer\nreachable (SHA1_3); SHA1_2 is sitting at the tip of a ref.\n\n> +\t\t\tgit cat-file commit $SHA1_3 >/dev/null &&\n> +\t\t\tgit cat-file commit $SHA1_3 >/dev/null\n\nI think one of the latter two is $SHA1_1, i.e. you make sure SHA1_{1,2,3}\nare there in testrepo.\n\n> +\t\t) &&\n> +\t\tmk_empty shallow &&\n> +\t\t(\n> +\t\t\tcd shallow &&\n> +\t\t\ttest_must_fail git fetch ../testrepo/.git $SHA1_3 &&\n> +\t\t\ttest_must_fail git fetch ../testrepo/.git $SHA1_1 &&\n\nWith allowTip only, whether it is set to true or false, fetching _1\nor _3 that are not at tip will fail.  Good.\n\n> +\t\t\tgit --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&\n> +\t\t\tgit fetch ../testrepo/.git $SHA1_1 &&\n> +\t\t\tgit cat-file commit $SHA1_1 >/dev/null &&\n\nWith allowReachable, _1 which is reachable from tip is possible,\nregardless of the setting of allowTip.  Good.\n\n> +\t\t\ttest_must_fail git cat-file commit $SHA1_2 >/dev/null &&\n\nAnd fetching _1 will not pull in _2, which is _1's child, that we\ndid not ask for.  Good (but it is probably not very relevant for the\npurpose of these tests).\n\n> +\t\t\tgit fetch ../testrepo/.git $SHA1_2 &&\n> +\t\t\tgit cat-file commit $SHA1_2 >/dev/null &&\n\nAnd of course, _2 can be fetched.\n\n> +\t\t\ttest_must_fail git fetch ../testrepo/.git $SHA1_3\n\nAnd _3 that is not reachable cannot.\n\n> +\t\t)\n> +\t'\n> +done\n\nAgain, very carefully covering combinations.  Good.\n\n\n\n\n Documentation/technical/protocol-capabilities.txt |  2 +-\n t/t5516-fetch-push.sh                             | 14 +++++++-------\n 2 files changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt\nindex 265fcab..eaab6b4 100644\n--- a/Documentation/technical/protocol-capabilities.txt\n+++ b/Documentation/technical/protocol-capabilities.txt\n@@ -261,7 +261,7 @@ send \"want\" lines with SHA-1s that exist at the server but are not\n advertised by upload-pack.\n \n allow-reachable-sha1-in-want\n-----------------------\n+----------------------------\n \n If the upload-pack server advertises this capability, fetch-pack may\n send \"want\" lines with SHA-1s that exist at the server but are not\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex fdcc114..ec22c98 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1137,7 +1137,7 @@ do\n \t\t\ttest_must_fail git fetch --depth=1 ../testrepo/.git $SHA1 &&\n \t\t\tgit --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&\n \t\t\tgit fetch --depth=1 ../testrepo/.git $SHA1 &&\n-\t\t\tgit cat-file commit $SHA1 >/dev/null\n+\t\t\tgit cat-file commit $SHA1\n \t\t)\n \t'\n \n@@ -1149,15 +1149,15 @@ do\n \t\t\tgit commit --allow-empty -m foo &&\n \t\t\tgit commit --allow-empty -m bar &&\n \t\t\tgit commit --allow-empty -m xyz\n-\t\t)\n+\t\t) &&\n \t\tSHA1_1=$(git --git-dir=testrepo/.git rev-parse HEAD^^) &&\n \t\tSHA1_2=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&\n \t\tSHA1_3=$(git --git-dir=testrepo/.git rev-parse HEAD) &&\n \t\t(\n \t\t\tcd testrepo &&\n \t\t\tgit reset --hard $SHA1_2 &&\n-\t\t\tgit cat-file commit $SHA1_3 >/dev/null &&\n-\t\t\tgit cat-file commit $SHA1_3 >/dev/null\n+\t\t\tgit cat-file commit $SHA1_1 &&\n+\t\t\tgit cat-file commit $SHA1_3\n \t\t) &&\n \t\tmk_empty shallow &&\n \t\t(\n@@ -1166,10 +1166,10 @@ do\n \t\t\ttest_must_fail git fetch ../testrepo/.git $SHA1_1 &&\n \t\t\tgit --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&\n \t\t\tgit fetch ../testrepo/.git $SHA1_1 &&\n-\t\t\tgit cat-file commit $SHA1_1 >/dev/null &&\n-\t\t\ttest_must_fail git cat-file commit $SHA1_2 >/dev/null &&\n+\t\t\tgit cat-file commit $SHA1_1 &&\n+\t\t\ttest_must_fail git cat-file commit $SHA1_2 &&\n \t\t\tgit fetch ../testrepo/.git $SHA1_2 &&\n-\t\t\tgit cat-file commit $SHA1_2 >/dev/null &&\n+\t\t\tgit cat-file commit $SHA1_2 &&\n \t\t\ttest_must_fail git fetch ../testrepo/.git $SHA1_3\n \t\t)\n \t'\n-- \n2.4.1-439-gcfa393f\n"},{"id":"261964","messageId":"CABA5-zm4E71DV4hMw1Tifn-gqE4LzDDRzuo4vRL848bo_V5LLA@mail.gmail.com","threadId":"39224","inReplyTo":"xmqqpp5th9dy.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5 2/3] upload-pack: prepare to extend allow-tip-sha1-in-want","fromName":"Fredrik Medley","fromEmail":"fredrik.medley@gmail.com","sentAt":"2015-05-22T23:47:40Z","receivedAt":"2015-05-22T23:47:40Z","isPatch":true,"sender":{"key":"fredrik.medley@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5401799?v=4"},"body":"2015-05-22 0:07 GMT+02:00 Junio C Hamano <gitster@pobox.com>:\n> Fredrik Medley <fredrik.medley@gmail.com> writes:\n>\n>> To allow future extensions, e.g. allowing non-tip sha1, replace the\n>> boolean allow_tip_sha1_in_want variable with the flag-style\n>> allow_request_with_bare_object_name variable.\n>>\n>> Signed-off-by: Fredrik Medley <fredrik.medley@gmail.com>\n>> ---\n>>  fetch-pack.c  |  9 ++++++---\n>>  upload-pack.c | 20 +++++++++++++-------\n>>  2 files changed, 19 insertions(+), 10 deletions(-)\n>>\n>> diff --git a/fetch-pack.c b/fetch-pack.c\n>> index 48526aa..699f586 100644\n>> --- a/fetch-pack.c\n>> +++ b/fetch-pack.c\n>> @@ -43,7 +43,10 @@ static int marked;\n>>  #define MAX_IN_VAIN 256\n>>\n>>  static struct prio_queue rev_list = { compare_commits_by_commit_date };\n>> -static int non_common_revs, multi_ack, use_sideband, allow_tip_sha1_in_want;\n>> +static int non_common_revs, multi_ack, use_sideband;\n>> +/* Allow specifying sha1 if it is a ref tip. */\n>> +#define ALLOW_TIP_SHA1       01\n>> +static int allow_unadvertised_object_request;\n>\n> It is better to use \"unsigned int\" for these bit masks, as we are\n> not interested in the top-most bit getting special-cased by using a\n> signed type.  I'll amend this (and the one in upload-pack.c) while\n> applying, so no need to resend only to correct these two, unless you\n> have other reasons to reroll.\n\nSounds like a good idea to change. Please amend it while applying.\n\n>\n> Thanks.\n\nThank you too for the review.\n\n>\n>> diff --git a/upload-pack.c b/upload-pack.c\n>> index 745fda8..726486b 100644\n>> --- a/upload-pack.c\n>> +++ b/upload-pack.c\n>> @@ -35,7 +35,9 @@ static int multi_ack;\n>>  static int no_done;\n>>  static int use_thin_pack, use_ofs_delta, use_include_tag;\n>>  static int no_progress, daemon_mode;\n>> -static int allow_tip_sha1_in_want;\n>> +/* Allow specifying sha1 if it is a ref tip. */\n>> +#define ALLOW_TIP_SHA1       01\n>> +static int allow_unadvertised_object_request;\n>\n"},{"id":"261969","messageId":"CABA5-zm3LqA+BAHj0EegoXmTuaNobhEHyD0v2kg=VfCzzqx-+A@mail.gmail.com","threadId":"39224","inReplyTo":"xmqqioblh91w.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v5 3/3] upload-pack: optionally allow fetching reachable sha1","fromName":"Fredrik Medley","fromEmail":"fredrik.medley@gmail.com","sentAt":"2015-05-22T23:55:12Z","receivedAt":"2015-05-22T23:55:12Z","isPatch":true,"sender":{"key":"fredrik.medley@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5401799?v=4"},"body":"2015-05-22 0:15 GMT+02:00 Junio C Hamano <gitster@pobox.com>:\n> Fredrik Medley <fredrik.medley@gmail.com> writes:\n>\n>> --- a/Documentation/technical/protocol-capabilities.txt\n>> +++ b/Documentation/technical/protocol-capabilities.txt\n>> @@ -260,6 +260,13 @@ If the upload-pack server advertises this capability, fetch-pack may\n>>  send \"want\" lines with SHA-1s that exist at the server but are not\n>>  advertised by upload-pack.\n>>\n>> +allow-reachable-sha1-in-want\n>> +----------------------\n>\n> This is an underline applied to one line prior, and their length\n> must match.  I'll amend while applying (attached at end), so there\n> is no need to resend with correction unless you have other reasons\n> to do so.\n>\n>> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\n>> index 8a5f236..fdcc114 100755\n>> --- a/t/t5516-fetch-push.sh\n>> +++ b/t/t5516-fetch-push.sh\n>> @@ -1120,6 +1120,61 @@ test_expect_success 'fetch exact SHA1' '\n>>       )\n>>  '\n>\n> It looks like this new set of tests are well thought out; good job.\n>\n> I spotted a few minor nits, though.  All I'll amend while applying\n> so there is no need to resend only to correct them.\n\nI agree on all your comments and your proposed amendment further down\nlooks good.\nShould the test code contain the explanations you've written in this email?\n\n>\n>> +for configallowtipsha1inwant in true false\n>> +do\n>> +     test_expect_success \"shallow fetch reachable SHA1 (but not a ref), allowtipsha1inwant=$configallowtipsha1inwant\" '\n>> +             mk_empty testrepo &&\n>> +             (\n>> +                     cd testrepo &&\n>> +                     git config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&\n>> +                     git commit --allow-empty -m foo &&\n>> +                     git commit --allow-empty -m bar\n>> +             ) &&\n>> +             SHA1=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&\n>> +             mk_empty shallow &&\n>> +             (\n>> +                     cd shallow &&\n>> +                     test_must_fail git fetch --depth=1 ../testrepo/.git $SHA1 &&\n>\n> This tries to fetch one before the tip with \"allowTipSHA1InWant\" set\n> to true or false; either should fail.  Good.\n>\n>> +                     git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&\n>> +                     git fetch --depth=1 ../testrepo/.git $SHA1 &&\n>\n> And regardless of allowTip setting, with allowReachable set to true,\n> fetching the reachable HEAD^ would succeed.  Good.\n>\n>> +                     git cat-file commit $SHA1 >/dev/null\n>\n> Minor nit; drop \">/dev/null\", as test framework will squelch the\n> output by default, and when the test is run with \"-v\" option, the\n> output would help debugging the script.\n>\n>> +             )\n>> +     '\n>> +\n>> +     test_expect_success \"deny fetch unreachable SHA1, allowtipsha1inwant=$configallowtipsha1inwant\" '\n>> +             mk_empty testrepo &&\n>> +             (\n>> +                     cd testrepo &&\n>> +                     git config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&\n>> +                     git commit --allow-empty -m foo &&\n>> +                     git commit --allow-empty -m bar &&\n>> +                     git commit --allow-empty -m xyz\n>> +             )\n>\n> Broken && chain\n>\n>> +             SHA1_1=$(git --git-dir=testrepo/.git rev-parse HEAD^^) &&\n>> +             SHA1_2=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&\n>> +             SHA1_3=$(git --git-dir=testrepo/.git rev-parse HEAD) &&\n>> +             (\n>> +                     cd testrepo &&\n>> +                     git reset --hard $SHA1_2 &&\n>\n> We have one before the tip (SHA1_1), one after the tip and no longer\n> reachable (SHA1_3); SHA1_2 is sitting at the tip of a ref.\n>\n>> +                     git cat-file commit $SHA1_3 >/dev/null &&\n>> +                     git cat-file commit $SHA1_3 >/dev/null\n>\n> I think one of the latter two is $SHA1_1, i.e. you make sure SHA1_{1,2,3}\n> are there in testrepo.\n\nYes, that was intended.\n\n>\n>> +             ) &&\n>> +             mk_empty shallow &&\n>> +             (\n>> +                     cd shallow &&\n>> +                     test_must_fail git fetch ../testrepo/.git $SHA1_3 &&\n>> +                     test_must_fail git fetch ../testrepo/.git $SHA1_1 &&\n>\n> With allowTip only, whether it is set to true or false, fetching _1\n> or _3 that are not at tip will fail.  Good.\n>\n>> +                     git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&\n>> +                     git fetch ../testrepo/.git $SHA1_1 &&\n>> +                     git cat-file commit $SHA1_1 >/dev/null &&\n>\n> With allowReachable, _1 which is reachable from tip is possible,\n> regardless of the setting of allowTip.  Good.\n>\n>> +                     test_must_fail git cat-file commit $SHA1_2 >/dev/null &&\n>\n> And fetching _1 will not pull in _2, which is _1's child, that we\n> did not ask for.  Good (but it is probably not very relevant for the\n> purpose of these tests).\n>\n>> +                     git fetch ../testrepo/.git $SHA1_2 &&\n>> +                     git cat-file commit $SHA1_2 >/dev/null &&\n>\n> And of course, _2 can be fetched.\n>\n>> +                     test_must_fail git fetch ../testrepo/.git $SHA1_3\n>\n> And _3 that is not reachable cannot.\n>\n>> +             )\n>> +     '\n>> +done\n>\n> Again, very carefully covering combinations.  Good.\n>\n>\n>\n>\n>  Documentation/technical/protocol-capabilities.txt |  2 +-\n>  t/t5516-fetch-push.sh                             | 14 +++++++-------\n>  2 files changed, 8 insertions(+), 8 deletions(-)\n>\n> diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt\n> index 265fcab..eaab6b4 100644\n> --- a/Documentation/technical/protocol-capabilities.txt\n> +++ b/Documentation/technical/protocol-capabilities.txt\n> @@ -261,7 +261,7 @@ send \"want\" lines with SHA-1s that exist at the server but are not\n>  advertised by upload-pack.\n>\n>  allow-reachable-sha1-in-want\n> -----------------------\n> +----------------------------\n>\n>  If the upload-pack server advertises this capability, fetch-pack may\n>  send \"want\" lines with SHA-1s that exist at the server but are not\n> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\n> index fdcc114..ec22c98 100755\n> --- a/t/t5516-fetch-push.sh\n> +++ b/t/t5516-fetch-push.sh\n> @@ -1137,7 +1137,7 @@ do\n>                         test_must_fail git fetch --depth=1 ../testrepo/.git $SHA1 &&\n>                         git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&\n>                         git fetch --depth=1 ../testrepo/.git $SHA1 &&\n> -                       git cat-file commit $SHA1 >/dev/null\n> +                       git cat-file commit $SHA1\n>                 )\n>         '\n>\n> @@ -1149,15 +1149,15 @@ do\n>                         git commit --allow-empty -m foo &&\n>                         git commit --allow-empty -m bar &&\n>                         git commit --allow-empty -m xyz\n> -               )\n> +               ) &&\n>                 SHA1_1=$(git --git-dir=testrepo/.git rev-parse HEAD^^) &&\n>                 SHA1_2=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&\n>                 SHA1_3=$(git --git-dir=testrepo/.git rev-parse HEAD) &&\n>                 (\n>                         cd testrepo &&\n>                         git reset --hard $SHA1_2 &&\n> -                       git cat-file commit $SHA1_3 >/dev/null &&\n> -                       git cat-file commit $SHA1_3 >/dev/null\n> +                       git cat-file commit $SHA1_1 &&\n> +                       git cat-file commit $SHA1_3\n>                 ) &&\n>                 mk_empty shallow &&\n>                 (\n> @@ -1166,10 +1166,10 @@ do\n>                         test_must_fail git fetch ../testrepo/.git $SHA1_1 &&\n>                         git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&\n>                         git fetch ../testrepo/.git $SHA1_1 &&\n> -                       git cat-file commit $SHA1_1 >/dev/null &&\n> -                       test_must_fail git cat-file commit $SHA1_2 >/dev/null &&\n> +                       git cat-file commit $SHA1_1 &&\n> +                       test_must_fail git cat-file commit $SHA1_2 &&\n>                         git fetch ../testrepo/.git $SHA1_2 &&\n> -                       git cat-file commit $SHA1_2 >/dev/null &&\n> +                       git cat-file commit $SHA1_2 &&\n>                         test_must_fail git fetch ../testrepo/.git $SHA1_3\n>                 )\n>         '\n> --\n> 2.4.1-439-gcfa393f\n>\n"},{"id":"261973","messageId":"xmqqa8wwdshj.fsf@gitster.dls.corp.google.com","threadId":"39224","inReplyTo":"CABA5-zm4E71DV4hMw1Tifn-gqE4LzDDRzuo4vRL848bo_V5LLA@mail.gmail.com","subject":"Re: [PATCH v5 2/3] upload-pack: prepare to extend allow-tip-sha1-in-want","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-23T00:53:28Z","receivedAt":"2015-05-23T00:53:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Fredrik Medley <fredrik.medley@gmail.com> writes:\n\n>>> +#define ALLOW_TIP_SHA1       01\n>>> +static int allow_unadvertised_object_request;\n>>\n>> It is better to use \"unsigned int\" for these bit masks, as we are\n>> not interested in the top-most bit getting special-cased by using a\n>> signed type.  I'll amend this (and the one in upload-pack.c) while\n>> applying, so no need to resend only to correct these two, unless you\n>> have other reasons to reroll.\n>\n> Sounds like a good idea to change. Please amend it while applying.\n\nAlright, will do.\n\nThanks.\n"},{"id":"261974","messageId":"xmqq617kds4r.fsf@gitster.dls.corp.google.com","threadId":"39224","inReplyTo":"CABA5-zm3LqA+BAHj0EegoXmTuaNobhEHyD0v2kg=VfCzzqx-+A@mail.gmail.com","subject":"Re: [PATCH v5 3/3] upload-pack: optionally allow fetching reachable sha1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-23T01:01:08Z","receivedAt":"2015-05-23T01:01:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Fredrik Medley <fredrik.medley@gmail.com> writes:\n\n> 2015-05-22 0:15 GMT+02:00 Junio C Hamano <gitster@pobox.com>:\n>\n>> It looks like this new set of tests are well thought out; good job.\n>>\n>> I spotted a few minor nits, though.  All I'll amend while applying\n>> so there is no need to resend only to correct them.\n>\n> I agree on all your comments and your proposed amendment further down\n> looks good.\n\nThanks.\n\n> Should the test code contain the explanations you've written in this email?\n\nNo, I don't think so.\n\nI was just showing that thinking aloud while reviewing would be a\ngood way to do a review.  The practice\n\n (1) makes sure that the reviewer actually understood what the patch\n     wanted to do (and the reviewee can point out misunderstandings\n     if there are any); and\n\n (2) shows others that the reviewer actually read the patch ;-).\n\nThe latter is primarily meant for other people who review the\npatches.  I want to see people get in the habit of responding with\nsomething more than just a single-liner \"Reviewed-by:\", which I\noften have hard time guessing if the reviewer really read the patch,\nor just skimmed without spending effort to spot issues, and this\nmessage was my attempt to lead with an example.\n\nWill squash in the fix-up in the message you are responding to.\nLet's move the topic to 'next' after that.\n\nThanks.\n"}]}