{"thread":{"id":"35230","subject":"[PATCH v4] remote-curl: fix large pushes with GSSAPI","startedAt":"2013-10-29T02:36:37Z","lastAt":"2013-11-03T08:03:00Z","messageCount":17,"participants":["brian m. carlson","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":4,"patchTotal":null},"messages":[{"id":"229697","messageId":"1383014197-11607-1-git-send-email-sandals@crustytoothpaste.net","threadId":"35230","inReplyTo":null,"subject":"[PATCH v4] remote-curl: fix large pushes with GSSAPI","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2013-10-29T02:36:37Z","receivedAt":"2013-10-29T02:36:37Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Due to an interaction between the way libcurl handles GSSAPI authentication over\nHTTP and the way git uses libcurl, large pushes (those over http.postBuffer\nbytes) would fail due to an authentication failure requiring a rewind of the\ncurl buffer.  Such a rewind was not possible because the data did not fit into\nthe entire buffer.\n\nEnable the use of the Expect: 100-continue header for large requests where the\nserver offers GSSAPI authentication to avoid this issue, since the request would\notherwise fail.  This allows git to get the authentication data right before\nsending the pack contents.  Existing cases where pushes would succeed, including\nsmall requests using GSSAPI, still disable the use of 100 Continue, as it causes\nproblems for some remote HTTP implementations (servers and proxies).\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n remote-curl.c | 13 ++++++++++++-\n 1 file changed, 12 insertions(+), 1 deletion(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex c9b891a..2c276de 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -449,6 +449,7 @@ static int post_rpc(struct rpc_state *rpc)\n \tchar *gzip_body = NULL;\n \tsize_t gzip_size = 0;\n \tint err, large_request = 0;\n+\tint needs_100_continue = 0;\n \n \t/* Try to load the entire request, if we can fit it into the\n \t * allocated buffer space we can use HTTP/1.0 and avoid the\n@@ -472,6 +473,8 @@ static int post_rpc(struct rpc_state *rpc)\n \t}\n \n \tif (large_request) {\n+\t\tlong authtype = 0;\n+\n \t\tdo {\n \t\t\terr = probe_rpc(rpc);\n \t\t\tif (err == HTTP_REAUTH)\n@@ -479,11 +482,19 @@ static int post_rpc(struct rpc_state *rpc)\n \t\t} while (err == HTTP_REAUTH);\n \t\tif (err != HTTP_OK)\n \t\t\treturn -1;\n+\n+#if LIBCURL_VERSION_NUM >= 0x070a08\n+\t\tslot = get_active_slot();\n+\t\tcurl_easy_getinfo(slot->curl, CURLINFO_HTTPAUTH_AVAIL, &authtype);\n+\t\tif (authtype & CURLAUTH_GSSNEGOTIATE)\n+\t\t\tneeds_100_continue = 1;\n+#endif\n \t}\n \n \theaders = curl_slist_append(headers, rpc->hdr_content_type);\n \theaders = curl_slist_append(headers, rpc->hdr_accept);\n-\theaders = curl_slist_append(headers, \"Expect:\");\n+\theaders = curl_slist_append(headers, needs_100_continue ?\n+\t\t\"Expect: 100-continue\" : \"Expect:\");\n \n retry:\n \tslot = get_active_slot();\n-- \n1.8.4.1.635.g55556a5\n"},{"id":"229800","messageId":"20131030084510.GO11317@sigill.intra.peff.net","threadId":"35230","inReplyTo":"1383014197-11607-1-git-send-email-sandals@crustytoothpaste.net","subject":"Re: [PATCH v4] remote-curl: fix large pushes with GSSAPI","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-30T08:45:10Z","receivedAt":"2013-10-30T08:45:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 29, 2013 at 02:36:37AM +0000, brian m. carlson wrote:\n\n>  \tif (large_request) {\n> +\t\tlong authtype = 0;\n> +\n\nMinor nit, but this will produce an unused variable warning if the code\nin the #if below doesn't get compiled. I don't know how much we care.\n\n> +#if LIBCURL_VERSION_NUM >= 0x070a08\n> +\t\tslot = get_active_slot();\n> +\t\tcurl_easy_getinfo(slot->curl, CURLINFO_HTTPAUTH_AVAIL, &authtype);\n> +\t\tif (authtype & CURLAUTH_GSSNEGOTIATE)\n> +\t\t\tneeds_100_continue = 1;\n> +#endif\n\nI didn't notice this in the last round, but this is somewhat of an abuse\nof get_active_slot. It is intended to give you a new, pristine curl slot\nthat you can use to make a request, and it is yours until you call\nfinish_active_slot. You're not supposed to look at it after that.\n\nHowever, we do reuse the curl handles. And in the case of rpc case, we\nare only doing one request at a time, so the handle you get is\nguaranteed to be the last one used.  So it works in practice, but it\nwould break if the curl handle code breaks any of these assumptions.\n\nI think the clean way to do it would be to teach the slot code to pull\nout the available auth methods, and pass them up through the call chain.\nLike this on top of your patch:\n\ndiff --git a/http.c b/http.c\nindex 0ddb164..32fa998 100644\n--- a/http.c\n+++ b/http.c\n@@ -761,6 +761,12 @@ void finish_active_slot(struct active_request_slot *slot)\n \tif (slot->results != NULL) {\n \t\tslot->results->curl_result = slot->curl_result;\n \t\tslot->results->http_code = slot->http_code;\n+#if LIBCURL_VERSION_NUM >= 0x070a08\n+\t\tcurl_easy_getinfo(slot->curl, CURLINFO_HTTPAUTH_AVAIL,\n+\t\t\t\t  &slot->results->authtype);\n+#else\n+\t\tslot->results->authtype = 0;\n+#endif\n \t}\n \n \t/* Run callback if appropriate */\ndiff --git a/http.h b/http.h\nindex d77c1b5..4b32b9b 100644\n--- a/http.h\n+++ b/http.h\n@@ -54,6 +54,7 @@\n struct slot_results {\n \tCURLcode curl_result;\n \tlong http_code;\n+\tlong authtype;\n };\n \n struct active_request_slot {\ndiff --git a/remote-curl.c b/remote-curl.c\nindex eaa286c..d026f05 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -383,25 +383,29 @@ static size_t rpc_in(char *ptr, size_t eltsize,\n \treturn size;\n }\n \n-static int run_slot(struct active_request_slot *slot)\n+static int run_slot(struct active_request_slot *slot,\n+\t\t    struct slot_results *results)\n {\n \tint err;\n-\tstruct slot_results results;\n+\tstruct slot_results results_buf;\n \n-\tslot->results = &results;\n+\tif (!results)\n+\t\tresults = &results_buf;\n+\n+\tslot->results = results;\n \tslot->curl_result = curl_easy_perform(slot->curl);\n \tfinish_active_slot(slot);\n \n-\terr = handle_curl_result(&results);\n+\terr = handle_curl_result(results);\n \tif (err != HTTP_OK && err != HTTP_REAUTH) {\n \t\terror(\"RPC failed; result=%d, HTTP code = %ld\",\n-\t\t      results.curl_result, results.http_code);\n+\t\t      results->curl_result, results->http_code);\n \t}\n \n \treturn err;\n }\n \n-static int probe_rpc(struct rpc_state *rpc)\n+static int probe_rpc(struct rpc_state *rpc, struct slot_results *results)\n {\n \tstruct active_request_slot *slot;\n \tstruct curl_slist *headers = NULL;\n@@ -423,7 +427,7 @@ static int probe_rpc(struct rpc_state *rpc)\n \tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);\n \tcurl_easy_setopt(slot->curl, CURLOPT_FILE, &buf);\n \n-\terr = run_slot(slot);\n+\terr = run_slot(slot, results);\n \n \tcurl_slist_free_all(headers);\n \tstrbuf_release(&buf);\n@@ -462,20 +466,16 @@ static int post_rpc(struct rpc_state *rpc)\n \t}\n \n \tif (large_request) {\n-\t\tlong authtype = 0;\n+\t\tstruct slot_results results;\n \n \t\tdo {\n-\t\t\terr = probe_rpc(rpc);\n+\t\t\terr = probe_rpc(rpc, &results);\n \t\t} while (err == HTTP_REAUTH);\n \t\tif (err != HTTP_OK)\n \t\t\treturn -1;\n \n-#if LIBCURL_VERSION_NUM >= 0x070a08\n-\t\tslot = get_active_slot();\n-\t\tcurl_easy_getinfo(slot->curl, CURLINFO_HTTPAUTH_AVAIL, &authtype);\n-\t\tif (authtype & CURLAUTH_GSSNEGOTIATE)\n+\t\tif (results.authtype & CURLAUTH_GSSNEGOTIATE)\n \t\t\tneeds_100_continue = 1;\n-#endif\n \t}\n \n \theaders = curl_slist_append(headers, rpc->hdr_content_type);\n@@ -572,7 +572,7 @@ retry:\n \tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, rpc_in);\n \tcurl_easy_setopt(slot->curl, CURLOPT_FILE, rpc);\n \n-\terr = run_slot(slot);\n+\terr = run_slot(slot, NULL);\n \tif (err == HTTP_REAUTH && !large_request)\n \t\tgoto retry;\n \tif (err != HTTP_OK)\n\nThat's note tested beyond compiling, but I think it should work. Feel\nfree to squash it into your patch, or if you'd like, I can split out the\nrefactoring steps with a commit message for you.\n\n-Peff\n"},{"id":"229869","messageId":"20131030224030.GC12595@vauxhall.crustytoothpaste.net","threadId":"35230","inReplyTo":"20131030084510.GO11317@sigill.intra.peff.net","subject":"Re: [PATCH v4] remote-curl: fix large pushes with GSSAPI","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2013-10-30T22:40:30Z","receivedAt":"2013-10-30T22:40:30Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Wed, Oct 30, 2013 at 04:45:10AM -0400, Jeff King wrote:\n> However, we do reuse the curl handles. And in the case of rpc case, we\n> are only doing one request at a time, so the handle you get is\n> guaranteed to be the last one used.  So it works in practice, but it\n> would break if the curl handle code breaks any of these assumptions.\n> \n> I think the clean way to do it would be to teach the slot code to pull\n> out the available auth methods, and pass them up through the call chain.\n> Like this on top of your patch:\n> \n> diff --git a/http.c b/http.c\n> index 0ddb164..32fa998 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -761,6 +761,12 @@ void finish_active_slot(struct active_request_slot *slot)\n>  \tif (slot->results != NULL) {\n>  \t\tslot->results->curl_result = slot->curl_result;\n>  \t\tslot->results->http_code = slot->http_code;\n> +#if LIBCURL_VERSION_NUM >= 0x070a08\n> +\t\tcurl_easy_getinfo(slot->curl, CURLINFO_HTTPAUTH_AVAIL,\n> +\t\t\t\t  &slot->results->authtype);\n> +#else\n> +\t\tslot->results->authtype = 0;\n> +#endif\n>  \t}\n>  \n>  \t/* Run callback if appropriate */\n> diff --git a/http.h b/http.h\n> index d77c1b5..4b32b9b 100644\n> --- a/http.h\n> +++ b/http.h\n> @@ -54,6 +54,7 @@\n>  struct slot_results {\n>  \tCURLcode curl_result;\n>  \tlong http_code;\n> +\tlong authtype;\n>  };\n>  \n>  struct active_request_slot {\n> diff --git a/remote-curl.c b/remote-curl.c\n> index eaa286c..d026f05 100644\n> --- a/remote-curl.c\n> +++ b/remote-curl.c\n> @@ -383,25 +383,29 @@ static size_t rpc_in(char *ptr, size_t eltsize,\n>  \treturn size;\n>  }\n>  \n> -static int run_slot(struct active_request_slot *slot)\n> +static int run_slot(struct active_request_slot *slot,\n> +\t\t    struct slot_results *results)\n>  {\n>  \tint err;\n> -\tstruct slot_results results;\n> +\tstruct slot_results results_buf;\n>  \n> -\tslot->results = &results;\n> +\tif (!results)\n> +\t\tresults = &results_buf;\n> +\n> +\tslot->results = results;\n>  \tslot->curl_result = curl_easy_perform(slot->curl);\n>  \tfinish_active_slot(slot);\n>  \n> -\terr = handle_curl_result(&results);\n> +\terr = handle_curl_result(results);\n>  \tif (err != HTTP_OK && err != HTTP_REAUTH) {\n>  \t\terror(\"RPC failed; result=%d, HTTP code = %ld\",\n> -\t\t      results.curl_result, results.http_code);\n> +\t\t      results->curl_result, results->http_code);\n>  \t}\n>  \n>  \treturn err;\n>  }\n>  \n> -static int probe_rpc(struct rpc_state *rpc)\n> +static int probe_rpc(struct rpc_state *rpc, struct slot_results *results)\n>  {\n>  \tstruct active_request_slot *slot;\n>  \tstruct curl_slist *headers = NULL;\n> @@ -423,7 +427,7 @@ static int probe_rpc(struct rpc_state *rpc)\n>  \tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);\n>  \tcurl_easy_setopt(slot->curl, CURLOPT_FILE, &buf);\n>  \n> -\terr = run_slot(slot);\n> +\terr = run_slot(slot, results);\n>  \n>  \tcurl_slist_free_all(headers);\n>  \tstrbuf_release(&buf);\n> @@ -462,20 +466,16 @@ static int post_rpc(struct rpc_state *rpc)\n>  \t}\n>  \n>  \tif (large_request) {\n> -\t\tlong authtype = 0;\n> +\t\tstruct slot_results results;\n>  \n>  \t\tdo {\n> -\t\t\terr = probe_rpc(rpc);\n> +\t\t\terr = probe_rpc(rpc, &results);\n>  \t\t} while (err == HTTP_REAUTH);\n>  \t\tif (err != HTTP_OK)\n>  \t\t\treturn -1;\n>  \n> -#if LIBCURL_VERSION_NUM >= 0x070a08\n> -\t\tslot = get_active_slot();\n> -\t\tcurl_easy_getinfo(slot->curl, CURLINFO_HTTPAUTH_AVAIL, &authtype);\n> -\t\tif (authtype & CURLAUTH_GSSNEGOTIATE)\n> +\t\tif (results.authtype & CURLAUTH_GSSNEGOTIATE)\n>  \t\t\tneeds_100_continue = 1;\n> -#endif\n>  \t}\n>  \n>  \theaders = curl_slist_append(headers, rpc->hdr_content_type);\n> @@ -572,7 +572,7 @@ retry:\n>  \tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, rpc_in);\n>  \tcurl_easy_setopt(slot->curl, CURLOPT_FILE, rpc);\n>  \n> -\terr = run_slot(slot);\n> +\terr = run_slot(slot, NULL);\n>  \tif (err == HTTP_REAUTH && !large_request)\n>  \t\tgoto retry;\n>  \tif (err != HTTP_OK)\n> \n> That's note tested beyond compiling, but I think it should work. Feel\n> free to squash it into your patch, or if you'd like, I can split out the\n> refactoring steps with a commit message for you.\n\nIf you would split it out, that would be great.  Then I'll simply rebase\nmy patch on top of yours and go from there.\n\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"},{"id":"229873","messageId":"20131031063451.GA5513@sigill.intra.peff.net","threadId":"35230","inReplyTo":"20131030224030.GC12595@vauxhall.crustytoothpaste.net","subject":"Re: [PATCH v4] remote-curl: fix large pushes with GSSAPI","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-31T06:34:51Z","receivedAt":"2013-10-31T06:34:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 30, 2013 at 10:40:30PM +0000, brian m. carlson wrote:\n\n> If you would split it out, that would be great.  Then I'll simply rebase\n> my patch on top of yours and go from there.\n\nI just included your patch on top, since it was the residue left over\nafter committing my refactoring. Please read over the result to make\nsure I am not defaming you. :)\n\nI noticed while committing the first patch that we do not actually\nfollow the \"do not look at curl after finish_active_slot\" rule for the\ncontent-type. Again, we get away with it because we are not running\nmultiple slots at the time (we only check content-type during the\ninitial discovery).\n\nI think the refactoring here is the cleanest thing by the existing\nrules, but I also think we could get away with the somewhat simpler\npatch of just teaching probe_rpc to grab the AUTHAVAIL (because it still\nhas the old slot and does not need to call get_active_slot again, and\nbecause we know we are only using a single slot).\n\nGoing through all of this, I can't help but be annoyed at how much http\nbaggage we are carrying around for the curl_multi code for parallel\nfetches, which is only used for dumb http. The smart-http code would be\nhappy with a single curl handle we used each time. But I imagine there\nare still people relying on dumb http, and dropping the parallel fetch\nwould be a pretty severe regression for them.\n\n  [1/3]: http: return curl's AUTHAVAIL via slot_results\n  [2/3]: remote-curl: pass curl slot_results back through run_slot\n  [3/3]: remote-curl: fix large pushes with GSSAPI\n\n-Peff\n"},{"id":"229874","messageId":"20131031063530.GA5812@sigill.intra.peff.net","threadId":"35230","inReplyTo":"20131031063451.GA5513@sigill.intra.peff.net","subject":"[PATCH 1/3] http: return curl's AUTHAVAIL via slot_results","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-31T06:35:31Z","receivedAt":"2013-10-31T06:35:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Callers of the http code may want to know which auth types\nwere available for the previous request. But after finishing\nwith the curl slot, they are not supposed to look at the\ncurl handle again. We already handle returning other\ninformation via the slot_results struct; let's add a flag to\ncheck the available auth.\n\nNote that older versions of curl did not support this, so we\nsimply return 0 (something like \"-1\" would be worse, as the\nvalue is a bitflag and we might accidentally set a flag).\nThis is sufficient for the callers planned in this series,\nwho only trigger some optional behavior if particular bits\nare set, and can live with a fake \"no bits\" answer.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n http.c | 6 ++++++\n http.h | 1 +\n 2 files changed, 7 insertions(+)\n\ndiff --git a/http.c b/http.c\nindex 0ddb164..5c51865 100644\n--- a/http.c\n+++ b/http.c\n@@ -761,6 +761,12 @@ void finish_active_slot(struct active_request_slot *slot)\n \tif (slot->results != NULL) {\n \t\tslot->results->curl_result = slot->curl_result;\n \t\tslot->results->http_code = slot->http_code;\n+#if LIBCURL_VERSION_NUM >= 0x070a08\n+\t\tcurl_easy_getinfo(slot->curl, CURLINFO_HTTPAUTH_AVAIL,\n+\t\t\t\t  &slot->results->auth_avail);\n+#else\n+\t\tslot->results->auth_avail = 0;\n+#endif\n \t}\n \n \t/* Run callback if appropriate */\ndiff --git a/http.h b/http.h\nindex d77c1b5..81d4843 100644\n--- a/http.h\n+++ b/http.h\n@@ -54,6 +54,7 @@\n struct slot_results {\n \tCURLcode curl_result;\n \tlong http_code;\n+\tlong auth_avail;\n };\n \n struct active_request_slot {\n-- \n1.8.4.1.898.g8bf8a41.dirty\n"},{"id":"229875","messageId":"20131031063626.GB5812@sigill.intra.peff.net","threadId":"35230","inReplyTo":"20131031063451.GA5513@sigill.intra.peff.net","subject":"[PATCH 2/3] remote-curl: pass curl slot_results back through run_slot","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-31T06:36:26Z","receivedAt":"2013-10-31T06:36:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Some callers may want to know more than just the integer\nerror code we return. Let them optionally pass a\nslot_results struct to fill in (or NULL if they do not\ncare). In either case we continue to return the integer\ncode.\n\nWe can also give probe_rpc the same treatment (since it\nbuilds directly on run_slot).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n remote-curl.c | 22 +++++++++++++---------\n 1 file changed, 13 insertions(+), 9 deletions(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex b5ebe01..79db21e 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -383,25 +383,29 @@ static size_t rpc_in(char *ptr, size_t eltsize,\n \treturn size;\n }\n \n-static int run_slot(struct active_request_slot *slot)\n+static int run_slot(struct active_request_slot *slot,\n+\t\t    struct slot_results *results)\n {\n \tint err;\n-\tstruct slot_results results;\n+\tstruct slot_results results_buf;\n \n-\tslot->results = &results;\n+\tif (!results)\n+\t\tresults = &results_buf;\n+\n+\tslot->results = results;\n \tslot->curl_result = curl_easy_perform(slot->curl);\n \tfinish_active_slot(slot);\n \n-\terr = handle_curl_result(&results);\n+\terr = handle_curl_result(results);\n \tif (err != HTTP_OK && err != HTTP_REAUTH) {\n \t\terror(\"RPC failed; result=%d, HTTP code = %ld\",\n-\t\t      results.curl_result, results.http_code);\n+\t\t      results->curl_result, results->http_code);\n \t}\n \n \treturn err;\n }\n \n-static int probe_rpc(struct rpc_state *rpc)\n+static int probe_rpc(struct rpc_state *rpc, struct slot_results *results)\n {\n \tstruct active_request_slot *slot;\n \tstruct curl_slist *headers = NULL;\n@@ -423,7 +427,7 @@ static int probe_rpc(struct rpc_state *rpc)\n \tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);\n \tcurl_easy_setopt(slot->curl, CURLOPT_FILE, &buf);\n \n-\terr = run_slot(slot);\n+\terr = run_slot(slot, results);\n \n \tcurl_slist_free_all(headers);\n \tstrbuf_release(&buf);\n@@ -462,7 +466,7 @@ static int post_rpc(struct rpc_state *rpc)\n \n \tif (large_request) {\n \t\tdo {\n-\t\t\terr = probe_rpc(rpc);\n+\t\t\terr = probe_rpc(rpc, NULL);\n \t\t} while (err == HTTP_REAUTH);\n \t\tif (err != HTTP_OK)\n \t\t\treturn -1;\n@@ -561,7 +565,7 @@ retry:\n \tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, rpc_in);\n \tcurl_easy_setopt(slot->curl, CURLOPT_FILE, rpc);\n \n-\terr = run_slot(slot);\n+\terr = run_slot(slot, NULL);\n \tif (err == HTTP_REAUTH && !large_request)\n \t\tgoto retry;\n \tif (err != HTTP_OK)\n-- \n1.8.4.1.898.g8bf8a41.dirty\n"},{"id":"229876","messageId":"20131031063651.GC5812@sigill.intra.peff.net","threadId":"35230","inReplyTo":"20131031063451.GA5513@sigill.intra.peff.net","subject":"[PATCH 3/3] remote-curl: fix large pushes with GSSAPI","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-31T06:36:51Z","receivedAt":"2013-10-31T06:36:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"From: brian m. carlson <sandals@crustytoothpaste.net>\n\nDue to an interaction between the way libcurl handles GSSAPI\nauthentication over HTTP and the way git uses libcurl, large\npushes (those over http.postBuffer bytes) would fail due to\nan authentication failure requiring a rewind of the curl\nbuffer.  Such a rewind was not possible because the data did\nnot fit into the entire buffer.\n\nEnable the use of the Expect: 100-continue header for large\nrequests where the server offers GSSAPI authentication to\navoid this issue, since the request would otherwise fail.\nThis allows git to get the authentication data right before\nsending the pack contents.  Existing cases where pushes\nwould succeed, including small requests using GSSAPI, still\ndisable the use of 100 Continue, as it causes problems for\nsome remote HTTP implementations (servers and proxies).\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\nSigned-off-by: Jeff King <peff@peff.net>\n---\n remote-curl.c | 11 +++++++++--\n 1 file changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 79db21e..f646b5f 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -442,6 +442,7 @@ static int post_rpc(struct rpc_state *rpc)\n \tchar *gzip_body = NULL;\n \tsize_t gzip_size = 0;\n \tint err, large_request = 0;\n+\tint needs_100_continue = 0;\n \n \t/* Try to load the entire request, if we can fit it into the\n \t * allocated buffer space we can use HTTP/1.0 and avoid the\n@@ -465,16 +466,22 @@ static int post_rpc(struct rpc_state *rpc)\n \t}\n \n \tif (large_request) {\n+\t\tstruct slot_results results;\n+\n \t\tdo {\n-\t\t\terr = probe_rpc(rpc, NULL);\n+\t\t\terr = probe_rpc(rpc, &results);\n \t\t} while (err == HTTP_REAUTH);\n \t\tif (err != HTTP_OK)\n \t\t\treturn -1;\n+\n+\t\tif (results.auth_avail & CURLAUTH_GSSNEGOTIATE)\n+\t\t\tneeds_100_continue = 1;\n \t}\n \n \theaders = curl_slist_append(headers, rpc->hdr_content_type);\n \theaders = curl_slist_append(headers, rpc->hdr_accept);\n-\theaders = curl_slist_append(headers, \"Expect:\");\n+\theaders = curl_slist_append(headers, needs_100_continue ?\n+\t\t\"Expect: 100-continue\" : \"Expect:\");\n \n retry:\n \tslot = get_active_slot();\n-- \n1.8.4.1.898.g8bf8a41.dirty\n"},{"id":"229922","messageId":"xmqqeh719xgq.fsf_-_@gitster.dls.corp.google.com","threadId":"35230","inReplyTo":"20131031063626.GB5812@sigill.intra.peff.net","subject":"[BUG?] \"error: cache entry has null sha1\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-31T17:11:49Z","receivedAt":"2013-10-31T17:11:49Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This is totally unrelated to this thread, but I am seeing a strange\nerror from the \"let's make sure we won't write null sha1\" code we\nadded recently.  This reproduces reliably for me, even in a freshly\ncloned repository without previous rerere records:\n\n - Check out v1.8.4.1\n\n   $ git checkout v1.8.4.1^0\n\n   at this point your HEAD should be detached.\n\n - Apply the first patch from this series\n\n   $ git am -3 20131031063530.GA5812@sigill.intra.peff.net\n\n - Apply the second patch from this series\n\n   $ git am -3 20131031063626.GB5812@sigill.intra.peff.net\n\n   Up to this point things should advance smoothly\n\n - Attempt to apply the second patch _again_.\n\n   $ git am -3 20131031063626.GB5812@sigill.intra.peff.net\n\n   This should detect a no-op, but somehow leaves things in a\n   conflicted state.  I haven't looked into what is wrong, but this\n   message is not about this failure.\n\n - Try to discard\n\n   $ git am --abort\n   error: cache entry has null sha1: remote-curl.c\n   fatal: unable to write new index file\n\n   This should not happen, no?\n\n\"git reset --hard\" will remove the funnies, but still...\n"},{"id":"229923","messageId":"20131031171539.GA19142@sigill.intra.peff.net","threadId":"35230","inReplyTo":"xmqqeh719xgq.fsf_-_@gitster.dls.corp.google.com","subject":"Re: [BUG?] \"error: cache entry has null sha1\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-31T17:15:40Z","receivedAt":"2013-10-31T17:15:40Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 31, 2013 at 10:11:49AM -0700, Junio C Hamano wrote:\n\n>  - Try to discard\n> \n>    $ git am --abort\n>    error: cache entry has null sha1: remote-curl.c\n>    fatal: unable to write new index file\n> \n>    This should not happen, no?\n> \n> \"git reset --hard\" will remove the funnies, but still...\n\nI ran into this recently, too. Isn't it just the twoway_merge bug we\nwere discussing here:\n\n  http://thread.gmane.org/gmane.comp.version-control.git/202440/focus=212316\n\nI don't think we ever actually applied a fix.\n\n-Peff\n"},{"id":"229925","messageId":"20131031172116.GA19428@sigill.intra.peff.net","threadId":"35230","inReplyTo":"20131031171539.GA19142@sigill.intra.peff.net","subject":"Re: [BUG?] \"error: cache entry has null sha1\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-10-31T17:21:16Z","receivedAt":"2013-10-31T17:21:16Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 31, 2013 at 01:15:39PM -0400, Jeff King wrote:\n\n> On Thu, Oct 31, 2013 at 10:11:49AM -0700, Junio C Hamano wrote:\n> \n> >  - Try to discard\n> > \n> >    $ git am --abort\n> >    error: cache entry has null sha1: remote-curl.c\n> >    fatal: unable to write new index file\n> > \n> >    This should not happen, no?\n> > \n> > \"git reset --hard\" will remove the funnies, but still...\n> \n> I ran into this recently, too. Isn't it just the twoway_merge bug we\n> were discussing here:\n> \n>   http://thread.gmane.org/gmane.comp.version-control.git/202440/focus=212316\n> \n> I don't think we ever actually applied a fix.\n\nAlso, the followup:\n\n  http://thread.gmane.org/gmane.comp.version-control.git/217172\n\n-Peff\n"},{"id":"229931","messageId":"xmqq8ux98gau.fsf@gitster.dls.corp.google.com","threadId":"35230","inReplyTo":"20131031172116.GA19428@sigill.intra.peff.net","subject":"Re: [BUG?] \"error: cache entry has null sha1\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-31T18:07:53Z","receivedAt":"2013-10-31T18:07:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks for the pointers.\n"},{"id":"229983","messageId":"20131031224913.GB41460@vauxhall.crustytoothpaste.net","threadId":"35230","inReplyTo":"20131031063651.GC5812@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] remote-curl: fix large pushes with GSSAPI","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2013-10-31T22:49:15Z","receivedAt":"2013-10-31T22:49:15Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Thu, Oct 31, 2013 at 02:36:51AM -0400, Jeff King wrote:\n> From: brian m. carlson <sandals@crustytoothpaste.net>\n> \n> Due to an interaction between the way libcurl handles GSSAPI\n> authentication over HTTP and the way git uses libcurl, large\n> pushes (those over http.postBuffer bytes) would fail due to\n> an authentication failure requiring a rewind of the curl\n> buffer.  Such a rewind was not possible because the data did\n> not fit into the entire buffer.\n> \n> Enable the use of the Expect: 100-continue header for large\n> requests where the server offers GSSAPI authentication to\n> avoid this issue, since the request would otherwise fail.\n> This allows git to get the authentication data right before\n> sending the pack contents.  Existing cases where pushes\n> would succeed, including small requests using GSSAPI, still\n> disable the use of 100 Continue, as it causes problems for\n> some remote HTTP implementations (servers and proxies).\n> \n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> Signed-off-by: Jeff King <peff@peff.net>\n\nThe entire series looks fine by me.  Thanks for fixing this up.\n\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"},{"id":"230020","messageId":"1383345895-23341-1-git-send-email-gitster@pobox.com","threadId":"35230","inReplyTo":"20131031172116.GA19428@sigill.intra.peff.net","subject":"Re* [BUG?] \"error: cache entry has null sha1\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-01T22:44:52Z","receivedAt":"2013-11-01T22:44:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Oct 31, 2013 at 01:15:39PM -0400, Jeff King wrote:\n>\n>> On Thu, Oct 31, 2013 at 10:11:49AM -0700, Junio C Hamano wrote:\n>> \n>> >  - Try to discard\n>> > \n>> >    $ git am --abort\n>> >    error: cache entry has null sha1: remote-curl.c\n>> >    fatal: unable to write new index file\n>> > \n>> >    This should not happen, no?\n>> > \n>> > \"git reset --hard\" will remove the funnies, but still...\n>> \n>> I ran into this recently, too. Isn't it just the twoway_merge bug we\n>> were discussing here:\n>> \n>>   http://thread.gmane.org/gmane.comp.version-control.git/202440/focus=212316\n>> \n>> I don't think we ever actually applied a fix.\n>\n> Also, the followup:\n>\n>   http://thread.gmane.org/gmane.comp.version-control.git/217172\n\nOK, so it appears that we sort of agreed that the pieces of patches\nin the thread was good at $gmane/217300, but somehow I ended up\nveering into a tangent from there and forgot about the topic X-<?\n\nHere is a proposed endgame for the topic in a patch form, then.\n\nI've added a test for low-level \"read-tree --reset -u A B\", and\ntried the \"am --abort\" I saw the problem with manually, but other\nthan that, I haven't (re)thought about the issue hard enough to be\ncomfortable with this change yet.\n\nTo be applied on top of c479d14, but the result should be mergeable\ncleanly all the way up to 'pu'.\n\nJeff King (1):\n  unpack-trees: fix \"read-tree -u --reset A B\" with conflicted index\n\nJunio C Hamano (2):\n  t1005: reindent\n  t1005: add test for \"read-tree --reset -u A B\"\n\n t/lib-read-tree.sh         |  52 ++++++++---------\n t/t1005-read-tree-reset.sh | 141 +++++++++++++++++++++++++--------------------\n unpack-trees.c             |  25 +++++---\n 3 files changed, 121 insertions(+), 97 deletions(-)\n\n-- \n1.8.5-rc0-281-g8951339\n"},{"id":"230018","messageId":"1383345895-23341-2-git-send-email-gitster@pobox.com","threadId":"35230","inReplyTo":"1383345895-23341-1-git-send-email-gitster@pobox.com","subject":"[PATCH 1/3] unpack-trees: fix \"read-tree -u --reset A B\" with conflicted index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-01T22:44:53Z","receivedAt":"2013-11-01T22:44:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Jeff King <peff@peff.net>\n\nWhen we call \"read-tree --reset -u HEAD ORIG_HEAD\", the first thing we\ndo with the index is to call read_cache_unmerged.  Originally that\nwould read the index, leaving aside any unmerged entries.  However, as\nof d1a43f2 (reset --hard/read-tree --reset -u: remove unmerged new\npaths, 2008-10-15), it actually creates a new cache entry to serve as\na placeholder, so that we later know to update the working tree.\n\nHowever, we later noticed that the sha1 of that unmerged entry was\njust copied from some higher stage, leaving you with random content in\nthe index.  That was fixed by e11d7b5 (\"reset --merge\": fix unmerged\ncase, 2009-12-31), which instead puts the null sha1 into the newly\ncreated entry, and sets a CE_CONFLICTED flag. At the same time, it\nteaches the unpack-trees machinery to pay attention to this flag, so\nthat oneway_merge throws away the current value.\n\nHowever, it did not update the code paths for twoway_merge, which is\nwhere we end up in the two-way read-tree with --reset. We notice that\nthe HEAD and ORIG_HEAD versions are the same, and say \"oh, we can just\nreuse the current version\". But that's not true. The current version\nis bogus.\n\nNotice this case and make sure we do not keep the bogus entry; either\nwe do not have that path in the tree we are moving to (i.e. remove\nit), or we want to have the cache entry we created for the tree we are\nmoving to (i.e. resolve by explicitly saying the \"newtree\" version is\nwhat we want).\n\n[jc: this is from the almost year-old $gmane/212316; the credit goes\nto Peff, but we need his sign-off]\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n unpack-trees.c | 25 +++++++++++++++++--------\n 1 file changed, 17 insertions(+), 8 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 7c9ecf6..bf978e1 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1728,14 +1728,23 @@ int twoway_merge(struct cache_entry **src, struct unpack_trees_options *o)\n \t\tnewtree = NULL;\n \n \tif (current) {\n-\t\tif ((!oldtree && !newtree) || /* 4 and 5 */\n-\t\t    (!oldtree && newtree &&\n-\t\t     same(current, newtree)) || /* 6 and 7 */\n-\t\t    (oldtree && newtree &&\n-\t\t     same(oldtree, newtree)) || /* 14 and 15 */\n-\t\t    (oldtree && newtree &&\n-\t\t     !same(oldtree, newtree) && /* 18 and 19 */\n-\t\t     same(current, newtree))) {\n+\t\tif (current->ce_flags & CE_CONFLICTED) {\n+\t\t\tif (same(oldtree, newtree) || o->reset) {\n+\t\t\t\tif (!newtree)\n+\t\t\t\t\treturn deleted_entry(current, current, o);\n+\t\t\t\telse\n+\t\t\t\t\treturn merged_entry(newtree, current, o);\n+\t\t\t}\n+\t\t\treturn o->gently ? -1 : reject_merge(current, o);\n+\t\t}\n+\t\telse if ((!oldtree && !newtree) || /* 4 and 5 */\n+\t\t\t (!oldtree && newtree &&\n+\t\t\t  same(current, newtree)) || /* 6 and 7 */\n+\t\t\t (oldtree && newtree &&\n+\t\t\t  same(oldtree, newtree)) || /* 14 and 15 */\n+\t\t\t (oldtree && newtree &&\n+\t\t\t  !same(oldtree, newtree) && /* 18 and 19 */\n+\t\t\t  same(current, newtree))) {\n \t\t\treturn keep_entry(current, o);\n \t\t}\n \t\telse if (oldtree && !newtree && same(current, oldtree)) {\n-- \n1.8.5-rc0-281-g8951339\n"},{"id":"230019","messageId":"1383345895-23341-3-git-send-email-gitster@pobox.com","threadId":"35230","inReplyTo":"1383345895-23341-1-git-send-email-gitster@pobox.com","subject":"[PATCH 2/3] t1005: reindent","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-01T22:44:54Z","receivedAt":"2013-11-01T22:44:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Just to update the style of this ancient test script to match\nour house style.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/lib-read-tree.sh         |  52 +++++++++----------\n t/t1005-read-tree-reset.sh | 126 ++++++++++++++++++++++-----------------------\n 2 files changed, 89 insertions(+), 89 deletions(-)\n\ndiff --git a/t/lib-read-tree.sh b/t/lib-read-tree.sh\nindex abc2c6f..ef079af 100644\n--- a/t/lib-read-tree.sh\n+++ b/t/lib-read-tree.sh\n@@ -5,39 +5,39 @@\n # write the index and that together with -u it doesn't touch the work tree.\n #\n read_tree_must_succeed () {\n-    git ls-files -s >pre-dry-run &&\n-    git read-tree -n \"$@\" &&\n-    git ls-files -s >post-dry-run &&\n-    test_cmp pre-dry-run post-dry-run &&\n-    git read-tree \"$@\"\n+\tgit ls-files -s >pre-dry-run &&\n+\tgit read-tree -n \"$@\" &&\n+\tgit ls-files -s >post-dry-run &&\n+\ttest_cmp pre-dry-run post-dry-run &&\n+\tgit read-tree \"$@\"\n }\n \n read_tree_must_fail () {\n-    git ls-files -s >pre-dry-run &&\n-    test_must_fail git read-tree -n \"$@\" &&\n-    git ls-files -s >post-dry-run &&\n-    test_cmp pre-dry-run post-dry-run &&\n-    test_must_fail git read-tree \"$@\"\n+\tgit ls-files -s >pre-dry-run &&\n+\ttest_must_fail git read-tree -n \"$@\" &&\n+\tgit ls-files -s >post-dry-run &&\n+\ttest_cmp pre-dry-run post-dry-run &&\n+\ttest_must_fail git read-tree \"$@\"\n }\n \n read_tree_u_must_succeed () {\n-    git ls-files -s >pre-dry-run &&\n-    git diff-files -p >pre-dry-run-wt &&\n-    git read-tree -n \"$@\" &&\n-    git ls-files -s >post-dry-run &&\n-    git diff-files -p >post-dry-run-wt &&\n-    test_cmp pre-dry-run post-dry-run &&\n-    test_cmp pre-dry-run-wt post-dry-run-wt &&\n-    git read-tree \"$@\"\n+\tgit ls-files -s >pre-dry-run &&\n+\tgit diff-files -p >pre-dry-run-wt &&\n+\tgit read-tree -n \"$@\" &&\n+\tgit ls-files -s >post-dry-run &&\n+\tgit diff-files -p >post-dry-run-wt &&\n+\ttest_cmp pre-dry-run post-dry-run &&\n+\ttest_cmp pre-dry-run-wt post-dry-run-wt &&\n+\tgit read-tree \"$@\"\n }\n \n read_tree_u_must_fail () {\n-    git ls-files -s >pre-dry-run &&\n-    git diff-files -p >pre-dry-run-wt &&\n-    test_must_fail git read-tree -n \"$@\" &&\n-    git ls-files -s >post-dry-run &&\n-    git diff-files -p >post-dry-run-wt &&\n-    test_cmp pre-dry-run post-dry-run &&\n-    test_cmp pre-dry-run-wt post-dry-run-wt &&\n-    test_must_fail git read-tree \"$@\"\n+\tgit ls-files -s >pre-dry-run &&\n+\tgit diff-files -p >pre-dry-run-wt &&\n+\ttest_must_fail git read-tree -n \"$@\" &&\n+\tgit ls-files -s >post-dry-run &&\n+\tgit diff-files -p >post-dry-run-wt &&\n+\ttest_cmp pre-dry-run post-dry-run &&\n+\ttest_cmp pre-dry-run-wt post-dry-run-wt &&\n+\ttest_must_fail git read-tree \"$@\"\n }\ndiff --git a/t/t1005-read-tree-reset.sh b/t/t1005-read-tree-reset.sh\nindex f53de79..e29cf63 100755\n--- a/t/t1005-read-tree-reset.sh\n+++ b/t/t1005-read-tree-reset.sh\n@@ -8,84 +8,84 @@ test_description='read-tree -u --reset'\n # two-tree test\n \n test_expect_success 'setup' '\n-  git init &&\n-  mkdir df &&\n-  echo content >df/file &&\n-  git add df/file &&\n-  git commit -m one &&\n-  git ls-files >expect &&\n-  rm -rf df &&\n-  echo content >df &&\n-  git add df &&\n-  echo content >new &&\n-  git add new &&\n-  git commit -m two\n+\tgit init &&\n+\tmkdir df &&\n+\techo content >df/file &&\n+\tgit add df/file &&\n+\tgit commit -m one &&\n+\tgit ls-files >expect &&\n+\trm -rf df &&\n+\techo content >df &&\n+\tgit add df &&\n+\techo content >new &&\n+\tgit add new &&\n+\tgit commit -m two\n '\n \n test_expect_success 'reset should work' '\n-  read_tree_u_must_succeed -u --reset HEAD^ &&\n-  git ls-files >actual &&\n-  test_cmp expect actual\n+\tread_tree_u_must_succeed -u --reset HEAD^ &&\n+\tgit ls-files >actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'reset should remove remnants from a failed merge' '\n-  read_tree_u_must_succeed --reset -u HEAD &&\n-  git ls-files -s >expect &&\n-  sha1=$(git rev-parse :new) &&\n-  (\n-\techo \"100644 $sha1 1\told\"\n-\techo \"100644 $sha1 3\told\"\n-  ) | git update-index --index-info &&\n-  >old &&\n-  git ls-files -s &&\n-  read_tree_u_must_succeed --reset -u HEAD &&\n-  git ls-files -s >actual &&\n-  ! test -f old\n+\tread_tree_u_must_succeed --reset -u HEAD &&\n+\tgit ls-files -s >expect &&\n+\tsha1=$(git rev-parse :new) &&\n+\t(\n+\t\techo \"100644 $sha1 1\told\"\n+\t\techo \"100644 $sha1 3\told\"\n+\t) | git update-index --index-info &&\n+\t>old &&\n+\tgit ls-files -s &&\n+\tread_tree_u_must_succeed --reset -u HEAD &&\n+\tgit ls-files -s >actual &&\n+\t! test -f old\n '\n \n test_expect_success 'Porcelain reset should remove remnants too' '\n-  read_tree_u_must_succeed --reset -u HEAD &&\n-  git ls-files -s >expect &&\n-  sha1=$(git rev-parse :new) &&\n-  (\n-\techo \"100644 $sha1 1\told\"\n-\techo \"100644 $sha1 3\told\"\n-  ) | git update-index --index-info &&\n-  >old &&\n-  git ls-files -s &&\n-  git reset --hard &&\n-  git ls-files -s >actual &&\n-  ! test -f old\n+\tread_tree_u_must_succeed --reset -u HEAD &&\n+\tgit ls-files -s >expect &&\n+\tsha1=$(git rev-parse :new) &&\n+\t(\n+\t\techo \"100644 $sha1 1\told\"\n+\t\techo \"100644 $sha1 3\told\"\n+\t) | git update-index --index-info &&\n+\t>old &&\n+\tgit ls-files -s &&\n+\tgit reset --hard &&\n+\tgit ls-files -s >actual &&\n+\t! test -f old\n '\n \n test_expect_success 'Porcelain checkout -f should remove remnants too' '\n-  read_tree_u_must_succeed --reset -u HEAD &&\n-  git ls-files -s >expect &&\n-  sha1=$(git rev-parse :new) &&\n-  (\n-\techo \"100644 $sha1 1\told\"\n-\techo \"100644 $sha1 3\told\"\n-  ) | git update-index --index-info &&\n-  >old &&\n-  git ls-files -s &&\n-  git checkout -f &&\n-  git ls-files -s >actual &&\n-  ! test -f old\n+\tread_tree_u_must_succeed --reset -u HEAD &&\n+\tgit ls-files -s >expect &&\n+\tsha1=$(git rev-parse :new) &&\n+\t(\n+\t\techo \"100644 $sha1 1\told\"\n+\t\techo \"100644 $sha1 3\told\"\n+\t) | git update-index --index-info &&\n+\t>old &&\n+\tgit ls-files -s &&\n+\tgit checkout -f &&\n+\tgit ls-files -s >actual &&\n+\t! test -f old\n '\n \n test_expect_success 'Porcelain checkout -f HEAD should remove remnants too' '\n-  read_tree_u_must_succeed --reset -u HEAD &&\n-  git ls-files -s >expect &&\n-  sha1=$(git rev-parse :new) &&\n-  (\n-\techo \"100644 $sha1 1\told\"\n-\techo \"100644 $sha1 3\told\"\n-  ) | git update-index --index-info &&\n-  >old &&\n-  git ls-files -s &&\n-  git checkout -f HEAD &&\n-  git ls-files -s >actual &&\n-  ! test -f old\n+\tread_tree_u_must_succeed --reset -u HEAD &&\n+\tgit ls-files -s >expect &&\n+\tsha1=$(git rev-parse :new) &&\n+\t(\n+\t\techo \"100644 $sha1 1\told\"\n+\t\techo \"100644 $sha1 3\told\"\n+\t) | git update-index --index-info &&\n+\t>old &&\n+\tgit ls-files -s &&\n+\tgit checkout -f HEAD &&\n+\tgit ls-files -s >actual &&\n+\t! test -f old\n '\n \n test_done\n-- \n1.8.5-rc0-281-g8951339\n"},{"id":"230021","messageId":"1383345895-23341-4-git-send-email-gitster@pobox.com","threadId":"35230","inReplyTo":"1383345895-23341-1-git-send-email-gitster@pobox.com","subject":"[PATCH 3/3] t1005: add test for \"read-tree --reset -u A B\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-01T22:44:55Z","receivedAt":"2013-11-01T22:44:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"With a conflicted index, this used to give us an error.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t1005-read-tree-reset.sh | 15 +++++++++++++++\n 1 file changed, 15 insertions(+)\n\ndiff --git a/t/t1005-read-tree-reset.sh b/t/t1005-read-tree-reset.sh\nindex e29cf63..0745685 100755\n--- a/t/t1005-read-tree-reset.sh\n+++ b/t/t1005-read-tree-reset.sh\n@@ -43,6 +43,21 @@ test_expect_success 'reset should remove remnants from a failed merge' '\n \t! test -f old\n '\n \n+test_expect_success 'two-way reset should remove remnants too' '\n+\tread_tree_u_must_succeed --reset -u HEAD &&\n+\tgit ls-files -s >expect &&\n+\tsha1=$(git rev-parse :new) &&\n+\t(\n+\t\techo \"100644 $sha1 1\told\"\n+\t\techo \"100644 $sha1 3\told\"\n+\t) | git update-index --index-info &&\n+\t>old &&\n+\tgit ls-files -s &&\n+\tread_tree_u_must_succeed --reset -u HEAD HEAD &&\n+\tgit ls-files -s >actual &&\n+\t! test -f old\n+'\n+\n test_expect_success 'Porcelain reset should remove remnants too' '\n \tread_tree_u_must_succeed --reset -u HEAD &&\n \tgit ls-files -s >expect &&\n-- \n1.8.5-rc0-281-g8951339\n"},{"id":"230047","messageId":"20131103080300.GA2868@sigill.intra.peff.net","threadId":"35230","inReplyTo":"1383345895-23341-1-git-send-email-gitster@pobox.com","subject":"Re: Re* [BUG?] \"error: cache entry has null sha1\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-11-03T08:03:00Z","receivedAt":"2013-11-03T08:03:00Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 01, 2013 at 03:44:52PM -0700, Junio C Hamano wrote:\n\n> Here is a proposed endgame for the topic in a patch form, then.\n> \n> I've added a test for low-level \"read-tree --reset -u A B\", and\n> tried the \"am --abort\" I saw the problem with manually, but other\n> than that, I haven't (re)thought about the issue hard enough to be\n> comfortable with this change yet.\n\nThanks for moving this forward.\n\nI read over the old discussion and the patches, and I think the patch is\na good thing. There was some question from me earlier on whether there\nwere other cases we weren't considering.  The discussion convinced me\nthat there almost certainly aren't. And even if there are, this is still\nthe right direction. Writing out a bogus CE_CONFLICTED entry is\n_certainly_ wrong, so even if we do not get the details right (e.g.,\nrejecting a merge we should be accepting), this patch still forms a base\nfor further fixups.\n\n> Jeff King (1):\n>   unpack-trees: fix \"read-tree -u --reset A B\" with conflicted index\n\nMy missing signoff:\n\n  Signed-off-by: Jeff King <peff@peff.net>\n\n-Peff\n"}]}