* [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).