Re: Unexpected or wrong ff, no-ff and ff-only behaviour

[Elijah Newren <newren@gmail.com>]

On Fri, Jul 12, 2019 at 6:50 AM Sergey Organov <sorganov@gmail.com> wrote:
>
> That said, even if we rather all do agree rebase workflow is always
> inferior to merge one, is it satisfactory excuse to actively resist
> otherwise logical behavior of 'git merge' that is even documented? I
> don't think so.
>
> Thus, one way or another, I'd still vote for keeping options description
> intact, and rather fix the implementation to match the manual, i.e.,
> make the --[no]-ff actually orthogonal to --ff-only.

This seems like it's asking for trouble.  --ff-only and --no-ff,
ignoring any knowledge of what they do, are contradictory by their
names.  Attempting to explain to users that they are not opposites but
can be used together compatibly sounds very problematic to me.

But let me dig a little deeper into your statement of "resisting
otherwise logical behavior of 'git merge" that is even documented."
Going back to when the --ff-only option was introduced:

commit 134748353b2a71a34f899c9b1326ccf7ae082412
Author: Björn Gustavsson <bgustavsson@gmail.com>
Date:   Thu Oct 29 23:08:31 2009 +0100

    Teach 'git merge' and 'git pull' the option --ff-only
   
    For convenience in scripts and aliases, add the option
    --ff-only to only allow fast-forwards (and up-to-date,
    despite the name).
   
    Disallow combining --ff-only and --no-ff, since they
    flatly contradict each other.

We see that the original author even stated pretty clearly and
strongly that these were polar opposite options.  Our description
today of --ff-only is the same one he introduced in that patch, and I
always read the description the same way I think he did: that it
implied a polar opposite of --no-ff.  You and Bryan came along and
pointed out that the description was actually ambiguous and could be
interpreted a different way...and I then had to re-read the text a
time or two after you pointed it out to see the alternate reading.  I
totally admit that the wording is apparently ambiguous and can be read
the way you have, but I don't at all see that as a reason to keep the
documentation as-is.  Either it should be clarified to rule out what
you and usbuser wanted and perhaps misunderstood it to do, or if the
behavior is changed then it should be clarified so that folks like me
don't read it to behave as it has since Björn added it.

Personally, I think that if new functionality is wanted, it should
definitely go in a different flag (perhaps someone can find something
shorter than --no-ff-and-only-if-ff-is-possible).  But I think Junio
makes a pretty good argument to be leery of such a new option, and
Brian points out how to easily get this behavior by just stringing
together a merge-base command with merge --no-ff.

That's just my $0.02, but since this is the second time in the thread
that I've suggested improving the documentation, here's a patch to do
so:


-- 8< --
Subject: [PATCH] merge-options.txt: clarify meaning of various ff-related
 options

As discovered on the mailing list, some of the descriptions of the
ff-related options were unclear.  Try to be more precise with what these
options do.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/merge-options.txt | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 79a00d2a4a..e888c99d48 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -40,20 +40,21 @@ set to `no` at the beginning of them.
 	case of a merge conflict.
 
 --ff::
-	When the merge resolves as a fast-forward, only update the branch
-	pointer, without creating a merge commit.  This is the default
-	behavior.
+	When the merge resolves as a fast-forward, only update the
+	branch pointer (without creating a merge commit).  When a fast
+	forward update is not possible, create a merge commit.  This is
+	the default behavior.
 
 --no-ff::
-	Create a merge commit even when the merge resolves as a
-	fast-forward.  This is the default behaviour when merging an
-	annotated (and possibly signed) tag that is not stored in
-	its natural place in 'refs/tags/' hierarchy.
+	Create a merge commit even when the merge could instead resolve
+	as a fast-forward.  This is the default behaviour when merging
+	an annotated (and possibly signed) tag that is not stored in its
+	natural place in 'refs/tags/' hierarchy.
 
 --ff-only::
-	Refuse to merge and exit with a non-zero status unless the
-	current `HEAD` is already up to date or the merge can be
-	resolved as a fast-forward.
+	When possible, resolve the merge as a fast-forward (do not
+	create a merge commit).  When not possible, refuse to merge and
+	exit with a non-zero status.
 
 -S[<keyid>]::
 --gpg-sign[=<keyid>]::
-- 
2.22.0.429.gbb0a79e5f4