worktree: teach "add" to ignore submodule.recurse config

[phil-blain]

"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.

[gitgitgadget]

Welcome to GitGitGadget

Hi @phil-blain, and welcome to GitGitGadget, the GitHub App to send patch series to the Git mailing list from GitHub Pull Requests.

Please make sure that this Pull Request has a good description, as it will be used as cover letter.

Also, it is a good idea to review the commit messages one last time, as the Git project expects them in a quite specific form:

• the lines should not exceed 76 columns,
• the first line should be like a header and typically start with a prefix like "tests:" or "commit:", and
• the commit messages' body should be describing the "why?" of the change.
• Finally, the commit messages should end in a Signed-off-by: line matching the commits' author.

It is in general a good idea to await the automated test ("Checks") in this Pull Request before contributing the patches, e.g. to avoid trivial issues such as unportable code.

Contributing the patches

Before you can contribute the patches, your GitHub username needs to be added to the list of permitted users. Any already-permitted user can do that, by adding a PR comment of the form /allow <username>.

Once on the list of permitted usernames, you can contribute the patches to the Git mailing list by adding a PR comment /submit.

After you submit, GitGitGadget will respond with another comment that contains the link to the cover letter mail in the Git mailing list archive. Please make sure to monitor the discussion in that thread and to address comments and suggestions.

If you want to see what email(s) would be sent for a submit request, add a PR comment /preview to have the email(s) sent to you. You must have a public GitHub email address for this.

If you do not want to subscribe to the Git mailing list just to be able to respond to a mail, you can download the mbox ("raw") file corresponding to the mail you want to reply to from the Git mailing list. If you use GMail, you can upload that raw mbox file via:

curl -g --user "<EMailAddress>:<Password>" --url "imaps://imap.gmail.com/INBOX" -T /path/to/raw.txt

[dscho]

/allow

[gitgitgadget]

User phil-blain is now allowed to use GitGitGadget.

WARNING: phil-blain has no public email address set on GitHub

[phil-blain]

thanks @dscho !

[phil-blain]

/submit

[gitgitgadget]

Submitted as pull.430.git.1572196585.gitgitgadget@gmail.com

WARNING: phil-blain has no public email address set on GitHub

[gitgitgadget]

On the Git mailing list, Junio C Hamano wrote (reply to this):

"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);

[dscho]

@phil-blain the custom on the Git mailing list is to reply to reviewer comments, either dismissing their suggestion (backed up by good arguments, of course) or agreeing with them, and (if needed) amending the patch, force-pushing, and sending another iteration via /submit.

I tend to agree with Junio, this would make for a good patch to fix your compile problem, but since it is not even in code touched by this here PR, I would suggest to open another Pull Request.

[phil-blain]

I will! I’m just learning to use git am and only have a few minutes each day to contribute. I was going to respond this morning asking if I should apply the patch to this branch or a new one.

[gitgitgadget]

On the Git mailing list, Philippe Blain wrote (reply to this):

Hi Junio,

That indeed makes the trick. Thanks!=20
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 =C3=A0 22:26, Junio C Hamano <gitster@pobox.com> a =
=C3=A9crit :
>=20
> I am not sure if the compiler needs fixing in this case, but the
> following may work it around.
>=20
> builtin/merge.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>=20
> 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 =3D=3D =
COMMIT_MSG_CLEANUP_SCISSORS ?
> -			scissors_editor_comment :
> -			no_scissors_editor_comment), comment_line_char);
> +
> +		if (cleanup_mode =3D=3D 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);

[gitgitgadget]

On the Git mailing list, Junio C Hamano wrote (reply to this):

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 =C3=A0 22:26, Junio C Hamano <gitster@pobox.com> a =C3=
=A9crit :
>>=20
>> I am not sure if the compiler needs fixing in this case, but the
>> following may work it around.
>>=20
>> builtin/merge.c | 9 ++++++---
>> 1 file changed, 6 insertions(+), 3 deletions(-)
>>=20
>> 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 =3D=3D COMMIT_MSG_CLEANU=
P_SCISSORS ?
>> -			scissors_editor_comment :
>> -			no_scissors_editor_comment), comment_line_char);
>> +
>> +		if (cleanup_mode =3D=3D 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);

[gitgitgadget]

This branch is now known as pb/no-recursive-reset-hard-in-worktree-add.

[gitgitgadget]

This patch series was integrated into pu via git@bea3667.

[gitgitgadget]

This patch series was integrated into pu via git@2f5bc80.

[gitgitgadget]

This patch series was integrated into pu via git@1054950.

[gitgitgadget]

This patch series was integrated into pu via git@dbfeddb.

[gitgitgadget]

This patch series was integrated into pu via git@161431e.

[gitgitgadget]

This patch series was integrated into pu via git@5637b78.

[gitgitgadget]

This patch series was integrated into pu via git@b8655bb.

[gitgitgadget]

This patch series was integrated into pu via git@80e4bbd.

[gitgitgadget]

This patch series was integrated into pu via git@90b8030.

[gitgitgadget]

This patch series was integrated into pu via git@f318d8b.

[gitgitgadget]

This patch series was integrated into pu via git@cbbc788.

[gitgitgadget]

This patch series was integrated into next via git@cdfb064.

[gitgitgadget]

This patch series was integrated into pu via git@b6d1189.

[gitgitgadget]

This patch series was integrated into pu via git@aa8a208.

[gitgitgadget]

This patch series was integrated into pu via git@dd75d42.

[gitgitgadget]

This patch series was integrated into pu via git@a8678ce.

[gitgitgadget]

This patch series was integrated into pu via git@fb7f171.

[gitgitgadget]

This patch series was integrated into pu via git@05fc647.

[gitgitgadget]

This patch series was integrated into next via git@05fc647.

[gitgitgadget]

This patch series was integrated into master via git@05fc647.

[gitgitgadget]

Closed via 05fc647.