# git merge (resolve) _is_ stupid 5 messages from 2006-07-31 to 2006-07-31. Participants: Junio C Hamano, Johannes Schindelin, Luben Tuikov. Thread: https://gitlist.dev/t/5056 ## Junio C Hamano, 2006-07-31 08:44 Subject: git merge (resolve) _is_ stupid Message-ID: <7v7j1u88ol.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7v7j1u88ol.fsf%40assigned-by-dhcp.cox.net ``` 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(-) ``` ## Johannes Schindelin, 2006-07-31 10:42 Subject: Re: git merge (resolve) _is_ stupid Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0607311236070.29667%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <7v7j1u88ol.fsf@assigned-by-dhcp.cox.net> ``` Hi, On Mon, 31 Jul 2006, Junio C Hamano wrote: > 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 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 --- merge-recursive.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index 10bce70..74a329f 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -43,6 +43,8 @@ static struct commit *make_virtual_commi commit->tree = tree; commit->util = (void*)comment; *(int*)commit->object.sha1 = virtual_id++; + /* avoid warnings */ + commit->object.parsed = 1; return commit; } -- 1.4.2.rc2.gfd00-dirty ``` ## Junio C Hamano, 2006-07-31 11:01 Subject: Re: git merge (resolve) _is_ stupid Message-ID: <7v7j1u6nsx.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7v7j1u6nsx.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` Johannes Schindelin writes: > The culprit is the call to parse_commit() in merge_bases(). How about > this? 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. ``` ## Johannes Schindelin, 2006-07-31 11:13 Subject: Re: git merge (resolve) _is_ stupid Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0607311307350.29667%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <7v7j1u6nsx.fsf@assigned-by-dhcp.cox.net> ``` Hi, On Mon, 31 Jul 2006, Junio C Hamano wrote: > Johannes Schindelin writes: > > > The culprit is the call to parse_commit() in merge_bases(). How about > > this? > > Do you mean merge_bases() in commit.c which is called by > get_merge_bases()? Yes, the same. > 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? 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 am confused, but going to bed first. I know the feeling ;-) Ciao, Dscho ``` ## Luben Tuikov, 2006-07-31 17:12 Subject: Re: git merge (resolve) _is_ stupid Message-ID: <20060731171229.24870.qmail@web31802.mail.mud.yahoo.com> URL: https://gitlist.dev/e/20060731171229.24870.qmail%40web31802.mail.mud.yahoo.com In-Reply-To: <7v7j1u88ol.fsf@assigned-by-dhcp.cox.net> ``` 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 ```