Bug 380314 - Recursive merge strategy feature
Summary: Recursive merge strategy feature
Status: RESOLVED FIXED
Alias: None
Product: JGit
Classification: Technology
Component: JGit (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P3 enhancement with 5 votes (vote)
Target Milestone: 3.0   Edit
Assignee: George Young CLA
QA Contact:
URL:
Whiteboard:
Keywords: noteworthy
: 353876 400111 (view as bug list)
Depends on:
Blocks: 389413
  Show dependency tree
 
Reported: 2012-05-22 14:45 EDT by George Young CLA
Modified: 2013-10-17 06:53 EDT (History)
16 users (show)

See Also:


Attachments
Recursive Merge Strategy proposal as patch to master@bc66934a8317f4 (111.02 KB, application/octet-stream)
2012-10-05 12:06 EDT, George Young CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description George Young CLA 2012-05-22 14:45:05 EDT
Build Identifier: Eclipse Java EE IDE for Web Developers.  Version: Indigo Service Release 1 Build id: 20110916-0149

JGit should have the ability to merge two commits when multiple merge bases exist, just as CGit. The JGit java architecture provides for addition merge strategies, so adding a new ThreeWayMerger subclass of ResolveMerger called RecursiveMerge, and an accompanying StrategyRecursive should be sufficient. The approximately 1300 lines of CGit code that implement the recursive algorithm should boil down to much less java code as much of the CGit code is preoccupied with extracting hashes, while much of the JGit code already has developed these utilities, leaving the algorithms missing.

Reproducible: Always

Steps to Reproduce:
1.Multiple merge base
2.jgit merge --strategy=resolve
3.fails:
jgit merge --strategy=resolve prod-LCL
org.eclipse.jgit.api.errors.JGitInternalException: Exception caught during execution of merge command. java.io.IOException: Multiple merge bases for:
  5e406881fb0ccf2325020540bed16977944674a6
  adc77966d11a622be35ec4413ad6e056cc4bd9bd found:
  020fc5bb8e0faa53f836ade4489a0a78e61b3d7f
  dd156b6d0ebbaf27cbd66de89d8de7e51376ba38
        at org.eclipse.jgit.api.MergeCommand.call(MergeCommand.java:260)
        at org.eclipse.jgit.pgm.Merge.run(Merge.java:85)
        at org.eclipse.jgit.pgm.TextBuiltin.execute(TextBuiltin.java:148)
        at org.eclipse.jgit.pgm.Main.execute(Main.java:191)
        at org.eclipse.jgit.pgm.Main.run(Main.java:120)
        at org.eclipse.jgit.pgm.Main.main(Main.java:94)
Caused by: java.io.IOException: Multiple merge bases for:
  5e406881fb0ccf2325020540bed16977944674a6
  adc77966d11a622be35ec4413ad6e056cc4bd9bd found:
  020fc5bb8e0faa53f836ade4489a0a78e61b3d7f
  dd156b6d0ebbaf27cbd66de89d8de7e51376ba38
        at org.eclipse.jgit.merge.Merger.getBaseCommit(Merger.java:217)
        at org.eclipse.jgit.merge.Merger.mergeBase(Merger.java:182)
        at org.eclipse.jgit.merge.ThreeWayMerger.mergeBase(ThreeWayMerger.java:121)
        at org.eclipse.jgit.merge.ResolveMerger.mergeImpl(ResolveMerger.java:187)
        at org.eclipse.jgit.merge.Merger.merge(Merger.java:156)
        at org.eclipse.jgit.merge.ThreeWayMerger.merge(ThreeWayMerger.java:108)
        at org.eclipse.jgit.api.MergeCommand.call(MergeCommand.java:208)
        ... 5 more

using test repo attached to https://bugs.eclipse.org/bugs/show_bug.cgi?id=353876

tested using jgit version:
jgit version 1.3.0-SNAPSHOT f70b24a
Comment 1 Robin Stocker CLA 2012-08-28 12:01:17 EDT
*** Bug 353876 has been marked as a duplicate of this bug. ***
Comment 2 George Young CLA 2012-08-28 23:29:15 EDT
Have created a version of a Recursive merge strategy against JGit 1.3.1 to use with an older Gerrit installation. Will need another month before publishing a patch for review against the current "tip" of master. Will be able to jgit merge --strategy=recursive or the equivalent using the java API.

If anyone would like I could deploy. Is there a public maven repo that could accept a side verion 1.3.1-RP?
Comment 3 Robin Stocker CLA 2012-08-29 06:42:24 EDT
The patch would be more interesting.
Comment 4 Philipp Marx CLA 2012-09-06 09:07:03 EDT
George: Did you upload your MergeStrategy somewhere? I am currently facing this issue and would like to check if your solution can resolve my problem.

Thanks.
Comment 5 George Young CLA 2012-09-06 09:27:32 EDT
Should be releasing this soon. Have 3 other inhouse items to integrate first.
Comment 6 Philipp Marx CLA 2012-10-05 04:24:04 EDT
Hi George,

any updates on this? :-) Would it be feasible for you to show me the sources of your solution up front? The issue got really worse in our system and we need to decide whether we have to rewrite some basic parts or if we can wait for a fix for this issue.

Thanks a lot.
Best,
Philipp

(In reply to comment #5)
> Should be releasing this soon. Have 3 other inhouse items to integrate first.
Comment 7 George Young CLA 2012-10-05 12:06:23 EDT
Created attachment 221963 [details]
Recursive Merge Strategy proposal as patch to master@bc66934a8317f4
Comment 8 George Young CLA 2012-10-05 12:15:21 EDT
You may not want the .launch and .prefs files, just .properties and .java

The existing test code fails when I run on Cygwin, so I need a better set-up.

An earlier version of this code is based on v1.3.0.201202151440-r.75-gff13648 and is running under our customised Gerrit which optionally replaces 
 MergeStrategy.RESOLVE.newMerger
with
 MergeStrategy.RECURSIVE.newMerger
Comment 9 George Young CLA 2012-10-09 16:32:01 EDT
Comment on attachment 221963 [details]
Recursive Merge Strategy proposal as patch to master@bc66934a8317f4

When would the HEAD of master branch be patched with this? Does a maintainer first test it or do voters?
Comment 10 Chris Aniszczyk CLA 2012-10-09 16:45:08 EDT
(In reply to comment #9)
> Comment on attachment 221963 [details]
> Recursive Merge Strategy proposal as patch to master@bc66934a8317f4
> 
> When would the HEAD of master branch be patched with this? Does a maintainer
> first test it or do voters?

HEAD is the right place.

Is it possible for to follow the contributor guide?
http://wiki.eclipse.org/EGit/Contributor_Guide#Contributing_Patches

Essentially, please sign up for Gerrit and then push the patch to Gerrit. That would be the quickest way to get the patch in.

Are there any tests associated with this patch too?
Comment 11 Chris Aniszczyk CLA 2012-10-09 17:02:24 EDT
Feel free to ping me over gchat if you have issues George

caniszczyk
Comment 12 Chris Aniszczyk CLA 2012-10-30 19:59:35 EDT
This is currently being worked on here:

https://git.eclipse.org/r/#/c/8113/
Comment 13 Philipp Marx CLA 2012-10-31 09:11:50 EDT
Hi George,

I have successfully patched your code into our project and it works pretty well. Almost all of the "multiple merge bases" are gone but a few. I will continue investigating why they appear.

Thanks a lot :)

Cheers
Philipp
Comment 14 Chris Aniszczyk CLA 2012-11-01 00:40:57 EDT
George, can you verify that you:

- authored 100% the content they are contributing
- have the rights to donate the content to Eclipse Foundation
- contribute the content under the EDL
Comment 15 Chris Aniszczyk CLA 2012-11-02 13:36:17 EDT
CQ opened: http://dev.eclipse.org/ipzilla/show_bug.cgi?id=6854

Now we wait for IP team..
Comment 16 Robin Rosenberg CLA 2013-01-07 19:36:05 EST
*** Bug 359951 has been marked as a duplicate of this bug. ***
Comment 17 Robin Rosenberg CLA 2013-01-07 19:36:46 EST
*** Bug 372606 has been marked as a duplicate of this bug. ***
Comment 18 Matthias Sohn CLA 2013-02-07 03:43:23 EST
*** Bug 400111 has been marked as a duplicate of this bug. ***
Comment 19 Chris Aniszczyk CLA 2013-02-21 10:30:54 EST
The CQ is finally approved, we should be able to get this in for 2.4 now :)
Comment 20 Matthias Sohn CLA 2013-02-21 17:11:25 EST
(In reply to comment #19)
> The CQ is finally approved, we should be able to get this in for 2.4 now :)

at the moment hudson is broken due to a hardware failure so this has to wait a bit
Comment 21 Chris Aniszczyk CLA 2013-02-22 17:55:37 EST
Fixed in master: ab99b78ca08a6b52e9ae8b49afa04dd16496f2ac

Thanks everyone for being patient.
Comment 22 Robin Stocker CLA 2013-04-24 05:31:57 EDT
FYI: Recursive was made the default strategy (for merge, pull) by Matthias here:

http://git.eclipse.org/c/jgit/jgit.git/commit/?id=aa7be667bcca4bdb28b2485e28a05da54c431df7
Comment 23 Robin Stocker CLA 2013-10-17 06:53:00 EDT
There is a problem when a recursive merge has conflicts, see bug 419641.