[1.7.9] usage regression when merging annotated tag objects

[1.7.9] usage regression when merging annotated tag objects

[Bart Trojanowski]

I recently started using git 1.7.9.  Earlier today GregKH released a stable
kernel update and I tried my tried and true procedure using 'git merge
--ff-only v3.2.3'.  I was a bit surprised with the results.

There are two tags I am toying with...

69bade0 is v3.2.3
3499d64 is v3.2.2

And here is where we start...

$ git describe
v3.2.2

$ git merge-base v3.2.2 v3.2.3 | xargs git describe
v3.2.2

(it is thus eligible for a fast-forward)

Finally, the strangeness...

$ git merge --ff-only v3.2.3
fatal: Not possible to fast-forward, aborting.

$ git merge --ff-only v3.2.3~
Updating 3499d64..7b171c5
Fast-forward
...

$ git merge --ff-only v3.2.3
fatal: Not possible to fast-forward, aborting.

After talking to Junio, he pointed out that "merging tag objects gained new
meanings in 1.7.9".

I am not sure if my confusion will be shared by others and if --ff-only
needs a clarification.  Perhaps --ff-only should just continue to work as
it did before, and fast-forward from tag to tag.

Cheers,
-Bart

Re: [1.7.9] usage regression when merging annotated tag objects

[Junio C Hamano]

Some examples you gave were irrelevant, so I'd give an updated version.

Here are the facts of the day, without judging if the behaviour is good or
bad.

   1) If you are at Linux v3.2.2 and do not have any development on top, 

        $ git merge v3.2.3

      historically would have fast-forwarded. Git v1.7.9 would now create a
      merge commit, authored by you (who is unlikely to be Linus).

   2) You do not want such a merge, so try to work it around by this:

        $ git merge --ff-only v3.2.3
	fatal: Not possible to fast-forward, aborting.

      which is refused because merging a tag object requires a new merge
      commit.

Here are my assessments.

1. I do not think the first one is a real issue. 99% of the people who are
   merely following along the upstream will never say "git merge v3.2.3".
   They will instead say "git pull" and this _will_ fast-forward.  No
   merging of tag objects involved.  Also when they want to check out that
   specific version, they won't be using "git merge".  It will be "git
   checkout v3.2.3".  So I do not think this is an issue for the case
   where it used to result in a fast-forward.

1.5 A variant of the first one is when you have forked and are trying to
   synchronize with the latest stable. In that case, you _do_ want a merge
   to happen. It is possible that you may not want to get the "mergetag"
   header in the resulting merge commit, and "git merge v3.2.3^0" is a new
   way to do so.

   Strictly speaking, this _is_ a usage regression caused by the new
   meaning "git merge" gained in v1.7.9.  Recording the tag in the a merge
   commit, however, is the whole point of "git merge v3.2.3" that is given
   a tag; this behaviour is not going to to change.

2. This is somewhat problematic. "git merge --ff-only v2.6.29" to people
   who merely follow Linus has always been possible, and I would expect it
   to be the case.

   But again, the reason they said --ff-only in the first place is because
   they feared that they might have some unexpected commits in their
   history, and asked "git merge" to error out if the command has to
   create a merge to let them know.  So at that point, they could be
   trained to run "git merge --ff-only v3.2.3^0" instead, *given enough
   clue*.

   The problem is that we are not giving enough clue.  We just say "Not
   possible to fast-forward" without explaining why.

   We could solve this in one of two ways. We could tell them to merge
   v3.2.3^0 instead. Or we could just go ahead and do that for them
   automatically ourselves.  I am inclined to say that we should unwrap
   the tag given from the command line when --ff-only was given, i.e. we
   do the latter.

merge: do not create a signed tag merge under --ff-only option

[Junio C Hamano]

Starting at release v1.7.9, if you ask to merge a signed tag, "git merge"
always creates a merge commit, even when the tag points at a commit that
happens to be a descendant of your current commit.

Unfortunately, this interacts rather badly for people who use --ff-only to
make sure that their branch is free of local developments. It used to be
possible to say:

	$ git checkout -b frotz v1.7.9~30
        $ git merge --ff-only v1.7.9

and expect that the resulting tip of frotz branch matches v1.7.9^0 (aka
the commit tagged as v1.7.9), but this fails with the updated Git with:

	fatal: Not possible to fast-forward, aborting.

because a merge that merges v1.7.9 tag to v1.7.9~30 cannot be created by
fast forwarding.

We could teach users that now they have to do

	$ git merge --ff-only v1.7.9^0

but it is far more pleasant for users if we DWIMmed this ourselves.

When an integrator pulls in a topic from a lieutenant via a signed tag,
even when the work done by the lieutenant happens to fast-forward, the
integrator wants to have a merge record, so the integrator will not be
asking for --ff-only when running "git pull" in such a case. Therefore,
this change should not regress the support for the use case v1.7.9 wanted
to add.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

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

    We could solve this in one of two ways. We could tell them to merge
    v3.2.3^0 instead. Or we could just go ahead and do that for them
    automatically ourselves.  I am inclined to say that we should unwrap
    the tag given from the command line when --ff-only was given, i.e. we
    do the latter.

  And it turns out that it is just a single-liner patch.

 builtin/merge.c  |    3 ++-
 t/t7600-merge.sh |   13 +++++++++++++
 2 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 3a45172..b4fbc60 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1283,7 +1283,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 			    sha1_to_hex(commit->object.sha1));
 		setenv(buf.buf, argv[i], 1);
 		strbuf_reset(&buf);
-		if (merge_remote_util(commit) &&
+		if (!fast_forward_only &&
+		    merge_remote_util(commit) &&
 		    merge_remote_util(commit)->obj &&
 		    merge_remote_util(commit)->obj->type == OBJ_TAG) {
 			option_edit = 1;
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 5d8c428..a598dfa 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -27,6 +27,7 @@ Testing basic merge operations/option parsing.
 '
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-gpg.sh
 
 printf '%s\n' 1 2 3 4 5 6 7 8 9 >file
 printf '%s\n' '1 X' 2 3 4 5 6 7 8 9 >file.1
@@ -670,4 +671,16 @@ test_expect_success 'merge --no-ff --edit' '
 	test_cmp actual expected
 '
 
+test_expect_success GPG 'merge --ff-only tag' '
+	git reset --hard c0 &&
+	git commit --allow-empty -m "A newer commit" &&
+	git tag -s -m "A newer commit" signed &&
+	git reset --hard c0 &&
+
+	git merge --ff-only signed &&
+	git rev-parse signed^0 >expect &&
+	git rev-parse HEAD >actual &&
+	test_cmp actual expect
+'
+
 test_done