{"thread":{"id":"52133","subject":"[PATCH 0/1] worktree: teach \"add\" to ignore submodule.recurse config","startedAt":"2019-10-27T17:16:30Z","lastAt":"2019-10-30T01:02:04Z","messageCount":5,"participants":["Philippe Blain via GitGitGadget","Junio C Hamano","Philippe Blain"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"384932","messageId":"pull.430.git.1572196585.gitgitgadget@gmail.com","threadId":"52133","inReplyTo":null,"subject":"[PATCH 0/1] worktree: teach \"add\" to ignore submodule.recurse config","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-27T17:16:24Z","receivedAt":"2019-10-27T17:16:30Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"\"worktree add\" internally calls \"reset --hard\", but if submodule.recurse is\nset, reset tries to recurse into initialized submodules, which makes\nstart_command try to cd into non-existing submodule paths and die.\n\nFix that by making sure that the call to reset in \"worktree add\" does not\nrecurse.\n\nSome remarks:\n\n 1. This patch is based on maint\n \n \n 2. The 2 Travis CI macOS builds fail (they also fail on maint) with the\n message:\n \n > +brew install caskroom/cask/perforce Error: caskroom/cask was moved. Tap\n homebrew/cask-cask instead. The command \"ci/install-dependencies.sh\"\n failed and exited with 1 during .\n \n \n \n 3. I'm on OS X 10.11.6 (Apple clang-800.0.42.1) and I get a warning\n compiling builtin/merge.c : \n \n > CC builtin/merge.o\n builtin/merge.c:831:33: warning: data argument not used by format string [-Wformat-extra-args]\n no_scissors_editor_comment), comment_line_char);\n ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^\n builtin/merge.c:809:4: note: format string is defined here\n N_(\"An empty message aborts the commit.\\n\");\n ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n ./gettext.h:86:20: note: expanded from macro 'N_'\n #define N_(msgid) (msgid)\n ^~~~~\n 1 warning generated.\n \n \n This makes me unable to build with DEVELOPER=1.\n\nPhilippe Blain (1):\n worktree: teach \"add\" to ignore submodule.recurse config\n\n builtin/worktree.c | 2 +-\n t/t2400-worktree-add.sh | 24 ++++++++++++++++++++++++\n 2 files changed, 25 insertions(+), 1 deletion(-)\n\n\nbase-commit: 5fa0f5238b0cd46cfe7f6fa76c3f526ea98148d9\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-430%2Fphil-blain%2Fworktree-add-recurse-submodule-config-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-430/phil-blain/worktree-add-recurse-submodule-config-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/430\n-- \ngitgitgadget\n"},{"id":"384933","messageId":"e707bcc0a8482a8e8e82ecb259fa0dff84e63955.1572196585.git.gitgitgadget@gmail.com","threadId":"52133","inReplyTo":"pull.430.git.1572196585.gitgitgadget@gmail.com","subject":"[PATCH 1/1] worktree: teach \"add\" to ignore submodule.recurse config","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-27T17:16:25Z","receivedAt":"2019-10-27T17:16:40Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\n\"worktree add\" internally calls \"reset --hard\", but if\nsubmodule.recurse is set, reset tries to recurse into\ninitialized submodules, which makes start_command try to\ncd into non-existing submodule paths and die.\n\nFix that by making sure that the call to reset in \"worktree add\"\ndoes not recurse.\n\nSigned-off-by: Philippe Blain \n---\n builtin/worktree.c | 2 +-\n t/t2400-worktree-add.sh | 24 ++++++++++++++++++++++++\n 2 files changed, 25 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex a5bb02b207..958bea97fe 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -377,7 +377,7 @@ static int add_worktree(const char *path, const char *refname,\n \tif (opts->checkout) {\n \t\tcp.argv = NULL;\n \t\targv_array_clear(&cp.args);\n-\t\targv_array_pushl(&cp.args, \"reset\", \"--hard\", NULL);\n+\t\targv_array_pushl(&cp.args, \"reset\", \"--hard\", \"--no-recurse-submodules\", NULL);\n \t\tif (opts->quiet)\n \t\t\targv_array_push(&cp.args, \"--quiet\");\n \t\tcp.env = child_env.argv;\ndiff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\nindex e819ba741e..8a9831413c 100755\n--- a/t/t2400-worktree-add.sh\n+++ b/t/t2400-worktree-add.sh\n@@ -587,4 +587,28 @@ test_expect_success '\"add\" should not fail because of another bad worktree' '\n \t)\n '\n \n+test_expect_success '\"add\" with uninitialized submodule, with submodule.recurse unset' '\n+\ttest_create_repo submodule &&\n+\ttest_commit -C submodule first &&\n+\ttest_create_repo project &&\n+\tgit -C project submodule add ../submodule &&\n+\tgit -C project add submodule &&\n+\ttest_tick &&\n+\tgit -C project commit -m add_sub &&\n+\tgit clone project project-clone &&\n+\tgit -C project-clone worktree add ../project-2\n+'\n+test_expect_success '\"add\" with uninitialized submodule, with submodule.recurse set' '\n+\tgit -C project-clone -c submodule.recurse worktree add ../project-3\n+'\n+\n+test_expect_success '\"add\" with initialized submodule, with submodule.recurse unset' '\n+\tgit -C project-clone submodule update --init &&\n+\tgit -C project-clone worktree add ../project-4\n+'\n+\n+test_expect_success '\"add\" with initialized submodule, with submodule.recurse set' '\n+\tgit -C project-clone -c submodule.recurse worktree add ../project-5\n+'\n+\n test_done\n-- \ngitgitgadget\n"},{"id":"384961","messageId":"xmqqtv7ta9lz.fsf@gitster-ct.c.googlers.com","threadId":"52133","inReplyTo":"pull.430.git.1572196585.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/1] worktree: teach \"add\" to ignore submodule.recurse config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-28T02:26:32Z","receivedAt":"2019-10-28T02:26:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philippe Blain via GitGitGadget\" writes:\n\n> 3. I'm on OS X 10.11.6 (Apple clang-800.0.42.1) and I get a warning\n> compiling builtin/merge.c : \n> \n> > CC builtin/merge.o\n> builtin/merge.c:831:33: warning: data argument not used by format string [-Wformat-extra-args]\n> no_scissors_editor_comment), comment_line_char);\n> ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^\n> builtin/merge.c:809:4: note: format string is defined here\n> N_(\"An empty message aborts the commit.\\n\");\n> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n> ./gettext.h:86:20: note: expanded from macro 'N_'\n> #define N_(msgid) (msgid)\n> ^~~~~\n> 1 warning generated.\n\nI am not sure if the compiler needs fixing in this case, but the\nfollowing may work it around.\n\n builtin/merge.c | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex e2ccbc44e2..0746f11df2 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -826,9 +826,12 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n \t\t\tstrbuf_commented_addf(&msg, \"\\n\");\n \t\t}\n \t\tstrbuf_commented_addf(&msg, _(merge_editor_comment));\n-\t\tstrbuf_commented_addf(&msg, _(cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS ?\n-\t\t\tscissors_editor_comment :\n-\t\t\tno_scissors_editor_comment), comment_line_char);\n+\n+\t\tif (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS)\n+\t\t\tstrbuf_commented_addf(&msg, _(scissors_editor_comment));\n+\t\telse\n+\t\t\tstrbuf_commented_addf(&msg, _(no_scissors_editor_comment),\n+\t\t\t\t\t comment_line_char);\n \t}\n \tif (signoff)\n \t\tappend_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);\n"},{"id":"385073","messageId":"85CAC41A-A0EA-4E22-BC22-F57A59D76678@me.com","threadId":"52133","inReplyTo":"xmqqtv7ta9lz.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/1] worktree: teach \"add\" to ignore submodule.recurse config","fromName":"Philippe Blain","fromEmail":"philippe.blain@me.com","sentAt":"2019-10-29T12:33:21Z","receivedAt":"2019-10-29T12:39:36Z","isPatch":true,"sender":{"key":"philippe.blain@me.com","avatar":null},"body":"Hi Junio,\n\nThat indeed makes the trick. Thanks! \nShould I send a separate patch series with this patch ? How would that work ? \"Signed-off by\" me and \"Based-on-patch-by\" you ?\n\nPhilippe.\n\n> Le 27 oct. 2019 à 22:26, Junio C Hamano a écrit :\n> \n> I am not sure if the compiler needs fixing in this case, but the\n> following may work it around.\n> \n> builtin/merge.c | 9 ++++++---\n> 1 file changed, 6 insertions(+), 3 deletions(-)\n> \n> diff --git a/builtin/merge.c b/builtin/merge.c\n> index e2ccbc44e2..0746f11df2 100644\n> --- a/builtin/merge.c\n> +++ b/builtin/merge.c\n> @@ -826,9 +826,12 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n> \t\t\tstrbuf_commented_addf(&msg, \"\\n\");\n> \t\t}\n> \t\tstrbuf_commented_addf(&msg, _(merge_editor_comment));\n> -\t\tstrbuf_commented_addf(&msg, _(cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS ?\n> -\t\t\tscissors_editor_comment :\n> -\t\t\tno_scissors_editor_comment), comment_line_char);\n> +\n> +\t\tif (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS)\n> +\t\t\tstrbuf_commented_addf(&msg, _(scissors_editor_comment));\n> +\t\telse\n> +\t\t\tstrbuf_commented_addf(&msg, _(no_scissors_editor_comment),\n> +\t\t\t\t\t comment_line_char);\n> \t}\n> \tif (signoff)\n> \t\tappend_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);\n\n"},{"id":"385123","messageId":"xmqqd0ef59mh.fsf@gitster-ct.c.googlers.com","threadId":"52133","inReplyTo":"85CAC41A-A0EA-4E22-BC22-F57A59D76678@me.com","subject":"Re: [PATCH 0/1] worktree: teach \"add\" to ignore submodule.recurse config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-30T01:01:58Z","receivedAt":"2019-10-30T01:02:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philippe Blain writes:\n\n> Hi Junio,\n>\n> That indeed makes the trick. Thanks! Should I send a separate\n> patch series with this patch ? How would that work ? \"Signed-off\n> by\" me and \"Based-on-patch-by\" you ?\n\nAs I said, I am not sure if we even want a work-around or would want\nto just tell the compiler folks to fix their product, so I do not\nwant to even have to decide if I should apply such a patch ;-)\n\nBut if this were a case where somebody suggested a small diff with\n\"something like this perhaps?\" and you wrapped it up as a proper\npatch (with log message, necessary bugfixes and clean-ups, and\npossibly tests if needed), just adding \"Helped-by: somebody\"\nfollowed by your s-o-b would be the norm. Anything more extensive\nyou may want to give more credit to the original than helped-by, but\nin this case I do not think it even deserves that.\n\n>\n> Philippe.\n>\n>> Le 27 oct. 2019 à 22:26, Junio C Hamano a écrit :\n>> \n>> I am not sure if the compiler needs fixing in this case, but the\n>> following may work it around.\n>> \n>> builtin/merge.c | 9 ++++++---\n>> 1 file changed, 6 insertions(+), 3 deletions(-)\n>> \n>> diff --git a/builtin/merge.c b/builtin/merge.c\n>> index e2ccbc44e2..0746f11df2 100644\n>> --- a/builtin/merge.c\n>> +++ b/builtin/merge.c\n>> @@ -826,9 +826,12 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n>> \t\t\tstrbuf_commented_addf(&msg, \"\\n\");\n>> \t\t}\n>> \t\tstrbuf_commented_addf(&msg, _(merge_editor_comment));\n>> -\t\tstrbuf_commented_addf(&msg, _(cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS ?\n>> -\t\t\tscissors_editor_comment :\n>> -\t\t\tno_scissors_editor_comment), comment_line_char);\n>> +\n>> +\t\tif (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS)\n>> +\t\t\tstrbuf_commented_addf(&msg, _(scissors_editor_comment));\n>> +\t\telse\n>> +\t\t\tstrbuf_commented_addf(&msg, _(no_scissors_editor_comment),\n>> +\t\t\t\t\t comment_line_char);\n>> \t}\n>> \tif (signoff)\n>> \t\tappend_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);\n"}]}