git merge (resolve) _is_ stupid

While dealing with Jakub's gitweb changes, I found that merge we use _is_ very stupid. Perhaps it is not a surprise...

My "next" was at 688a750, and "master" was at b63fafd. I had already merged gitweb changes from Luben to "master" at this point, and "next" was in sync with "master" with respect to gitweb/, i.e. this gave empty:

	$ git diff master next gitweb/

Jakub's gitweb changes were stored in jn/web branch which contained 16 patches on top of "master".

However, pulling "jn/web" (594e212) into "next" using resolve strategy produced interesting conflicts. Essentially, it said that some changes done by Luben (which has already merged to "master" and precedes what Jakub did in "jn/web") conflict with what "jn/web" does, which is quite bogus.

This merge has three merge-bases, but

	$ git log master..next -- gitweb/

shows only two merges, and that is not so surprising given that at this point gitweb/ files are identical between master and next.

More interestingly, if I merge "master" to "next" first and then merge "jn/web" on top of the result, the resolve strategy does the right thing. This is understandable -- by merging "master" into "next", merge base between "next" and "jn/web" becomes "master" and nothing else.

Using recursive strategy resolved this merge correctly, taking gitweb/ files from "jn/web" branch.

By the way, the "recur" strategy in "next" produces the correct result, but it produces a funny error in the middle (that is why Johannes is CC'ed).

	error: Could not read 0100000000000000000000000000000000000000

: gitster; git merge -s recur 'test merge' next jn/web
Merging next with 594e212bc849039a204deef1d16c2eddcc451532
Merging:
688a75071490101dbc660e3304aafb7a13e28807 Merge branch '__/setup-n-mv' into next
594e212bc849039a204deef1d16c2eddcc451532 gitweb: Ref refactoring - use git_get_referencing for marking tagged/head commits
found 3 common ancestor(s):
7061cf0f205e86613c3a3306fdfedf2a5dcc8a65 Merge branch 'lt/setup' into __/setup-n-mv
acb0f6f33760b43c1fc9617a45346ab3738f021a gitweb.cgi: git_blame2: slight optimization reading the blame lines
2d023581c9a0ae5efdebfd0084d54d09669a25d5 Set datarootdir in config.mak.in
  Merging:
  7061cf0f205e86613c3a3306fdfedf2a5dcc8a65 Merge branch 'lt/setup' into __/setup-n-mv
  acb0f6f33760b43c1fc9617a45346ab3738f021a gitweb.cgi: git_blame2: slight optimization reading the blame lines
  found 1 common ancestor(s):
  634331061599a82968daddae2d2c0896b6137d4c gitweb.cgi: Centralize printing of the page path
  Auto-merging gitweb/gitweb.cgi
  Merging:
  virtual merged tree
  2d023581c9a0ae5efdebfd0084d54d09669a25d5 Set datarootdir in config.mak.in
error: Could not read 0100000000000000000000000000000000000000
  found 1 common ancestor(s):
  7b8cf0cf2973cc8df3bdd36b9b36542b1f04d70a Rename man1 and man7 variables to man1dir and man7dir
  Auto-merging .gitignore
  Auto-merging INSTALL
  Auto-merging Makefile
Auto-merging Makefile
Merge made by recur.
 Documentation/git-tar-tree.txt |    5 
 Makefile                       |    3 
 git.c                          |   76 ++---
 gitweb/gitweb.cgi              |  609 +++++++++++++++++-----------------------
 setup.c                        |    4 
 5 files changed, 300 insertions(+), 397 deletions(-)

Hi,

On Mon, 31 Jul 2006, Junio C Hamano wrote:

I get "0000000100..." ;-) (surprixse her wi1h a big-endian ;-)

The culprit is the call to parse_commit() in merge_bases(). How about this?

-- 8< -- [PATCH] merge-recur: virtual commits shall never be parsed

It would not make sense to parse a virtual commit, therefore set the "parsed" flag to 1.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
 merge-recursive.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Do you mean merge_bases() in commit.c which is called by get_merge_bases()? If so the patch feels like papering over a more grave bug -- the result from make_virtual_commit does not seem to have any proper parent information, so how is merge_bases() expected to return anything sensible?

I am confused, but going to bed first.

Hi,

On Mon, 31 Jul 2006, Junio C Hamano wrote:

Yes, the same.

It is not a grave bug, but very much by design: remember, a recursive merge means that if you have more than one merge base, then the merge bases are merged first. And the result is -- tada -- a virtual commit.

It does have (virtual) parents, but we do _not_ want to traverse them in merge_bases().

However, they have (virtual) children, and _these_ relationships are important: in your case (I guess) that the merge_bases() finds the virtual commit, since (with the virtual history) the other commit is just a fast-forward.

So, a virtual commit is something like a merge base combining two or more merge bases.

I know the feeling ;-)

Ciao, Dscho

I had seen something similar:

Merging linux-scsi into my own branches whereby both had a particular patch (03aba2f79594ca94d159c8bab454de9bcc385b76), git-merge complained that a resolve is needed.

After manually "resolving" the result was identical to either branch (linux-scsi and my own), so in fact the diff was 0.

I found this strange, but due to lack of time couldn't pursue it further.

    Luben