-
Notifications
You must be signed in to change notification settings - Fork 899
Introduce basic merging #579
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
2baf633
6bbfcd7
7f21005
747b4e4
3c96d22
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
using System.Linq; | ||
using LibGit2Sharp.Tests.TestHelpers; | ||
using LibGit2Sharp.Tests.TestHelpers; | ||
using System.Linq; | ||
using Xunit; | ||
|
||
namespace LibGit2Sharp.Tests | ||
|
@@ -80,5 +80,45 @@ public void CanRetrieveTheBranchBeingMerged() | |
Assert.Null(mergedHeads[1].Tip); | ||
} | ||
} | ||
|
||
[Fact] | ||
public void CanMergeRepos() | ||
{ | ||
string path = CloneStandardTestRepo(); | ||
using (var repo = new Repository(path)) | ||
{ | ||
var firstBranch = repo.CreateBranch("FirstBranch"); | ||
firstBranch.Checkout(); | ||
var originalTreeCount = firstBranch.Tip.Tree.Count; | ||
|
||
//Commit with ONE new file to both first & second branch (SecondBranch is created on this commit). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another small style point: We usually put a space between the |
||
AddFileCommitToRepo(repo, "first+second branch file"); | ||
|
||
var secondBranch = repo.CreateBranch("SecondBranch"); | ||
//Commit with ONE new file to first branch (FirstBranch moves forward as it is checked out, SecondBranch stays back one). | ||
var firstBranchCommit = AddFileCommitToRepo(repo, "first branch file"); | ||
|
||
secondBranch.Checkout(); | ||
//Commit with ONE new file to second branch (FirstBranch and SecondBranch now point to separate commits that both have the same parent commit). | ||
var secondBranchCommit = AddFileCommitToRepo(repo, "second branch file"); | ||
|
||
MergeResult mergeResult = repo.Merge(repo.Branches["FirstBranch"].Tip); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't we assert anything regarding the state of |
||
|
||
var mergeCommit = repo.Commit("Merge First+Second", Constants.Signature, Constants.Signature); | ||
|
||
Assert.Equal(mergeCommit.Tree.Count, originalTreeCount + 3); //Expecting original tree count plussed by the 3 added files. | ||
Assert.Equal(mergeCommit.Parents.Count(), 2); //Merge commit should have 2 parents | ||
} | ||
} | ||
|
||
private Commit AddFileCommitToRepo(IRepository repository, string filename, string content = null) | ||
{ | ||
filename = filename + ".txt"; | ||
Touch(repository.Info.WorkingDirectory, filename, content); | ||
|
||
repository.Index.Stage(filename); | ||
|
||
return repository.Commit("New commit", Constants.Signature, Constants.Signature); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
using LibGit2Sharp.Core.Handles; | ||
using System; | ||
using System.Runtime.InteropServices; | ||
|
||
namespace LibGit2Sharp.Core | ||
{ | ||
[StructLayout(LayoutKind.Sequential)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is internal to libgit2. Is there some reason you need it exposed? |
||
internal struct GitMergeHead | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't find any code leveraging this type. Am I missing anything? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could be removed, I just pumped out a bunch of types I thought I'd need when I first started and missed removing it on the first commit, its mentioned by @ethomson above also. The reason I haven't removed it just yet though is I wasn't sure whether its what was being referenced in the comment above: #579 (diff) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm sorry, I was super vague there. To rewind a bit: libgit2 has some types of structures that are publicly visible (defined in the public header files) - some examples of that are the
Consumers can't create one of these because the compiler doesn't know anything about it -- but you can talk about a pointer to that structure: Since There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It still looks unused. Are there any reason to keep it around? |
||
{ | ||
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalCookie = UniqueId.UniqueIdentifier, MarshalTypeRef = typeof(StrictUtf8Marshaler))] string RefName; | ||
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalCookie = UniqueId.UniqueIdentifier, MarshalTypeRef = typeof(StrictUtf8Marshaler))] string RemoteUrl; | ||
|
||
GitOid Oid; | ||
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalCookie = UniqueId.UniqueIdentifier, MarshalTypeRef = typeof(StrictUtf8Marshaler))] string OidStr; | ||
GitObjectSafeHandle Commit; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
using System; | ||
using System.Runtime.InteropServices; | ||
|
||
namespace LibGit2Sharp.Core | ||
{ | ||
[Flags] | ||
internal enum MergeFlags | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ethomson Can one combine those flags? IOW, would There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One cannot, it definitely doesn't make sense. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But(!) my thinking is that this is a bitfield and we may have other options that could be combined later. You can't combine There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ethomson - This is not currently represented as a bit field in libgit2. If that is the intention of this type, should we update the libgit2 representation? In addition, should there be a named default value (both here and in libgit2) - |
||
{ | ||
GIT_MERGE_NO_FASTFORWARD = 1, | ||
GIT_MERGE_FASTFORWARD_ONLY = 2, | ||
} | ||
|
||
[StructLayout(LayoutKind.Sequential)] | ||
internal struct GitMergeOpts | ||
{ | ||
public uint Version; | ||
|
||
public MergeFlags MergeFlags; | ||
public GitMergeTreeOpts MergeTreeOpts; | ||
public GitCheckoutOpts CheckoutOpts; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
using LibGit2Sharp.Core.Handles; | ||
using System; | ||
using System.Runtime.InteropServices; | ||
|
||
namespace LibGit2Sharp.Core | ||
{ | ||
[Flags] | ||
internal enum MergeTreeFlags | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Most of the enums are of the form Git* (i.e. |
||
{ | ||
GIT_MERGE_TREE_FIND_RENAMES = (1 << 0), | ||
} | ||
[Flags] | ||
internal enum MergeAutomergeFlags | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly, are those exclusive options? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are indeed exclusive to one another. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this is not an actual flag type, maybe we should suffix with |
||
{ | ||
GIT_MERGE_AUTOMERGE_NORMAL = 0, | ||
GIT_MERGE_AUTOMERGE_NONE = 1, | ||
GIT_MERGE_AUTOMERGE_FAVOR_OURS = 2, | ||
GIT_MERGE_AUTOMERGE_FAVOR_THEIRS = 3, | ||
} | ||
|
||
[StructLayout(LayoutKind.Sequential)] | ||
internal struct GitMergeTreeOpts | ||
{ | ||
public uint Version; | ||
|
||
public MergeTreeFlags MergeTreeFlags; | ||
public uint RenameThreshold; | ||
public uint TargetLimit; | ||
|
||
public UIntPtr Metric; | ||
|
||
public MergeAutomergeFlags MergeAutomergeFlags; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
using System.Runtime.InteropServices; | ||
|
||
namespace LibGit2Sharp.Core.Handles | ||
{ | ||
internal class GitMergeHeadHandle : SafeHandleBase | ||
{ | ||
protected override bool ReleaseHandleImpl() | ||
{ | ||
Proxy.git_merge_head_free(handle); | ||
return true; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
using System.Runtime.InteropServices; | ||
|
||
namespace LibGit2Sharp.Core.Handles | ||
{ | ||
internal class GitMergeResultHandle : SafeHandleBase | ||
{ | ||
protected override bool ReleaseHandleImpl() | ||
{ | ||
Proxy.git_merge_result_free(handle); | ||
return true; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -934,6 +934,91 @@ public static ObjectId git_merge_base(RepositorySafeHandle repo, Commit first, C | |
} | ||
} | ||
|
||
public static GitMergeHeadHandle git_merge_head_from_oid(RepositorySafeHandle repo, GitOid oid) | ||
{ | ||
using (ThreadAffinity()) | ||
{ | ||
GitMergeHeadHandle their_head; | ||
|
||
int res = NativeMethods.git_merge_head_from_oid(out their_head, repo, ref oid); | ||
if (res == (int)GitErrorCode.NotFound) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When would this be triggered? |
||
return null; | ||
|
||
Ensure.ZeroResult(res); | ||
|
||
return their_head; | ||
} | ||
} | ||
|
||
public static GitMergeResultHandle git_merge(RepositorySafeHandle repo, GitMergeHeadHandle[] heads, GitMergeOpts options) | ||
{ | ||
using (ThreadAffinity()) | ||
{ | ||
GitMergeResultHandle ret; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You may be missing some code here (or in the calling layers) taking care of releasing the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wrapped them up in usings in the calling layers, also if there is a better approach (or just a standard that's used) please let me know? |
||
|
||
IntPtr[] their_heads = new IntPtr[heads.Length]; | ||
for (int i = 0; i < heads.Length; i++) | ||
{ | ||
their_heads[i] = heads[i].DangerousGetHandle(); | ||
} | ||
|
||
int res = NativeMethods.git_merge( | ||
out ret, | ||
repo, | ||
their_heads, | ||
(UIntPtr)their_heads.Length, | ||
ref options); | ||
|
||
Ensure.ZeroResult(res); | ||
|
||
return ret; | ||
} | ||
} | ||
|
||
public static bool git_merge_result_is_uptodate(GitMergeResultHandle handle) | ||
{ | ||
using (ThreadAffinity()) | ||
{ | ||
int res = NativeMethods.git_merge_result_is_uptodate(handle); | ||
Ensure.BooleanResult(res); | ||
|
||
return (res == 1); | ||
} | ||
} | ||
|
||
public static bool git_merge_result_is_fastforward(GitMergeResultHandle handle) | ||
{ | ||
using (ThreadAffinity()) | ||
{ | ||
int res = NativeMethods.git_merge_result_is_fastforward(handle); | ||
Ensure.BooleanResult(res); | ||
|
||
return (res == 1); | ||
} | ||
} | ||
|
||
public static GitOid git_merge_result_fastforward_oid(GitMergeResultHandle handle) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks unused as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would anticipate the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. GitMergeResult now uses it. |
||
{ | ||
using (ThreadAffinity()) | ||
{ | ||
GitOid oid; | ||
int res = NativeMethods.git_merge_result_fastforward_oid(out oid, handle); | ||
Ensure.ZeroResult(res); | ||
|
||
return oid; | ||
} | ||
} | ||
|
||
public static void git_merge_result_free(IntPtr handle) | ||
{ | ||
NativeMethods.git_merge_result_free(handle); | ||
} | ||
|
||
public static void git_merge_head_free(IntPtr handle) | ||
{ | ||
NativeMethods.git_merge_head_free(handle); | ||
} | ||
|
||
#endregion | ||
|
||
#region git_message_ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -202,6 +202,11 @@ public interface IRepository : IDisposable | |
/// </summary> | ||
IEnumerable<MergeHead> MergeHeads { get; } | ||
|
||
/// <summary> | ||
/// Merges the given commit into HEAD. | ||
/// </summary> | ||
MergeResult Merge(Commit commit); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please add some xml doc regarding the input/output params? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oops, added to Repository.cs but didn't in IRepository. Coming in next commit. |
||
|
||
/// <summary> | ||
/// Manipulate the currently ignored files. | ||
/// </summary> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
using LibGit2Sharp.Core; | ||
using LibGit2Sharp.Core.Handles; | ||
using System.Diagnostics; | ||
|
||
namespace LibGit2Sharp | ||
{ | ||
public class MergeResult | ||
{ | ||
public MergeResult(){} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For consistency with other protected default ctors, perhaps: /// <summary>
/// Needed for mocking purposes.
/// </summary>
protected MergeResult()
{ } |
||
internal MergeResult(GitMergeResultHandle handle) | ||
{ | ||
_isUpToDate = Proxy.git_merge_result_is_uptodate(handle); | ||
_isFastForward = Proxy.git_merge_result_is_fastforward(handle); | ||
|
||
if (_isFastForward) | ||
_oid = Proxy.git_merge_result_fastforward_oid(handle); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: Please add braces here |
||
} | ||
|
||
private bool _isUpToDate; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could also use automatic properties with a private setter for these.
Alternatively, if you decide to use the private fields, we don't usually use the |
||
public virtual bool IsUpToDate | ||
{ | ||
get { return _isUpToDate; } | ||
} | ||
|
||
private bool _isFastForward; | ||
public virtual bool IsFastForward | ||
{ | ||
get { return _isFastForward; } | ||
} | ||
|
||
private readonly GitOid _oid; | ||
internal GitOid FastForwardOid | ||
{ | ||
get { return _oid; } | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small style point: Normally, we have sorted the System using directives above all other directives. This default sorting behavior has changed in Visual Studio 2013.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Man, it just looks wrong to have System anywhere but sorted first. :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah - at least there is a option to sort these first.