{"thread":{"id":"44054","subject":"git clone http:// fails some times with \"Request for d53.. aborted\"","startedAt":"2016-09-09T22:58:24Z","lastAt":"2016-09-21T22:31:47Z","messageCount":15,"participants":["Yaroslav Halchenko","Eric Wong","Junio C Hamano","Stefan Beller","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"301578","messageId":"20160909221942.GS9830@onerussian.com","threadId":"44054","inReplyTo":null,"subject":"git clone http:// fails some times with \"Request for d53.. aborted\"","fromName":"Yaroslav Halchenko","fromEmail":"yoh@onerussian.com","sentAt":"2016-09-09T22:19:42Z","receivedAt":"2016-09-09T22:58:24Z","isPatch":false,"sender":{"key":"yoh@onerussian.com","avatar":"https://gravatar.com/avatar/8901b82415ae451a83aea49409708912726e53620e3ac92320bf1f86548d97e9?d=mp&s=160"},"body":"even when (v 2.7.0) ran on the box where the server is, so\nunlikely to be network issue\n\nor from my laptop (v 2.9.3) with ok but wifi with a weakish signal to the\naccess point:\n\n$> ( set -e; for s in {1..100}; do rm -rf fbirn_phaseIII ; git clone http://datasets.datalad.org/nidm/fbirn_phaseIII/.git; done; )\nCloning into 'fbirn_phaseIII'...\nChecking connectivity... done.\nCloning into 'fbirn_phaseIII'...\nerror: Request for d53302dfc7ad13b786923927021039d21a10d5bd aborted\nerror: Unable to find d53302dfc7ad13b786923927021039d21a10d5bd under http://datasets.datalad.org/nidm/fbirn_phaseIII/.git\nCannot obtain needed tree d53302dfc7ad13b786923927021039d21a10d5bd\nwhile processing commit 22dd4c49417cad6f4082ac2aebef45da8b6e473d.\nerror: fetch failed.\n\neven if I build fresh 2.10.0, discovered that we can trace now CURL calls\n\n$> export PATH=$HOME/proj/misc/git:$PATH\n$> git version\ngit version 2.10.0\n$> ( set -e; for s in {1..100}; do rm -rf fbirn_phaseIII ; GIT_TRACE_CURL=/tmp/git-trace-curl-failed.log git clone http://datasets.datalad.org/nidm/fbirn_phaseIII/.git &&  mv /tmp/git-trace-curl-failed.log /tmp/git-trace-curl-good.log || exit 1; done; ) \nCloning into 'fbirn_phaseIII'...\nwarning: templates not found /home/yoh/share/git-core/templates\n... a few more ...\nCloning into 'fbirn_phaseIII'...\nwarning: templates not found /home/yoh/share/git-core/templates\nerror: Request for d53302dfc7ad13b786923927021039d21a10d5bd aborted\nerror: Unable to find d53302dfc7ad13b786923927021039d21a10d5bd under http://datasets.datalad.org/nidm/fbirn_phaseIII/.git\nCannot obtain needed tree d53302dfc7ad13b786923927021039d21a10d5bd\nwhile processing commit 22dd4c49417cad6f4082ac2aebef45da8b6e473d.\nerror: fetch failed.\n( set -e; for s in {1..100}; do; rm -rf fbirn_phaseIII; GIT_TRACE_CURL= git  )  18.70s user 8.01s system 34% cpu 1:16.84 total\n\nNB \n- note that it is d53302dfc7ad13b786923927021039d21a10d5bd all the time!\n- that one corresponds to a tree\n$> git show d53302dfc7ad13b786923927021039d21a10d5bd\ntree d53302dfc7ad13b786923927021039d21a10d5bd\n\n1ec/\nda9/\n- I do not see that object being reported as failed to be found on the server:\n\n$> grep d5/3302dfc7ad13b786923927021039d21a10d5bd /var/log/apache2/datasets.datalad.org-error.log\n$> grep d5/3302dfc7ad13b786923927021039d21a10d5bd /var/log/apache2/datasets.datalad.org-access.log\n129.170.31.34 - - [09/Sep/2016:14:14:25 -0400] \"GET /nidm/fbirn_phaseIII/.git/objects/d5/3302dfc7ad13b786923927021039d21a10d5bd HTTP/1.1\" 200 72\n...\n- and the logs say that only \"good\" run had it requested:\n\n$> zgrep 3302dfc7ad13b786923927021039d21a10d5bd git-trace-curl-*.log.gz     \ngit-trace-curl-good.log.gz:18:09:13.749268 http.c:517              => Send header: GET /nidm/fbirn_phaseIII/.git/objects/d5/3302dfc7ad13b786923927021039d21a10d5bd HTTP/1.1\n\n\nand here are those two GIT_TRACE_CURL traces from good and bad runs -- may be\nsomeone with a better clue could figure it out or any other information I\nshould provide to help pin point why that \"request for d5... aborted\" ;)\n\nhttp://www.onerussian.com/tmp/git-trace-curl-good.log.gz\nhttp://www.onerussian.com/tmp/git-trace-curl-failed.log.gz\n\nThanks in advance and please CC me in replies\n-- \nYaroslav O. Halchenko\nCenter for Open Neuroscience     http://centerforopenneuroscience.org\nDartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755\nPhone: +1 (603) 646-9834                       Fax: +1 (603) 646-1419\nWWW:   http://www.linkedin.com/in/yarik        \n"},{"id":"301587","messageId":"20160910002151.GA8476@dcvr","threadId":"44054","inReplyTo":"20160909221942.GS9830@onerussian.com","subject":"Re: git clone http:// fails some times with \"Request for d53.. aborted\"","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-09-10T00:21:51Z","receivedAt":"2016-09-10T00:21:55Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"(Not sure how much time I'll have to continue in the next few weeks,\n just jotting down my debugging progress so far...)\n\nYaroslav Halchenko <yoh@onerussian.com> wrote:\n> even when (v 2.7.0) ran on the box where the server is, so\n> unlikely to be network issue\n> \n> or from my laptop (v 2.9.3) with ok but wifi with a weakish signal to the\n> access point:\n> \n> $> ( set -e; for s in {1..100}; do rm -rf fbirn_phaseIII ; git clone http://datasets.datalad.org/nidm/fbirn_phaseIII/.git; done; )\n> Cloning into 'fbirn_phaseIII'...\n> Checking connectivity... done.\n> Cloning into 'fbirn_phaseIII'...\n> error: Request for d53302dfc7ad13b786923927021039d21a10d5bd aborted\n> error: Unable to find d53302dfc7ad13b786923927021039d21a10d5bd under http://datasets.datalad.org/nidm/fbirn_phaseIII/.git\n> Cannot obtain needed tree d53302dfc7ad13b786923927021039d21a10d5bd\n> while processing commit 22dd4c49417cad6f4082ac2aebef45da8b6e473d.\n> error: fetch failed.\n> \n> even if I build fresh 2.10.0, discovered that we can trace now CURL calls\n\nAlso, GIT_CURL_VERBOSE=1 is useful, too\n\nAnd GIT_HTTP_MAX_REQUESTS=1 (default 5) can be used to limit concurrency\nif there's a suspected concurrency bug.\n\nInitially, I was worried some of my 2.10.0 http-walker speedups were\nresponsible, but it does not seem to be the case since it happens on old\nversions, too...\n\nI'm trying with the following to track state==ABORTED requests:\n\ndiff --git a/http-walker.c b/http-walker.c\nindex 0b24255..4f25b07 100644\n--- a/http-walker.c\n+++ b/http-walker.c\n@@ -56,6 +56,7 @@ static void start_object_request(struct walker *walker,\n \n \treq = new_http_object_request(obj_req->repo->base, obj_req->sha1);\n \tif (req == NULL) {\n+\t\twarning(\"obj aborted at %d\", __LINE__);\n \t\tobj_req->state = ABORTED;\n \t\treturn;\n \t}\ndiff --git a/http.c b/http.c\nindex cd40b01..cac5db9 100644\n--- a/http.c\n+++ b/http.c\n@@ -1022,6 +1022,8 @@ int start_active_slot(struct active_request_slot *slot)\n \n \tif (curlm_result != CURLM_OK &&\n \t    curlm_result != CURLM_CALL_MULTI_PERFORM) {\n+\t\twarning(\"curl_multi_add_handle failed: %s\",\n+\t\t\tcurl_multi_strerror(curlm_result));\n \t\tactive_requests--;\n \t\tslot->in_use = 0;\n \t\treturn 0;\n\n\nAnd getting a few of the following:\n\n\twarning: curl_multi_add_handle failed: The easy handle is already added to a multi handle\n\n\t(error code: CURLM_ADDED_ALREADY)\n\nI suspect there is some state management bug.   And yes, it's\nintermittent, I'm testing from my server (dcvr.yhbt.net) with good\nconnectivity and it does not happen all the time.\n\nIt also happens regardless of GIT_HTTP_MAX_REQUESTS being 1 or 5\n(default), too.\n\nSo yeah, hopefully this info is helpful to someone else on the list\nin case I don't return soon.\n"},{"id":"301791","messageId":"20160913002557.10671-1-e@80x24.org","threadId":"44054","inReplyTo":"20160909221942.GS9830@onerussian.com","subject":"[RFC 0/3] http: avoid repeatedly adding curl easy to curlm","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-09-13T00:25:54Z","receivedAt":"2016-09-13T00:26:01Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"(I have some hours online today, so I decided to work on this)\n\nThe key patch here is 3/3 which seems like an obvious fix to\nadding the problem of adding a curl easy handle to a curl multi\nhandle repeatedly.\n\nWhat is unclear to me is how only Yaroslav's repository seems to\ntrigger this bug after all these years...\n\nHowever, I am fairly sure this fixes the bug Yaroslav\nencountered.  This patch series is also needed for 2.9.3 and\nperhaps older maintenance tracks for distros.\n\nIn PATCH 2/3, I originally had error checking, but removed it\nafter noticing it was causing failures on wheezy.\n\nI will investigate those failures in a week or two when I regain\nregular computer access.\n\nThese patches are needed for 2.9.x (and probably earlier), too.\n\nThe following changes since commit cda1bbd474805e653dda8a71d4ea3790e2a66cbb:\n\n  Sync with maint (2016-09-08 22:00:53 -0700)\n\nare available in the git repository at:\n\n  git://bogomips.org/git-svn.git dumb-http-release\n\nfor you to fetch changes up to 3f561d0f0f78fd841708b5e81122e9d825919fd3:\n\n  http: always remove curl easy from curlm session on release (2016-09-12 23:59:35 +0000)\n\n----------------------------------------------------------------\nEric Wong (3):\n  http: warn on curl_multi_add_handle failures\n  http: consolidate #ifdefs for curl_multi_remove_handle\n  http: always remove curl easy from curlm session on release\n\n http.c | 29 ++++++++++++++++++-----------\n 1 file changed, 18 insertions(+), 11 deletions(-)\n\n-- \nEW\n"},{"id":"301792","messageId":"20160913002557.10671-2-e@80x24.org","threadId":"44054","inReplyTo":"20160913002557.10671-1-e@80x24.org","subject":"[RFC 1/3] http: warn on curl_multi_add_handle failures","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-09-13T00:25:55Z","receivedAt":"2016-09-13T00:26:05Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"This will be useful for tracking down curl usage errors.\n\nSigned-off-by: Eric Wong <e@80x24.org>\n---\n http.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/http.c b/http.c\nindex cd40b01..cac5db9 100644\n--- a/http.c\n+++ b/http.c\n@@ -1022,6 +1022,8 @@ int start_active_slot(struct active_request_slot *slot)\n \n \tif (curlm_result != CURLM_OK &&\n \t    curlm_result != CURLM_CALL_MULTI_PERFORM) {\n+\t\twarning(\"curl_multi_add_handle failed: %s\",\n+\t\t\tcurl_multi_strerror(curlm_result));\n \t\tactive_requests--;\n \t\tslot->in_use = 0;\n \t\treturn 0;\n-- \nEW\n\n"},{"id":"301794","messageId":"20160913002557.10671-3-e@80x24.org","threadId":"44054","inReplyTo":"20160913002557.10671-1-e@80x24.org","subject":"[RFC 2/3] http: consolidate #ifdefs for curl_multi_remove_handle","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-09-13T00:25:56Z","receivedAt":"2016-09-13T00:26:15Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"I find #ifdefs makes code difficult-to-follow.\n\nAn early version of this patch had error checking for\ncurl_multi_remove_handle calls, but caused some tests (e.g.\nt5541) to fail under curl 7.26.0 on old Debian wheezy.\n\nSigned-off-by: Eric Wong <e@80x24.org>\n---\n http.c | 17 ++++++++++-------\n 1 file changed, 10 insertions(+), 7 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex cac5db9..a7eaf7b 100644\n--- a/http.c\n+++ b/http.c\n@@ -201,6 +201,13 @@ static void finish_active_slot(struct active_request_slot *slot)\n \t\tslot->callback_func(slot->callback_data);\n }\n \n+static void xmulti_remove_handle(struct active_request_slot *slot)\n+{\n+#ifdef USE_CURL_MULTI\n+\tcurl_multi_remove_handle(curlm, slot->curl);\n+#endif\n+}\n+\n #ifdef USE_CURL_MULTI\n static void process_curl_messages(void)\n {\n@@ -216,7 +223,7 @@ static void process_curl_messages(void)\n \t\t\t       slot->curl != curl_message->easy_handle)\n \t\t\t\tslot = slot->next;\n \t\t\tif (slot != NULL) {\n-\t\t\t\tcurl_multi_remove_handle(curlm, slot->curl);\n+\t\t\t\txmulti_remove_handle(slot);\n \t\t\t\tslot->curl_result = curl_result;\n \t\t\t\tfinish_active_slot(slot);\n \t\t\t} else {\n@@ -881,9 +888,7 @@ void http_cleanup(void)\n \twhile (slot != NULL) {\n \t\tstruct active_request_slot *next = slot->next;\n \t\tif (slot->curl != NULL) {\n-#ifdef USE_CURL_MULTI\n-\t\t\tcurl_multi_remove_handle(curlm, slot->curl);\n-#endif\n+\t\t\txmulti_remove_handle(slot);\n \t\t\tcurl_easy_cleanup(slot->curl);\n \t\t}\n \t\tfree(slot);\n@@ -1164,9 +1169,7 @@ static void release_active_slot(struct active_request_slot *slot)\n {\n \tclosedown_active_slot(slot);\n \tif (slot->curl && curl_session_count > min_curl_sessions) {\n-#ifdef USE_CURL_MULTI\n-\t\tcurl_multi_remove_handle(curlm, slot->curl);\n-#endif\n+\t\txmulti_remove_handle(slot);\n \t\tcurl_easy_cleanup(slot->curl);\n \t\tslot->curl = NULL;\n \t\tcurl_session_count--;\n-- \nEW\n\n"},{"id":"301795","messageId":"20160913002557.10671-4-e@80x24.org","threadId":"44054","inReplyTo":"20160913002557.10671-1-e@80x24.org","subject":"[RFC 3/3] http: always remove curl easy from curlm session on release","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-09-13T00:25:57Z","receivedAt":"2016-09-13T00:26:18Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"We must call curl_multi_remove_handle when releasing the slot to\nprevent subsequent calls to curl_multi_add_handle from failing\nwith CURLM_ADDED_ALREADY (in curl 7.32.1+; older versions\nreturned CURLM_BAD_EASY_HANDLE)\n\nSigned-off-by: Eric Wong <e@80x24.org>\n---\n http.c | 10 ++++++----\n 1 file changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex a7eaf7b..3c4c3f5 100644\n--- a/http.c\n+++ b/http.c\n@@ -1168,11 +1168,13 @@ void run_active_slot(struct active_request_slot *slot)\n static void release_active_slot(struct active_request_slot *slot)\n {\n \tclosedown_active_slot(slot);\n-\tif (slot->curl && curl_session_count > min_curl_sessions) {\n+\tif (slot->curl) {\n \t\txmulti_remove_handle(slot);\n-\t\tcurl_easy_cleanup(slot->curl);\n-\t\tslot->curl = NULL;\n-\t\tcurl_session_count--;\n+\t\tif (curl_session_count > min_curl_sessions) {\n+\t\t\tcurl_easy_cleanup(slot->curl);\n+\t\t\tslot->curl = NULL;\n+\t\t\tcurl_session_count--;\n+\t\t}\n \t}\n #ifdef USE_CURL_MULTI\n \tfill_active_slots();\n-- \nEW\n\n"},{"id":"301868","messageId":"xmqqr38nv8ul.fsf@gitster.mtv.corp.google.com","threadId":"44054","inReplyTo":"20160913002557.10671-1-e@80x24.org","subject":"Re: [RFC 0/3] http: avoid repeatedly adding curl easy to curlm","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-13T20:32:50Z","receivedAt":"2016-09-13T20:33:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong <e@80x24.org> writes:\n\n> The key patch here is 3/3 which seems like an obvious fix to\n> adding the problem of adding a curl easy handle to a curl multi\n> handle repeatedly.\n\nYeah, sounds like the right thing to do and 2/3 makes it really easy\nto read the resulting code.\n\n> I will investigate those failures in a week or two when I regain\n> regular computer access.\n\nThanks. Will tentatively queue on 'pu' and wait for updates.\n\n"},{"id":"301903","messageId":"20160914115918.GS9830@onerussian.com","threadId":"44054","inReplyTo":"20160913002557.10671-1-e@80x24.org","subject":"Re: [RFC 0/3] http: avoid repeatedly adding curl easy to curlm","fromName":"Yaroslav Halchenko","fromEmail":"yoh@onerussian.com","sentAt":"2016-09-14T11:59:18Z","receivedAt":"2016-09-14T11:59:25Z","isPatch":false,"sender":{"key":"yoh@onerussian.com","avatar":"https://gravatar.com/avatar/8901b82415ae451a83aea49409708912726e53620e3ac92320bf1f86548d97e9?d=mp&s=160"},"body":"\nOn Tue, 13 Sep 2016, Eric Wong wrote:\n> What is unclear to me is how only Yaroslav's repository seems to\n> trigger this bug after all these years...\n\nThank you Eric very much for tracking down this issue!  Since issue is\nintermittent, I guess people just didn't bother going through reporting\nif they got an error once in a while, and/or could have attributed\nto somehow misran update-server-info.\n\n> However, I am fairly sure this fixes the bug Yaroslav\n> encountered.  This patch series is also needed for 2.9.3 and\n> perhaps older maintenance tracks for distros.\n\nFWIW I have tested your branch locally - and do not observe that bug any\nlonger.  Thanks again!\n\n-- \nYaroslav O. Halchenko\nCenter for Open Neuroscience     http://centerforopenneuroscience.org\nDartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755\nPhone: +1 (603) 646-9834                       Fax: +1 (603) 646-1419\nWWW:   http://www.linkedin.com/in/yarik        \n"},{"id":"301906","messageId":"20160914140318.GB9833@onerussian.com","threadId":"44054","inReplyTo":"20160909221942.GS9830@onerussian.com","subject":"git submodule add spits unrelated to actual problem error msg about .gitignore","fromName":"Yaroslav Halchenko","fromEmail":"yoh@onerussian.com","sentAt":"2016-09-14T14:03:18Z","receivedAt":"2016-09-14T14:03:46Z","isPatch":false,"sender":{"key":"yoh@onerussian.com","avatar":"https://gravatar.com/avatar/8901b82415ae451a83aea49409708912726e53620e3ac92320bf1f86548d97e9?d=mp&s=160"},"body":"I have spent some time chasing the wild goose (well - the .gitignore\nfile) after getting:\n\n    $> git-submodule add --name fcx-1 ./fcx-1/ ./fcx-1/\n    The following path is ignored by one of your .gitignore files:\n    fcx-1\n    Use -f if you really want to add it.\n\nlong story short -- the culprit is this piece of code in git-submodule:\n\n    if test -z \"$force\" && ! git add --dry-run --ignore-missing \"$sm_path\" > /dev/null 2>&1\n    then\n        eval_gettextln \"The following path is ignored by one of your .gitignore files:\n\\$sm_path\nUse -f if you really want to add it.\" >&2\n        exit 1\n    fi\n\n\nso if anything goes wrong in git add, it just reports  this error\nmessage.\n\nFTR -- actual problem in my case was:\n\n    $> git add --dry-run --ignore-missing fcx-1\n    fatal: Unable to create '/mnt/datasets/datalad/crawl/crcns/.git/index.lock': File exists.\n\n    Another git process seems to be running in this repository, e.g.\n    an editor opened by 'git commit'. Please make sure all processes\n    are terminated then try again. If it still fails, a git process\n    may have crashed in this repository earlier:\n    remove the file manually to continue.\n\n;-)\n\nCheers!\nP.S. Please CC me in replies\n-- \nYaroslav O. Halchenko\nCenter for Open Neuroscience     http://centerforopenneuroscience.org\nDartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755\nPhone: +1 (603) 646-9834                       Fax: +1 (603) 646-1419\nWWW:   http://www.linkedin.com/in/yarik        \n"},{"id":"301922","messageId":"CAGZ79kbdfWHDGzoe21LVqt6naMJPWGf45S1oknrAp6=Z-Qm8dQ@mail.gmail.com","threadId":"44054","inReplyTo":"20160914140318.GB9833@onerussian.com","subject":"Re: git submodule add spits unrelated to actual problem error msg about .gitignore","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-09-14T19:32:11Z","receivedAt":"2016-09-14T19:32:18Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Sep 14, 2016 at 7:03 AM, Yaroslav Halchenko <yoh@onerussian.com> wrote:\n> I have spent some time chasing the wild goose (well - the .gitignore\n> file) after getting:\n>\n>     $> git-submodule add --name fcx-1 ./fcx-1/ ./fcx-1/\n>     The following path is ignored by one of your .gitignore files:\n>     fcx-1\n>     Use -f if you really want to add it.\n>\n> long story short -- the culprit is this piece of code in git-submodule:\n>\n>     if test -z \"$force\" && ! git add --dry-run --ignore-missing \"$sm_path\" > /dev/null 2>&1\n>     then\n>         eval_gettextln \"The following path is ignored by one of your .gitignore files:\n> \\$sm_path\n> Use -f if you really want to add it.\" >&2\n>         exit 1\n>     fi\n>\n>\n> so if anything goes wrong in git add, it just reports  this error\n> message.\n\nThanks for the bug report!\nI think we could chop off \"2>&1\" as that would have exposed the\nunderlying error.\n\nAnother way to go would be to use verbose git-add and grep for\nthe string \"add '$sm_path'\".\n\n     if test -z \"$force\" && ! git add --verbose --dry-run\n--ignore-missing \"$sm_path\" |grep \"add $sm_path\"\n\ngit-add already gives the correct (the same error message) for  the\nignored files, so maybe we'd just do:\n\n    # no need for a if, but this single line will do:\n    test -z \"$force\" && git add --dry-run git.o >/dev/null || exit 1\n"},{"id":"301926","messageId":"6B9620C2-9459-497A-BDF7-7A58A22C687D@onerussian.com","threadId":"44054","inReplyTo":"CAGZ79kbdfWHDGzoe21LVqt6naMJPWGf45S1oknrAp6=Z-Qm8dQ@mail.gmail.com","subject":"Re: git submodule add spits unrelated to actual problem error msg about .gitignore","fromName":"Yaroslav Halchenko","fromEmail":"yoh@onerussian.com","sentAt":"2016-09-14T20:23:56Z","receivedAt":"2016-09-14T20:27:51Z","isPatch":false,"sender":{"key":"yoh@onerussian.com","avatar":"https://gravatar.com/avatar/8901b82415ae451a83aea49409708912726e53620e3ac92320bf1f86548d97e9?d=mp&s=160"},"body":"On September 14, 2016 3:32:11 PM EDT, Stefan Beller <sbeller@google.com> wrote:\n!\n>I think we could chop off \"2>&1\" as that would have exposed the\n>underlying error.\n>\n>Another way to go would be to use verbose git-add and grep for\n>the string \"add '$sm_path'\".\n>\n>     if test -z \"$force\" && ! git add --verbose --dry-run\n>--ignore-missing \"$sm_path\" |grep \"add $sm_path\"\n>\n>git-add already gives the correct (the same error message) for  the\n>ignored files, so maybe we'd just do:\n>\n>    # no need for a if, but this single line will do:\n>    test -z \"$force\" && git add --dry-run git.o >/dev/null || exit 1\n\nFWIW Imho exposing error is good but not sufficient alone, since custom gitignore message would still be confusing.\n-- \nSent from a phone which beats iPhone.\n"},{"id":"302332","messageId":"20160921214623.GA1919@whir","threadId":"44054","inReplyTo":"xmqqr38nv8ul.fsf@gitster.mtv.corp.google.com","subject":"[PATCH 4/3] http: check curl_multi_remove_handle error code","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-09-21T21:46:23Z","receivedAt":"2016-09-21T21:46:28Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Junio C Hamano <gitster@pobox.com> wrote:\n> Eric Wong <e@80x24.org> writes:\n> > The key patch here is 3/3 which seems like an obvious fix to\n> > adding the problem of adding a curl easy handle to a curl multi\n> > handle repeatedly.\n> \n> Yeah, sounds like the right thing to do and 2/3 makes it really easy\n> to read the resulting code.\n> \n> > I will investigate those failures in a week or two when I regain\n> > regular computer access.\n> \n> Thanks. Will tentatively queue on 'pu' and wait for updates.\n\nI'm comfortable with the original 3 patch series in 'next'\nand being merged to 'master' and 'maint', soon.\n\nI don't think the following 4/3 is strictly necessary now, so\nI'd be more comfortable with it being tested in 'pu' or 'next'\nfor a longer period.\n\n(online today, but not much tomorrow or another few days after)\n\n-----------8<-----------\nSubject: [PATCH] http: check curl_multi_remove_handle error code\n\nThis should help detect bugs in future changes.  While we're at\nit, fix a (probably innocuous) bug in our http_cleanup function\nfor users of older curl.\n\ncurl_multi_remove_handle was not idempotent until curl 7.33.0\nwith commit 84f3b3dd448399f9548468676e1bd1475dba8de5\n(\"curl_multi_remove_handle: allow multiple removes\"),\nso we track the \"curlm\" membership of the curl easy handle\nourselves with a new \"in_multi\" flag.\n\nTested with curl 7.26.0 and 7.38.0 on Debian 7.x (wheezy) and\nDebian 8.x (jessie) respectively.\n\nSigned-off-by: Eric Wong <e@80x24.org>\n---\n http.c | 12 ++++++++++--\n http.h |  1 +\n 2 files changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 82ed542..9f97749 100644\n--- a/http.c\n+++ b/http.c\n@@ -204,7 +204,12 @@ static void finish_active_slot(struct active_request_slot *slot)\n static void xmulti_remove_handle(struct active_request_slot *slot)\n {\n #ifdef USE_CURL_MULTI\n-\tcurl_multi_remove_handle(curlm, slot->curl);\n+\tCURLMcode code = curl_multi_remove_handle(curlm, slot->curl);\n+\n+\tif (code != CURLM_OK)\n+\t\tdie(\"curl_multi_remove_handle failed (%p): %s\",\n+\t\t\tslot->curl, curl_multi_strerror(code));\n+\tslot->in_multi = 0;\n #endif\n }\n \n@@ -888,7 +893,8 @@ void http_cleanup(void)\n \twhile (slot != NULL) {\n \t\tstruct active_request_slot *next = slot->next;\n \t\tif (slot->curl != NULL) {\n-\t\t\txmulti_remove_handle(slot);\n+\t\t\tif (slot->in_multi)\n+\t\t\t\txmulti_remove_handle(slot);\n \t\t\tcurl_easy_cleanup(slot->curl);\n \t\t}\n \t\tfree(slot);\n@@ -965,6 +971,7 @@ struct active_request_slot *get_active_slot(void)\n \t\tnewslot = xmalloc(sizeof(*newslot));\n \t\tnewslot->curl = NULL;\n \t\tnewslot->in_use = 0;\n+\t\tnewslot->in_multi = 0;\n \t\tnewslot->next = NULL;\n \n \t\tslot = active_queue_head;\n@@ -1033,6 +1040,7 @@ int start_active_slot(struct active_request_slot *slot)\n \t\tslot->in_use = 0;\n \t\treturn 0;\n \t}\n+\tslot->in_multi = 1;\n \n \t/*\n \t * We know there must be something to do, since we just added\ndiff --git a/http.h b/http.h\nindex 5ab9d9c..3339d70 100644\n--- a/http.h\n+++ b/http.h\n@@ -60,6 +60,7 @@ struct slot_results {\n struct active_request_slot {\n \tCURL *curl;\n \tint in_use;\n+\tint in_multi;\n \tCURLcode curl_result;\n \tlong http_code;\n \tint *finished;\n-- \nEW\n\n"},{"id":"302340","messageId":"20160921222208.ccgnpgx3gcvbjpfm@sigill.intra.peff.net","threadId":"44054","inReplyTo":"20160921214623.GA1919@whir","subject":"Re: [PATCH 4/3] http: check curl_multi_remove_handle error code","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-21T22:22:08Z","receivedAt":"2016-09-21T22:22:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 21, 2016 at 09:46:23PM +0000, Eric Wong wrote:\n\n> -----------8<-----------\n> Subject: [PATCH] http: check curl_multi_remove_handle error code\n> \n> This should help detect bugs in future changes.  While we're at\n> it, fix a (probably innocuous) bug in our http_cleanup function\n> for users of older curl.\n> \n> curl_multi_remove_handle was not idempotent until curl 7.33.0\n> with commit 84f3b3dd448399f9548468676e1bd1475dba8de5\n> (\"curl_multi_remove_handle: allow multiple removes\"),\n> so we track the \"curlm\" membership of the curl easy handle\n> ourselves with a new \"in_multi\" flag.\n\nDoes curl provide a meaningful error here? I'm just wondering if we\ncould simply let curl handle this, and just ignore the error that comes\nfrom older versions. We're basically just replicating curl's own state\ndata here.\n\n-Peff\n"},{"id":"302343","messageId":"20160921222959.GA4781@whir","threadId":"44054","inReplyTo":"20160921222208.ccgnpgx3gcvbjpfm@sigill.intra.peff.net","subject":"Re: [PATCH 4/3] http: check curl_multi_remove_handle error code","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-09-21T22:29:59Z","receivedAt":"2016-09-21T22:30:04Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Jeff King <peff@peff.net> wrote:\n> On Wed, Sep 21, 2016 at 09:46:23PM +0000, Eric Wong wrote:\n> \n> > -----------8<-----------\n> > Subject: [PATCH] http: check curl_multi_remove_handle error code\n> > \n> > This should help detect bugs in future changes.  While we're at\n> > it, fix a (probably innocuous) bug in our http_cleanup function\n> > for users of older curl.\n> > \n> > curl_multi_remove_handle was not idempotent until curl 7.33.0\n> > with commit 84f3b3dd448399f9548468676e1bd1475dba8de5\n> > (\"curl_multi_remove_handle: allow multiple removes\"),\n> > so we track the \"curlm\" membership of the curl easy handle\n> > ourselves with a new \"in_multi\" flag.\n> \n> Does curl provide a meaningful error here? I'm just wondering if we\n> could simply let curl handle this, and just ignore the error that comes\n> from older versions. We're basically just replicating curl's own state\n> data here.\n\ncurl before 7.33.0 returned CURLM_BAD_EASY_HANDLE.  This error\ncode also happens if we pass a bad/corrupt easy handle;\nso it could be hiding an error on our end.\n"},{"id":"302344","messageId":"20160921223140.qj3354gtrm6km6mg@sigill.intra.peff.net","threadId":"44054","inReplyTo":"20160921222959.GA4781@whir","subject":"Re: [PATCH 4/3] http: check curl_multi_remove_handle error code","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-21T22:31:40Z","receivedAt":"2016-09-21T22:31:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 21, 2016 at 10:29:59PM +0000, Eric Wong wrote:\n\n> Jeff King <peff@peff.net> wrote:\n> > On Wed, Sep 21, 2016 at 09:46:23PM +0000, Eric Wong wrote:\n> > \n> > > -----------8<-----------\n> > > Subject: [PATCH] http: check curl_multi_remove_handle error code\n> > > \n> > > This should help detect bugs in future changes.  While we're at\n> > > it, fix a (probably innocuous) bug in our http_cleanup function\n> > > for users of older curl.\n> > > \n> > > curl_multi_remove_handle was not idempotent until curl 7.33.0\n> > > with commit 84f3b3dd448399f9548468676e1bd1475dba8de5\n> > > (\"curl_multi_remove_handle: allow multiple removes\"),\n> > > so we track the \"curlm\" membership of the curl easy handle\n> > > ourselves with a new \"in_multi\" flag.\n> > \n> > Does curl provide a meaningful error here? I'm just wondering if we\n> > could simply let curl handle this, and just ignore the error that comes\n> > from older versions. We're basically just replicating curl's own state\n> > data here.\n> \n> curl before 7.33.0 returned CURLM_BAD_EASY_HANDLE.  This error\n> code also happens if we pass a bad/corrupt easy handle;\n> so it could be hiding an error on our end.\n\nHmm, yeah. So we would want to make it conditional on the version\nanyway, so that we detected such bugs on more modern systems. That still\ndoesn't seem unreasonable to me, but your patch is also not very big, so\nit's probably fine (the major risk with yours is simply that our state\nand curl's state get out of sync).\n\n-Peff\n"}]}