{"thread":{"id":"43039","subject":"[PATCH] submodule--helper: use parallel processor correctly.","startedAt":"2016-08-09T21:29:40Z","lastAt":"2016-08-09T21:29:40Z","messageCount":1,"participants":["Stefan Beller"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"293541","messageId":"20160809212913.14365-1-sbeller@google.com","threadId":"43039","inReplyTo":null,"subject":"[PATCH] submodule--helper: use parallel processor correctly.","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-08-09T21:29:13Z","receivedAt":"2016-08-09T21:29:40Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When developing another patch series I had a temporary state in which\ngit-clone would segfault, when the call was prepared in\nprepare_to_clone_next_submodule. This lead to the call failing, i.e. in\n`update_clone_task_finished` the task was scheduled to be tried again.\nThe second call to prepare_to_clone_next_submodule would return 0, as the\nsegfaulted clone did create the .git file already, such that was not\nconsidered to need to be cloned again. I was seeing the \"BUG: ce was\na submodule before?\\n\" message, which was the correct behavior at the\ntime as my local code was buggy. When trying to debug this failure, I\ntried to use printing messages into the strbuf that is passed around,\nbut these messages were never printed as the die(..) doesn't\nflush the `err` strbuf.\n\nWhen implementing the die() in 665b35ecc (2016-06-09, \"submodule--helper:\ninitial clone learns retry logic\"), I considered this condition to be\na severe condition, which should lead to an immediate abort as we do not\ntrust ourselves any more. However the queued messages in `err` are valuable\nso let's not toss them out by immediately dying, but a graceful return.\n\nAnother thing to note: The error message itself was misleading. A return\nvalue of 0 doesn't indicate the passed in `ce` is not a submodule any more,\nbut just that we do not consider cloning it any more.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n\n Here is the last patch of the series as an independant bug fix applicable\n on master.\n \n Thanks,\n Stefan\n\n builtin/submodule--helper.c | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 6f6d67a..bd7cce6 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -748,8 +748,12 @@ static int update_clone_get_next_task(struct child_process *child,\n \tif (index < suc->failed_clones_nr) {\n \t\tint *p;\n \t\tce = suc->failed_clones[index];\n-\t\tif (!prepare_to_clone_next_submodule(ce, child, suc, err))\n-\t\t\tdie(\"BUG: ce was a submodule before?\");\n+\t\tif (!prepare_to_clone_next_submodule(ce, child, suc, err)) {\n+\t\t\tsuc->current ++;\n+\t\t\tstrbuf_addf(err, \"BUG: submodule considered for cloning,\"\n+\t\t\t\t    \"doesn't need cloning any more?\\n\");\n+\t\t\treturn 0;\n+\t\t}\n \t\tp = xmalloc(sizeof(*p));\n \t\t*p = suc->current;\n \t\t*idx_task_cb = p;\n-- \n2.9.2.583.gd6329be.dirty\n\n"}]}