* [PATCH 0/1] worktree: teach "add" to ignore submodule.recurse config @ 2019-10-27 17:16 Philippe Blain via GitGitGadget 2019-10-27 17:16 ` [PATCH 1/1] " Philippe Blain via GitGitGadget 2019-10-28 2:26 ` [PATCH 0/1] " Junio C Hamano 0 siblings, 2 replies; 5+ messages in thread From: Philippe Blain via GitGitGadget @ 2019-10-27 17:16 UTC (permalink / raw) To: git; +Cc: Junio C Hamano "worktree add" internally calls "reset --hard", but if submodule.recurse is set, reset tries to recurse into initialized submodules, which makes start_command try to cd into non-existing submodule paths and die. Fix that by making sure that the call to reset in "worktree add" does not recurse. Some remarks: 1. This patch is based on maint 2. The 2 Travis CI macOS builds fail (they also fail on maint) with the message: > +brew install caskroom/cask/perforce Error: caskroom/cask was moved. Tap homebrew/cask-cask instead. The command "ci/install-dependencies.sh" failed and exited with 1 during . 3. I'm on OS X 10.11.6 (Apple clang-800.0.42.1) and I get a warning compiling builtin/merge.c : > CC builtin/merge.o builtin/merge.c:831:33: warning: data argument not used by format string [-Wformat-extra-args] no_scissors_editor_comment), comment_line_char); ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ builtin/merge.c:809:4: note: format string is defined here N_("An empty message aborts the commit.\n"); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ./gettext.h:86:20: note: expanded from macro 'N_' #define N_(msgid) (msgid) ^~~~~ 1 warning generated. This makes me unable to build with DEVELOPER=1. Philippe Blain (1): worktree: teach "add" to ignore submodule.recurse config builtin/worktree.c | 2 +- t/t2400-worktree-add.sh | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) base-commit: 5fa0f5238b0cd46cfe7f6fa76c3f526ea98148d9 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-430%2Fphil-blain%2Fworktree-add-recurse-submodule-config-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-430/phil-blain/worktree-add-recurse-submodule-config-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/430 -- gitgitgadget ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/1] worktree: teach "add" to ignore submodule.recurse config 2019-10-27 17:16 [PATCH 0/1] worktree: teach "add" to ignore submodule.recurse config Philippe Blain via GitGitGadget @ 2019-10-27 17:16 ` Philippe Blain via GitGitGadget 2019-10-28 2:26 ` [PATCH 0/1] " Junio C Hamano 1 sibling, 0 replies; 5+ messages in thread From: Philippe Blain via GitGitGadget @ 2019-10-27 17:16 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Philippe Blain From: Philippe Blain <levraiphilippeblain@gmail.com> "worktree add" internally calls "reset --hard", but if submodule.recurse is set, reset tries to recurse into initialized submodules, which makes start_command try to cd into non-existing submodule paths and die. Fix that by making sure that the call to reset in "worktree add" does not recurse. Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> --- builtin/worktree.c | 2 +- t/t2400-worktree-add.sh | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index a5bb02b207..958bea97fe 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -377,7 +377,7 @@ static int add_worktree(const char *path, const char *refname, if (opts->checkout) { cp.argv = NULL; argv_array_clear(&cp.args); - argv_array_pushl(&cp.args, "reset", "--hard", NULL); + argv_array_pushl(&cp.args, "reset", "--hard", "--no-recurse-submodules", NULL); if (opts->quiet) argv_array_push(&cp.args, "--quiet"); cp.env = child_env.argv; diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh index e819ba741e..8a9831413c 100755 --- a/t/t2400-worktree-add.sh +++ b/t/t2400-worktree-add.sh @@ -587,4 +587,28 @@ test_expect_success '"add" should not fail because of another bad worktree' ' ) ' +test_expect_success '"add" with uninitialized submodule, with submodule.recurse unset' ' + test_create_repo submodule && + test_commit -C submodule first && + test_create_repo project && + git -C project submodule add ../submodule && + git -C project add submodule && + test_tick && + git -C project commit -m add_sub && + git clone project project-clone && + git -C project-clone worktree add ../project-2 +' +test_expect_success '"add" with uninitialized submodule, with submodule.recurse set' ' + git -C project-clone -c submodule.recurse worktree add ../project-3 +' + +test_expect_success '"add" with initialized submodule, with submodule.recurse unset' ' + git -C project-clone submodule update --init && + git -C project-clone worktree add ../project-4 +' + +test_expect_success '"add" with initialized submodule, with submodule.recurse set' ' + git -C project-clone -c submodule.recurse worktree add ../project-5 +' + test_done -- gitgitgadget ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 0/1] worktree: teach "add" to ignore submodule.recurse config 2019-10-27 17:16 [PATCH 0/1] worktree: teach "add" to ignore submodule.recurse config Philippe Blain via GitGitGadget 2019-10-27 17:16 ` [PATCH 1/1] " Philippe Blain via GitGitGadget @ 2019-10-28 2:26 ` Junio C Hamano 2019-10-29 12:33 ` Philippe Blain 1 sibling, 1 reply; 5+ messages in thread From: Junio C Hamano @ 2019-10-28 2:26 UTC (permalink / raw) To: Philippe Blain via GitGitGadget; +Cc: git "Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com> writes: > 3. I'm on OS X 10.11.6 (Apple clang-800.0.42.1) and I get a warning > compiling builtin/merge.c : > > > CC builtin/merge.o > builtin/merge.c:831:33: warning: data argument not used by format string [-Wformat-extra-args] > no_scissors_editor_comment), comment_line_char); > ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ > builtin/merge.c:809:4: note: format string is defined here > N_("An empty message aborts the commit.\n"); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > ./gettext.h:86:20: note: expanded from macro 'N_' > #define N_(msgid) (msgid) > ^~~~~ > 1 warning generated. I am not sure if the compiler needs fixing in this case, but the following may work it around. builtin/merge.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/builtin/merge.c b/builtin/merge.c index e2ccbc44e2..0746f11df2 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -826,9 +826,12 @@ static void prepare_to_commit(struct commit_list *remoteheads) strbuf_commented_addf(&msg, "\n"); } strbuf_commented_addf(&msg, _(merge_editor_comment)); - strbuf_commented_addf(&msg, _(cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS ? - scissors_editor_comment : - no_scissors_editor_comment), comment_line_char); + + if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) + strbuf_commented_addf(&msg, _(scissors_editor_comment)); + else + strbuf_commented_addf(&msg, _(no_scissors_editor_comment), + comment_line_char); } if (signoff) append_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0); ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 0/1] worktree: teach "add" to ignore submodule.recurse config 2019-10-28 2:26 ` [PATCH 0/1] " Junio C Hamano @ 2019-10-29 12:33 ` Philippe Blain 2019-10-30 1:01 ` Junio C Hamano 0 siblings, 1 reply; 5+ messages in thread From: Philippe Blain @ 2019-10-29 12:33 UTC (permalink / raw) To: Junio C Hamano; +Cc: Philippe Blain via GitGitGadget, git Hi Junio, That indeed makes the trick. Thanks! Should I send a separate patch series with this patch ? How would that work ? "Signed-off by" me and "Based-on-patch-by" you ? Philippe. > Le 27 oct. 2019 à 22:26, Junio C Hamano <gitster@pobox.com> a écrit : > > I am not sure if the compiler needs fixing in this case, but the > following may work it around. > > builtin/merge.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/builtin/merge.c b/builtin/merge.c > index e2ccbc44e2..0746f11df2 100644 > --- a/builtin/merge.c > +++ b/builtin/merge.c > @@ -826,9 +826,12 @@ static void prepare_to_commit(struct commit_list *remoteheads) > strbuf_commented_addf(&msg, "\n"); > } > strbuf_commented_addf(&msg, _(merge_editor_comment)); > - strbuf_commented_addf(&msg, _(cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS ? > - scissors_editor_comment : > - no_scissors_editor_comment), comment_line_char); > + > + if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) > + strbuf_commented_addf(&msg, _(scissors_editor_comment)); > + else > + strbuf_commented_addf(&msg, _(no_scissors_editor_comment), > + comment_line_char); > } > if (signoff) > append_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0); ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 0/1] worktree: teach "add" to ignore submodule.recurse config 2019-10-29 12:33 ` Philippe Blain @ 2019-10-30 1:01 ` Junio C Hamano 0 siblings, 0 replies; 5+ messages in thread From: Junio C Hamano @ 2019-10-30 1:01 UTC (permalink / raw) To: Philippe Blain; +Cc: Philippe Blain via GitGitGadget, git Philippe Blain <philippe.blain@me.com> writes: > Hi Junio, > > That indeed makes the trick. Thanks! Should I send a separate > patch series with this patch ? How would that work ? "Signed-off > by" me and "Based-on-patch-by" you ? As I said, I am not sure if we even want a work-around or would want to just tell the compiler folks to fix their product, so I do not want to even have to decide if I should apply such a patch ;-) But if this were a case where somebody suggested a small diff with "something like this perhaps?" and you wrapped it up as a proper patch (with log message, necessary bugfixes and clean-ups, and possibly tests if needed), just adding "Helped-by: somebody" followed by your s-o-b would be the norm. Anything more extensive you may want to give more credit to the original than helped-by, but in this case I do not think it even deserves that. > > Philippe. > >> Le 27 oct. 2019 à 22:26, Junio C Hamano <gitster@pobox.com> a écrit : >> >> I am not sure if the compiler needs fixing in this case, but the >> following may work it around. >> >> builtin/merge.c | 9 ++++++--- >> 1 file changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/builtin/merge.c b/builtin/merge.c >> index e2ccbc44e2..0746f11df2 100644 >> --- a/builtin/merge.c >> +++ b/builtin/merge.c >> @@ -826,9 +826,12 @@ static void prepare_to_commit(struct commit_list *remoteheads) >> strbuf_commented_addf(&msg, "\n"); >> } >> strbuf_commented_addf(&msg, _(merge_editor_comment)); >> - strbuf_commented_addf(&msg, _(cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS ? >> - scissors_editor_comment : >> - no_scissors_editor_comment), comment_line_char); >> + >> + if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) >> + strbuf_commented_addf(&msg, _(scissors_editor_comment)); >> + else >> + strbuf_commented_addf(&msg, _(no_scissors_editor_comment), >> + comment_line_char); >> } >> if (signoff) >> append_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0); ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-10-30 1:02 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-27 17:16 [PATCH 0/1] worktree: teach "add" to ignore submodule.recurse config Philippe Blain via GitGitGadget 2019-10-27 17:16 ` [PATCH 1/1] " Philippe Blain via GitGitGadget 2019-10-28 2:26 ` [PATCH 0/1] " Junio C Hamano 2019-10-29 12:33 ` Philippe Blain 2019-10-30 1:01 ` Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).