subtree: ignore merge.ff setting

[koutcher]

When merge.ff is set to only in .gitconfig, git subtree pull will
fail with error fatal: Not possible to fast-forward, aborting.. This
fix ignores the merge.ff setting when using git merge within subtree.

Signed-off-by: Thomas Koutcher thomas.koutcher@online.fr
cc: Ævar Arnfjörð Bjarmason avarab@gmail.com
cc: Thomas Koutcher thomas.koutcher@online.fr
cc: Johannes Altmanninger aclopte@gmail.com

[koutcher]

/submit

[gitgitgadget-git]

Submitted as pull.1139.git.git.1636902454370.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git pr-git-1139/koutcher/subtree-merge-ff-fix-v1

To fetch this version to local tag pr-git-1139/koutcher/subtree-merge-ff-fix-v1:

git fetch --no-tags https://github.com/gitgitgadget/git tag pr-git-1139/koutcher/subtree-merge-ff-fix-v1

[gitgitgadget-git]

On the Git mailing list, Ævar Arnfjörð Bjarmason wrote (reply to this):

On Sun, Nov 14 2021, Thomas Koutcher via GitGitGadget wrote:

> From: Thomas Koutcher <thomas.koutcher@online.fr>
>
> When `merge.ff` is set to `only` in .gitconfig, `git subtree pull` will
> fail with error `fatal: Not possible to fast-forward, aborting.`. This
> fix ignores the `merge.ff` setting when using `git merge` within subtree.
>
> Signed-off-by: Thomas Koutcher <thomas.koutcher@online.fr>
> ---
>     subtree: ignore merge.ff setting
>     
>     When merge.ff is set to only in .gitconfig, git subtree pull will fail
>     with error fatal: Not possible to fast-forward, aborting.. This fix
>     ignores the merge.ff setting when using git merge within subtree.
>     
>     Signed-off-by: Thomas Koutcher thomas.koutcher@online.fr
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1139%2Fkoutcher%2Fsubtree-merge-ff-fix-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1139/koutcher/subtree-merge-ff-fix-v1
> Pull-Request: https://github.com/git/git/pull/1139
>
>  contrib/subtree/git-subtree.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index 7f767b5c38f..de918d9fb05 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -976,10 +976,10 @@ cmd_merge () {
>  
>  	if test -n "$arg_addmerge_message"
>  	then
> -		git merge -Xsubtree="$arg_prefix" \
> +		git -c merge.ff= merge -Xsubtree="$arg_prefix" \
>  			--message="$arg_addmerge_message" "$rev"
>  	else
> -		git merge -Xsubtree="$arg_prefix" $rev
> +		git -c merge.ff= merge -Xsubtree="$arg_prefix" $rev
>  	fi
>  }

"-c merge.ff=" works, it's lesser known syntax. I'd tihnk "-c
merge.ff=false" would be better here, i.e. what matches "git config"'s
description of "merge.ff".

[gitgitgadget-git]

User Ævar Arnfjörð Bjarmason <avarab@gmail.com> has been added to the cc: list.

[gitgitgadget-git]

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

"Thomas Koutcher via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Thomas Koutcher <thomas.koutcher@online.fr>
>
> When `merge.ff` is set to `only` in .gitconfig, `git subtree pull` will
> fail with error `fatal: Not possible to fast-forward, aborting.`. This
> fix ignores the `merge.ff` setting when using `git merge` within subtree.

The first sentence is understandasble as a statement of fact.  There
is a small logic gap between it and the second sentence, calling the
change in the patch a "fix".  I think ", but the command does want
to make merges in these places." added after the first sentence
would fix it.

> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1139%2Fkoutcher%2Fsubtree-merge-ff-fix-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1139/koutcher/subtree-merge-ff-fix-v1
> Pull-Request: https://github.com/git/git/pull/1139
>
>  contrib/subtree/git-subtree.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index 7f767b5c38f..de918d9fb05 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -976,10 +976,10 @@ cmd_merge () {
>  
>  	if test -n "$arg_addmerge_message"
>  	then
> -		git merge -Xsubtree="$arg_prefix" \
> +		git -c merge.ff= merge -Xsubtree="$arg_prefix" \
>  			--message="$arg_addmerge_message" "$rev"
>  	else
> -		git merge -Xsubtree="$arg_prefix" $rev
> +		git -c merge.ff= merge -Xsubtree="$arg_prefix" $rev

And the natural way to override what is configured is to pass a
countermanding command line option, e.g. "git merge --ff" (or "git
merge --no-ff", if it wants to always create a merge even when
taking a change that is a descendant---I do not know the need of
"git subtree" well enough to tell), and that is easier to read than
"git -c ...".

[koutcher]

Thanks for your reviews Ævar and Junio. I'll update the patch taking your comments into account.

[koutcher]

/preview

[gitgitgadget-git]

Error: Could not determine public email of koutcher

[koutcher]

/preview

[gitgitgadget-git]

Preview email sent as pull.1139.v2.git.git.1636919606686.gitgitgadget@gmail.com

[koutcher]

/preview

[gitgitgadget-git]

Preview email sent as pull.1139.v2.git.git.1636925106468.gitgitgadget@gmail.com

[koutcher]

/submit

[gitgitgadget-git]

Submitted as pull.1139.v2.git.git.1636926322423.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git pr-git-1139/koutcher/subtree-merge-ff-fix-v2

To fetch this version to local tag pr-git-1139/koutcher/subtree-merge-ff-fix-v2:

git fetch --no-tags https://github.com/gitgitgadget/git tag pr-git-1139/koutcher/subtree-merge-ff-fix-v2

[dscho]

Thanks for your reviews Ævar and Junio. I'll update the patch taking your comments into account.

Please note that the mentioned reviewers won't see this reply unless you send it to the Git mailing list as per https://github.com/gitgitgadget/gitgitgadget/wiki/ReplyToThis.

[gitgitgadget-git]

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

"Thomas Koutcher via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Thomas Koutcher <thomas.koutcher@online.fr>
>
> When `merge.ff` is set to `only` in .gitconfig, `git subtree pull` will
> fail with error `fatal: Not possible to fast-forward, aborting.`, but
> the command does want to make merges in these places. Add `--no-ff`
> argument to `git merge` to enforce this behaviour.
>
> Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Reviewed-by: Junio C Hamano <gitster@pobox.com>

Reviewed-by: is a bit tricky.  If you are sending a corrected
version after other people just took a look at a previous iteration,
you cannot generally have them for two reasons.  First of all, they
haven't said that you can have Reviewed-by: under there name (which
means that they are completely satisfied after giving a thorough
analysis).  Also, the version you are sending is different from what
they reviewed, so even if they were happy with the previous iteration,
it does not mean they would be with this version.

> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index 7f767b5c38f..cb51aee4cbf 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -976,10 +976,10 @@ cmd_merge () {
>  
>  	if test -n "$arg_addmerge_message"
>  	then
> -		git merge -Xsubtree="$arg_prefix" \
> +		git merge --no-ff -Xsubtree="$arg_prefix" \
>  			--message="$arg_addmerge_message" "$rev"
>  	else
> -		git merge -Xsubtree="$arg_prefix" $rev
> +		git merge --no-ff -Xsubtree="$arg_prefix" $rev
>  	fi
>  }

Thanks.  I'll drop these two reviewed-by lines while queuing.

[gitgitgadget-git]

On the Git mailing list, Thomas Koutcher wrote (reply to this):

Le 18/11/2021 à 05:53, Junio C Hamano a écrit :

> Reviewed-by: is a bit tricky.  If you are sending a corrected
> version after other people just took a look at a previous iteration,
> you cannot generally have them for two reasons.  First of all, they
> haven't said that you can have Reviewed-by: under there name (which
> means that they are completely satisfied after giving a thorough
> analysis).  Also, the version you are sending is different from what
> they reviewed, so even if they were happy with the previous iteration,
> it does not mean they would be with this version.

Thanks for the clarification and sorry for the confusion I created.

[gitgitgadget-git]

User Thomas Koutcher <thomas.koutcher@online.fr> has been added to the cc: list.

[gitgitgadget-git]

On the Git mailing list, Johannes Altmanninger wrote (reply to this):

From: Thomas Koutcher <thomas.koutcher@online.fr>

When `merge.ff` is set to `only` in .gitconfig, `git subtree pull` will
fail with error `fatal: Not possible to fast-forward, aborting.`, but
the command does want to make merges in these places. Add `--no-ff`
argument to `git merge` to enforce this behaviour.

Signed-off-by: Thomas Koutcher <thomas.koutcher@online.fr>
Reviewed-by: Johannes Altmanninger <aclopte@gmail.com>
---

I think this was meant to be queued but forgotten.

Junio C Hamano <gitster@pobox.com> writes:

> Thanks.  I'll drop these two reviewed-by lines while queuing.

Changes since v2: removed/added reviewed-by trailer

BTW is there a good way to tell "git send-email --in-reply-to"
to prefill "To:" and "Cc:" based on the message I'm replying to?

 contrib/subtree/git-subtree.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 71f1fd94bd..1af1d9653e 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -975,10 +975,10 @@ cmd_merge () {
 
 	if test -n "$arg_addmerge_message"
 	then
-		git merge -Xsubtree="$arg_prefix" \
+		git merge --no-ff -Xsubtree="$arg_prefix" \
 			--message="$arg_addmerge_message" "$rev"
 	else
-		git merge -Xsubtree="$arg_prefix" $rev
+		git merge --no-ff -Xsubtree="$arg_prefix" $rev
 	fi
 }
 
-- 
2.35.0.295.gee0e44bcb6

[gitgitgadget-git]

User Johannes Altmanninger <aclopte@gmail.com> has been added to the cc: list.

[gitgitgadget-git]

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

Johannes Altmanninger <aclopte@gmail.com> writes:

> BTW is there a good way to tell "git send-email --in-reply-to"
> to prefill "To:" and "Cc:" based on the message I'm replying to?

I do not think there is, and I do not think it is readily feasible.
Given a message ID, how would you figure out these two values?
Hardcode the URL of mailing list archive and the rules to find these
values given a message ID?  What if you have a local mail archive
that you'd rather use instead of going to the public internet?

[gitgitgadget-git]

On the Git mailing list, Johannes Altmanninger wrote (reply to this):

On Tue, Feb 01, 2022 at 11:19:45AM -0800, Junio C Hamano wrote:
> Johannes Altmanninger <aclopte@gmail.com> writes:
> 
> > BTW is there a good way to tell "git send-email --in-reply-to"
> > to prefill "To:" and "Cc:" based on the message I'm replying to?
> 
> I do not think there is, and I do not think it is readily feasible.
> Given a message ID, how would you figure out these two values?
> Hardcode the URL of mailing list archive and the rules to find these
> values given a message ID?  What if you have a local mail archive
> that you'd rather use instead of going to the public internet?

The "b4" tool accepts message IDs and allows to configure how to look up
message contents. This is the default:

	[b4]
	# Where to look up threads by message id
	midmask = https://lore.kernel.org/r/%s

b4 has some powerful features but I think I just want something that reads an
email on stdin and outputs the appropriate "send-email --in-reply-to" command.
I'll probably parse the mail headers myself.

[gitgitgadget-git]

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

Johannes Altmanninger <aclopte@gmail.com> writes:

> On Tue, Feb 01, 2022 at 11:19:45AM -0800, Junio C Hamano wrote:
>> Johannes Altmanninger <aclopte@gmail.com> writes:
>> 
>> > BTW is there a good way to tell "git send-email --in-reply-to"
>> > to prefill "To:" and "Cc:" based on the message I'm replying to?
>> 
>> I do not think there is, and I do not think it is readily feasible.
>> Given a message ID, how would you figure out these two values?
>> Hardcode the URL of mailing list archive and the rules to find these
>> values given a message ID?  What if you have a local mail archive
>> that you'd rather use instead of going to the public internet?
>
> The "b4" tool accepts message IDs and allows to configure how to look up
> message contents. This is the default:
>
> 	[b4]
> 	# Where to look up threads by message id
> 	midmask = https://lore.kernel.org/r/%s
>
> b4 has some powerful features but I think I just want something that reads an
> email on stdin and outputs the appropriate "send-email --in-reply-to" command.
> I'll probably parse the mail headers myself.

I know about "b4" and use it myself, but so what?  "git" is used by
a lot wider audience than those who can fetch patches from k.org.

So, I think it still is correct to say that it is not readily
feasible, without telling the command ways to turn a message-ID into
to/cc addresses.

[gitgitgadget-git]

This patch series was integrated into seen via 95d2499.

[gitgitgadget-git]

This branch is now known as tk/subtree-merge-not-ff-only.

[gitgitgadget-git]

This patch series was integrated into seen via 2169875.

[gitgitgadget-git]

This patch series was integrated into seen via 396116c.

[gitgitgadget-git]

There was a status update in the "New Topics" section about the branch tk/subtree-merge-not-ff-only on the Git mailing list:

When "git subtree" wants to create a merge, it used "git merge" and
let it be affected by end-user's "merge.ff" configuration, which
has been corrected.

Will merge to 'next'.
source: <20220201172601.262718-1-aclopte@gmail.com>

[gitgitgadget-git]

This patch series was integrated into seen via f6d4048.

[gitgitgadget-git]

This patch series was integrated into seen via 718e72f.

[gitgitgadget-git]

This patch series was integrated into seen via 8584493.

[gitgitgadget-git]

This patch series was integrated into seen via dac6550.

[gitgitgadget-git]

This patch series was integrated into seen via 9bc5bd4.

[gitgitgadget-git]

This patch series was integrated into next via 8494500.

[gitgitgadget-git]

There was a status update in the "Cooking" section about the branch tk/subtree-merge-not-ff-only on the Git mailing list:

When "git subtree" wants to create a merge, it used "git merge" and
let it be affected by end-user's "merge.ff" configuration, which
has been corrected.

Will merge to 'master'.
source: <20220201172601.262718-1-aclopte@gmail.com>

[gitgitgadget-git]

This patch series was integrated into seen via 08dfed4.

[gitgitgadget-git]

This patch series was integrated into seen via bb08780.

[gitgitgadget-git]

This patch series was integrated into seen via 738109b.

[gitgitgadget-git]

There was a status update in the "Cooking" section about the branch tk/subtree-merge-not-ff-only on the Git mailing list:

When "git subtree" wants to create a merge, it used "git merge" and
let it be affected by end-user's "merge.ff" configuration, which
has been corrected.

Will merge to 'master'.
source: <20220201172601.262718-1-aclopte@gmail.com>

[gitgitgadget-git]

This patch series was integrated into seen via 9109620.

[gitgitgadget-git]

This patch series was integrated into seen via 812f72d.

[gitgitgadget-git]

This patch series was integrated into seen via 0ac270c.

[gitgitgadget-git]

This patch series was integrated into master via 0ac270c.

[gitgitgadget-git]

This patch series was integrated into next via 0ac270c.

[gitgitgadget-git]

Closed via 0ac270c.