Skip to content

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

Introduce basic merging #579

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2baf633
Introduce basic merging
angusbjones Dec 5, 2013
6bbfcd7
Removed unnecessary changes to Tests project.
angusbjones Dec 6, 2013
7f21005
MergeFixture: Added in recommended asserts
angusbjones Dec 6, 2013
747b4e4
Explicitly define author in merge test commit and introduced merge me…
angusbjones Dec 6, 2013
3c96d22
Introduced suggested GitHub changes. +Added more tests.
angusbjones Dec 22, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions LibGit2Sharp.Tests/LibGit2Sharp.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,33 @@
<WarningLevel>4</WarningLevel>
<DocumentationFile />
</PropertyGroup>
<PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Debug|x86'">
<DebugSymbols>true</DebugSymbols>
<OutputPath>bin\x86\Debug\</OutputPath>
<DefineConstants>TRACE;DEBUG;NET35</DefineConstants>
<DebugType>full</DebugType>
<PlatformTarget>x86</PlatformTarget>
<ErrorReport>prompt</ErrorReport>
<CodeAnalysisRuleSet>MinimumRecommendedRules.ruleset</CodeAnalysisRuleSet>
</PropertyGroup>
<PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Release|x86'">
<OutputPath>bin\x86\Release\</OutputPath>
<DefineConstants>TRACE</DefineConstants>
<Optimize>true</Optimize>
<DebugType>pdbonly</DebugType>
<PlatformTarget>x86</PlatformTarget>
<ErrorReport>prompt</ErrorReport>
<CodeAnalysisRuleSet>MinimumRecommendedRules.ruleset</CodeAnalysisRuleSet>
</PropertyGroup>
<PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Leaks|x86'">
<DebugSymbols>true</DebugSymbols>
<OutputPath>bin\x86\Leaks\</OutputPath>
<DefineConstants>TRACE;DEBUG;NET35;LEAKS</DefineConstants>
<DebugType>full</DebugType>
<PlatformTarget>x86</PlatformTarget>
<ErrorReport>prompt</ErrorReport>
<CodeAnalysisRuleSet>MinimumRecommendedRules.ruleset</CodeAnalysisRuleSet>
</PropertyGroup>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need a dedicated x86 target?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about that, a cherry-pick casualty.

<ItemGroup>
<Reference Include="Moq, Version=4.0.10827.0, Culture=neutral, PublicKeyToken=69f491c39445e920, processorArchitecture=MSIL">
<SpecificVersion>False</SpecificVersion>
Expand Down Expand Up @@ -129,6 +156,9 @@
<Name>LibGit2Sharp</Name>
</ProjectReference>
</ItemGroup>
<ItemGroup>
<Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please drop this? It looks unrelated to the scope of this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

</ItemGroup>
<Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
<PropertyGroup>
<NativeBinariesDirectory>$(MSBuildProjectDirectory)\..\Lib\NativeBinaries</NativeBinariesDirectory>
Expand Down
44 changes: 44 additions & 0 deletions LibGit2Sharp.Tests/MergeFixture.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System.Linq;
using LibGit2Sharp.Tests.TestHelpers;
using Xunit;
using System;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this using necessary? if so, maybe sort it with the other System.* statements?


namespace LibGit2Sharp.Tests
{
Expand Down Expand Up @@ -80,5 +81,48 @@ 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();
AddFileCommitToRepo(repo, "first+second branch file");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't that be "first branch file"?


var secondBranch = repo.CreateBranch("SecondBranch");
AddFileCommitToRepo(repo, "first branch file");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And "first+second branch file" here?

secondBranch.Checkout();
AddFileCommitToRepo(repo, "second branch file");

repo.MergeOnto(repo.Branches["FirstBranch"].Tip);

repo.Commit("Merge First+Second");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add some assertions here?

Maybe:

  • Number of parents of the merge commit
  • Ensuring the Commit.Tree contains the expected files

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In. I'm thinking there should probably be some tests related to the merge result if it's felt that its necessary?

}
}

private Commit AddCommitToRepo(IRepository repository)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks unused, doesn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and another. Removed.

{
string random = Guid.NewGuid().ToString();
string filename = random + ".txt";

Touch(repository.Info.WorkingDirectory, filename, random);

repository.Index.Stage(filename);

return repository.Commit("New commit", Constants.Signature, Constants.Signature);
}

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);
}
}
}
17 changes: 17 additions & 0 deletions LibGit2Sharp/Core/GitMergeHead.cs
View file
Open in desktop
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)]
Copy link
Member

Choose a reason for hiding this comment

The 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
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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)

Copy link
Member

Choose a reason for hiding this comment

The 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 git_merge_opts and git_merge_tree_opts. These are used when we need consumers to be able to set a lot of options and pass it as a single argument to a function. On the other hand, we have a bunch of "opaque objects" - structures that are defined internally, but in the public header files you will only see a forward declaration:

typedef struct git_merge_head git_merge_head;

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: git_merge_head * -- since the compiler knows how big a pointer will be. Consumers can't deference it, but they can pass it to libgit2. This allows libgit2 to provide an object to consumers that the library knows about, but consumers don't, allowing us to hide some implementation details and be able to change the details of that structure without breaking compatibility for our consumers.

Since MergeHead is an opaque object in libgit2, we don't need to build a serializable structure. Instead we can call git_merge_head_from_oid, git_merge_head_from_ref or git_merge_head_from_fetchhead, get the opaque GitMergeHeadHandle back and pass that to git_merge.

Copy link
Member

Choose a reason for hiding this comment

The 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?

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;
}
}
22 changes: 22 additions & 0 deletions LibGit2Sharp/Core/GitMergeOpts.cs
View file
Open in desktop
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ethomson Can one combine those flags?

IOW, would GIT_MERGE_NO_FASTFORWARD | GIT_MERGE_FASTFORWARD_ONLY make sense?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One cannot, it definitely doesn't make sense.

Copy link
Member

Choose a reason for hiding this comment

The 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 GIT_MERGE_NO_FASTFORWARD with GIT_MERGE_FASTFORWARD_ONLY but maybe you could combine it with something else like GIT_MERGE_DONT_WRITE_CRAPPY_METADATA_FILES or something.

Copy link
Member

Choose a reason for hiding this comment

The 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_DEFAULT = 0?

{
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;
}
}
34 changes: 34 additions & 0 deletions LibGit2Sharp/Core/GitMergeTreeOpts.cs
View file
Open in desktop
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Most of the enums are of the form Git* (i.e. GitMergeTreeFlags) (although there are a couple of exceptions to this pattern).

{
GIT_MERGE_TREE_FIND_RENAMES = (1 << 0),
}
[Flags]
internal enum MergeAutomergeFlags
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, are those exclusive options?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are indeed exclusive to one another.

Copy link
Member

Choose a reason for hiding this comment

The 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 Strategy - (e.g. maybe GitMergeAutomergeStrategy)

{
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;
}
}
13 changes: 13 additions & 0 deletions LibGit2Sharp/Core/Handles/GitMergeHeadHandle.cs
View file
Open in desktop
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;
}
}
}
13 changes: 13 additions & 0 deletions LibGit2Sharp/Core/Handles/GitMergeResultHandle.cs
View file
Open in desktop
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;
}
}
}
35 changes: 35 additions & 0 deletions LibGit2Sharp/Core/NativeMethods.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,41 @@ internal static extern int git_merge_base(
GitObjectSafeHandle one,
GitObjectSafeHandle two);

[DllImport(libgit2)]
internal static extern int git_merge_head_from_oid(
out GitMergeHeadHandle their_heads,
RepositorySafeHandle repo,
ref GitOid mergeBase);

[DllImport(libgit2)]
internal static extern int git_merge(
out GitMergeResultHandle mergeResult,
RepositorySafeHandle repo,
ref GitMergeHeadHandle their_heads,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an array of ref heads, not a single one. I think this needs to be:

    [DllImport(libgit2)]
    internal static extern int git_merge(
        out MergeResultSafeHandle git_merge_result,
        RepositorySafeHandle repo,
        [MarshalAs(UnmanagedType.LPArray, SizeParamIndex = 2)] [In] IntPtr[] their_heads,
        int their_heads_length,
        GitMergeOptions options);

UIntPtr their_heads_len,
ref GitMergeOpts given_opts);

[DllImport(libgit2)]
internal static extern int git_merge_result_is_uptodate(
GitMergeResultHandle merge_result);

[DllImport(libgit2)]
internal static extern int git_merge_result_is_fastforward(
GitMergeResultHandle merge_result);

[DllImport(libgit2)]
internal static extern int git_merge_result_fastforward_oid(
out GitOid oid,
GitMergeResultHandle merge_result);

[DllImport(libgit2)]
internal static extern void git_merge_result_free(
IntPtr merge_result);

[DllImport(libgit2)]
internal static extern void git_merge_head_free(
IntPtr merge_head);

[DllImport(libgit2)]
internal static extern int git_message_prettify(
byte[] message_out, // NB: This is more properly a StringBuilder, but it's UTF8
Expand Down
71 changes: 71 additions & 0 deletions LibGit2Sharp/Core/Proxy.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -934,6 +934,77 @@ public static ObjectId git_merge_base(RepositorySafeHandle repo, Commit first, C
}
}

public static GitMergeResult git_merge(RepositorySafeHandle repo, GitMergeOpts opts, GitOid oid)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would much rather have git_merge take merge heads, so that we can merge branches and FETCH_HEADs, not just OIDs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example:

    public static MergeResultSafeHandle git_merge(RepositorySafeHandle repo, MergeHeadSafeHandle[] heads, GitMergeOptions options)
    {
        using (ThreadAffinity())
        {
            MergeResultSafeHandle ret;

            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,
                their_heads.Length,
                options);

            Ensure.ZeroResult(res);

            return ret;
        }
    }

{
using (ThreadAffinity())
{
GitMergeHeadHandle their_heads;

int res1 = NativeMethods.git_merge_head_from_oid(out their_heads, repo, ref oid);
if (res1 == (int)GitErrorCode.NotFound)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please cover with a test this branch of code?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: missing braces :trollface:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This methods been split up now, I'll pop in the test once I understand what has to happen with #579 (diff)

return null;

Ensure.ZeroResult(res1);

GitMergeResultHandle ret;
Copy link
Member

Choose a reason for hiding this comment

The 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 GitMergeResultHandle and GitMergeHeadHandle

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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?


int res2 = NativeMethods.git_merge(out ret, repo, ref their_heads, (UIntPtr)1, ref opts);

if (res2 == (int)GitErrorCode.NotFound)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please cover with a test this branch of code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe @ethomson can correct me but I think I was incorrect in putting the check there, his example method above does not do a similar check and this is now reflected in the new version of the method. I'll go over the git_merge method in libgit2 to see if it can return GIT_ENOTFOUND however (if no one knows off the top of their heads).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

git_merge should only ever return GIT_ENOTFOUND in an error case. I
wouldn't check the error code, I would just let it throw back to the
caller. (Unless you're seeing this in a case where you don't expect to!)

Actually, the same is true of git_merge_head_from_oid, I think the
pattern in libgit2# is to let that throw and not return null, but I may
be mistaken? I

On Fri, Dec 6, 2013 at 9:38 AM, crumblycake [email protected]:

In LibGit2Sharp/Core/Proxy.cs:

  •    {

  •        using (ThreadAffinity())

  •        {

  •            GitMergeHeadHandle their_heads;

  •            int res1 = NativeMethods.git_merge_head_
    from_oid(out their_heads, repo, ref oid);
  •            if (res1 == (int)GitErrorCode.NotFound)

  •                return null;

  •            Ensure.ZeroResult(res1);

  •            GitMergeResultHandle ret;

  •            int res2 = NativeMethods.git_merge(out ret, repo, ref their_heads, (UIntPtr)1, ref opts);

  •            if (res2 == (int)GitErrorCode.NotFound)

Maybe @ethomson https://github.com/ethomson can correct me but I think
I was incorrect in putting the check there, his example method above does
not do a similar check and this is now reflected in the new version of the
method. I'll go over the git_merge method in libgit2 to see if it can
return GIT_ENOTFOUND however (if no one knows off the top of their heads).

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/579/files#r8162091
.

{
return null;
}

Ensure.ZeroResult(res2);

return new GitMergeResult(ret);
}
}

public static bool git_merge_result_is_uptodate(GitMergeResultHandle handle)
{
using (ThreadAffinity())
{
int res = NativeMethods.git_merge_result_is_uptodate(handle);
Ensure.ZeroResult(res);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might be after Ensure.BooleanResult() here


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.ZeroResult(res);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure.BooleanResult() also, I think


return (res == 1);
}
}

public static GitOid git_merge_result_fastforward_oid(GitMergeResultHandle handle)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks unused as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would anticipate the GitMergeResult class using this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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_
Expand Down
42 changes: 42 additions & 0 deletions LibGit2Sharp/GitMergeResult.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Globalization;
using System.IO;
using System.Linq;
using System.Reflection;
using System.Text.RegularExpressions;
using LibGit2Sharp.Core;
using LibGit2Sharp.Core.Compat;
using LibGit2Sharp.Core.Handles;
using LibGit2Sharp.Handlers;

namespace LibGit2Sharp
{
[DebuggerDisplay("{DebuggerDisplay,nq}")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy/paste leftover? 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You got me, cleaned. I don't even have C & V keys anymore, I've worn them down to the desk below.

public class GitMergeResult
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This type doesn't look like currently used. How do you foresee it's usage?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Repository.Merge should return this. The test cases should use this as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. If this case, I'd prefer to make it eagerly load its properties rather than keeping a hold on the handle.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a nitpick: the name should be something like MergeResult. The Git prefix indicates that it's a marshalling struct, and will usually live in LibGit2Sharp.Core.

{
public GitMergeResult(){}
internal GitMergeResult(GitMergeResultHandle handle)
{
_handle = handle;
}

private readonly GitMergeResultHandle _handle;

internal GitMergeResultHandle Handle
{
get { return _handle; }
}

public virtual bool IsUpToDate
{
get { return Proxy.git_merge_result_is_uptodate(_handle); }
}

public virtual bool IsFastForward
{
get { return Proxy.git_merge_result_is_fastforward(_handle); }
}
}
}
6 changes: 6 additions & 0 deletions LibGit2Sharp/LibGit2Sharp.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,21 @@
<Compile Include="RefSpec.cs" />
<Compile Include="RefSpecCollection.cs" />
<Compile Include="Core\EncodingMarshaler.cs" />
<Compile Include="Core\GitMergeHead.cs" />
<Compile Include="Core\GitMergeOpts.cs" />
<Compile Include="Core\GitMergeTreeOpts.cs" />
<Compile Include="Core\Handles\BranchIteratorSafeHandle.cs" />
<Compile Include="Core\Handles\ConfigurationIteratorSafeHandle.cs" />
<Compile Include="Core\GitBlame.cs" />
<Compile Include="Core\Handles\BlameSafeHandle.cs" />
<Compile Include="Core\Handles\GitMergeHeadHandle.cs" />
<Compile Include="Core\Handles\GitMergeResultHandle.cs" />
<Compile Include="Core\PushTransferProgressCallbacks.cs" />
<Compile Include="Core\PackbuilderCallbacks.cs" />
<Compile Include="PushOptions.cs" />
<Compile Include="Core\GitBuf.cs" />
<Compile Include="FilteringOptions.cs" />
<Compile Include="GitMergeResult.cs" />
<Compile Include="ResetMode.cs" />
<Compile Include="NoteCollectionExtensions.cs" />
<Compile Include="RefSpecDirection.cs" />
Expand Down
Loading