Introduce `Repository.MergeCommits` #990

ethomson · 2015-03-11T18:47:05Z

This introduces Repository.MergeCommits which wraps git_merge_commits and is suitable for taking two commits and getting the merge results as an Index. There's a handful of interesting details here:

The conflict data previously had a pointer to the Repository. This meant that they always went to the repository's Index. I broke this and pass in the Index that they're supposed to pay attention to. This is probably the least contentious change.
Since the resultant index needs to be freed with git_index_free, I made Index implement IDisposable so that consumers of Repository.MergeCommits can just wrap that in a using. To prevent anybody from trying to Dispose the Repository.Index, I only wired up Dispose for things that came from a merge.
I refactored MergeOptions, CherryPickOptions and RevertOptions to share a common base class. They were getting a little copy-pastey. I also pulled out the git_merge_options-like things into their own base class so that MergeTreeOptions could also take advantage.

I suspect there's going to be something in here for everybody to hate! :) Other suggestions for how to implement these things are welcome.

nulltoken · 2015-03-11T18:57:10Z

LibGit2Sharp/Repository.cs

+        /// <param name="theirs">The second tree</param>
+        /// <param name="options">The <see cref="MergeTreeOptions"/> controlling the merge</param>
+        /// <returns>The <see cref="Index"/> containing the merged trees and any conflicts</returns>
+        public Index MergeCommits(Commit ours, Commit theirs, MergeTreeOptions options)


Couldn't we move this to repo.ObjectDatabase?

Sure. Do you like ObjectDatabase.MergeCommits?

However, I'm not sure about the returned parameter type. I wonder if we should return a different type (an interface?).

How would this* index behave when being added/removed anything? Does git_index_write behave nicely?

Yeah, I think so as well. Anything that hits UpdatePhysicalIndex is going to throw. I think perhaps something that wraps an index handle but is not an index might be appropriate. I think that you want to be able to:

Get Conflicts

Write it to a tree

...other things?

ReadOnlyIndex?

ethomson · 2015-03-19T19:49:09Z

@nulltoken did you have any thoughts on my question(s) about what to call this interface / concrete implementation and / or what additional things it should expose?

nulltoken · 2015-03-22T14:27:26Z

How about something like

MergeTreeResult ObjectDatabase.MergeTree(Commit one, Commit another, MergeTreeOptions options)

Where MergeTreeResult would expose a status (Success, Conflicted) and either a resulting Tree or a list of conflicts?

ethomson · 2015-03-24T02:46:36Z

I have updated this with some changes.

nulltoken · 2015-03-31T09:35:40Z

Neat!

Could you please update the commit message of Introduce ObjectDatabase.MergeCommits so that it doesn't refer to the Index any longer?

nulltoken · 2015-03-31T09:37:14Z

LibGit2Sharp.Tests/MergeFixture.cs

+                var master = repo.Branches["master"].Tip;
+
+                var result = repo.ObjectDatabase.MergeCommits(master, master, null);
+                Assert.Equal(MergeTreeStatus.Succeeded, result.Status);


Assert.NotNull(MergeTreeStatus.Tree);
Assert.Equal(0, MergeTreeStatus.Conflicts.Count());

ethomson · 2015-04-08T14:23:50Z

Updated with your suggestions, @nulltoken

ethomson · 2015-04-09T12:50:38Z

Oops, wrote that test sloppily. Updated.

nulltoken · 2015-04-09T13:10:31Z

@ethomson Thanks for this. One final nitpick. b2b6139 commit message states

Introduce ObjectDatabase.MergeCommits

Provide an API to merge two commits and return the resultant result.
This Index may be analyzed looking for conflicts or otherwise to
determine the success of the merge.  This Index must be disposed when
the caller has finished with it.  Thus, Index now implements
IDisposable and Indexes that are not the repository index (ie, generated
by merge) are expected to be disposed.  The repository's index is
not expected to be disposed and is protected from disposal.

Could you please reword it so that it doesn't refer to the Index any longer?

Provide an API to merge two commits and return the result. If successful, a new tree will be written to the object database that reflects the results of the merge. If the merge had conflicts, a ConflictCollection will be provided.

Introduce a MergeOptionsBase and a MergeAndCheckoutOptionsBase so that there's less duplication of the various options that go to merge-like things.

ethomson · 2015-04-09T13:16:14Z

Yeah, I updated that sloppily. Fixed.

Introduce `Repository.MergeCommits`

nulltoken · 2015-04-09T14:40:04Z

Published as NuGet pre-release package LibGit2Sharp.0.22.0-pre20150409142943

nulltoken reviewed Mar 11, 2015
View reviewed changes

ethomson force-pushed the ethomson/mergecommits branch 3 times, most recently from 850ee0e to edc7dc0 Compare March 24, 2015 02:34

nulltoken reviewed Mar 31, 2015
View reviewed changes

Conflicts: take the Index not the Repository

eaf7817

ethomson force-pushed the ethomson/mergecommits branch from edc7dc0 to d70b2e0 Compare April 8, 2015 14:23

ethomson force-pushed the ethomson/mergecommits branch 2 times, most recently from 71d12e4 to 853bd94 Compare April 9, 2015 12:42

Edward Thomson added 2 commits April 9, 2015 15:12

Introduce ObjectDatabase.MergeCommits

63da0d3

Provide an API to merge two commits and return the result. If successful, a new tree will be written to the object database that reflects the results of the merge. If the merge had conflicts, a ConflictCollection will be provided.

Introduce MergeOptionsBase

534d4e7

Introduce a MergeOptionsBase and a MergeAndCheckoutOptionsBase so that there's less duplication of the various options that go to merge-like things.

ethomson force-pushed the ethomson/mergecommits branch from 853bd94 to 534d4e7 Compare April 9, 2015 13:16

nulltoken added a commit that referenced this pull request Apr 9, 2015

Merge pull request #990 from libgit2/ethomson/mergecommits

cad89d0

Introduce `Repository.MergeCommits`

nulltoken merged commit cad89d0 into vNext Apr 9, 2015

nulltoken deleted the ethomson/mergecommits branch April 9, 2015 14:26

nulltoken added this to the v0.22 milestone Apr 9, 2015

nulltoken added the nuget-pre label Apr 9, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce `Repository.MergeCommits` #990

Introduce `Repository.MergeCommits` #990

ethomson commented Mar 11, 2015

nulltoken Mar 11, 2015

ethomson Mar 11, 2015

nulltoken Mar 11, 2015

nulltoken Mar 11, 2015

ethomson Mar 11, 2015

ethomson commented Mar 19, 2015

nulltoken commented Mar 22, 2015

ethomson commented Mar 24, 2015

nulltoken commented Mar 31, 2015

nulltoken Mar 31, 2015

ethomson commented Apr 8, 2015

ethomson commented Apr 9, 2015

nulltoken commented Apr 9, 2015

ethomson commented Apr 9, 2015

nulltoken commented Apr 9, 2015

Introduce Repository.MergeCommits #990

Introduce Repository.MergeCommits #990

Conversation

ethomson commented Mar 11, 2015

nulltoken Mar 11, 2015

Choose a reason for hiding this comment

ethomson Mar 11, 2015

Choose a reason for hiding this comment

nulltoken Mar 11, 2015

Choose a reason for hiding this comment

nulltoken Mar 11, 2015

Choose a reason for hiding this comment

ethomson Mar 11, 2015

Choose a reason for hiding this comment

ethomson commented Mar 19, 2015

nulltoken commented Mar 22, 2015

ethomson commented Mar 24, 2015

nulltoken commented Mar 31, 2015

nulltoken Mar 31, 2015

Choose a reason for hiding this comment

ethomson commented Apr 8, 2015

ethomson commented Apr 9, 2015

nulltoken commented Apr 9, 2015

ethomson commented Apr 9, 2015

nulltoken commented Apr 9, 2015

Introduce `Repository.MergeCommits` #990

Introduce `Repository.MergeCommits` #990