8000 Update to libgit2 6aa06b6 by carlosmn · Pull Request #1237 · libgit2/libgit2sharp · GitHub
[go: up one dir, main page]

Skip to content

Update to libgit2 6aa06b6 #1237

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

Merged
merged 4 commits into from
Feb 22, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


8000
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions LibGit2Sharp.Tests/MergeFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,24 @@ public void ConflictingMergeReposBinary()
}
}

[Fact]
public void CanFailOnFirstMergeConflict()
{
string path = SandboxMergeTestRepo();
using (var repo = new Repository(path))
{
var mergeResult = repo.Merge("conflicts", Constants.Signature, new MergeOptions() { FailOnConflict = true, });
Assert.Equal(MergeStatus.Conflicts, mergeResult.Status);

var master = repo.Branches["master"];
var branch = repo.Branches["conflicts"];
var mergeTreeResult = repo.ObjectDatabase.MergeCommits(master.Tip, branch.Tip, new MergeTreeOptions() { FailOnConflict = true });
Assert.Equal(MergeTreeStatus.Conflicts, mergeTreeResult.Status);
Assert.Empty(mergeTreeResult.Conflicts);
}

}

[Theory]
[InlineData(true, FastForwardStrategy.Default, fastForwardBranchInitialId, MergeStatus.FastForward)]
[InlineData(true, FastForwardStrategy.FastForwardOnly, fastForwardBranchInitialId, MergeStatus.FastForward)]
Expand Down
9 changes: 8 additions & 1 deletion LibGit2Sharp/Core/GitDiff.cs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,12 @@ internal delegate int diff_notify_cb(
IntPtr matched_pathspec,
IntPtr payload);

internal delegate int diff_progress_cb(
IntPtr diff_so_far,
IntPtr old_path,
IntPtr new_path,
IntPtr payload);

[StructLayout(LayoutKind.Sequential)]
internal class GitDiffOptions : IDisposable
{
Expand All @@ -208,7 +214,8 @@ internal class GitDiffOptions : IDisposable
public SubmoduleIgnore IgnoreSubmodules;
public GitStrArrayManaged PathSpec;
public diff_notify_cb NotifyCallback;
public IntPtr NotifyPayload;
public diff_progress_cb ProgressCallback;
public IntPtr Payload;

/* options controlling how to diff text is generated */

Expand Down
35 changes: 28 additions & 7 deletions LibGit2Sharp/Core/GitMergeOpts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ internal struct GitMergeOpts
{
public uint Version;

public GitMergeTreeFlags MergeTreeFlags;
public GitMergeFlag MergeTreeFlags;

/// <summary>
/// Similarity to consider a file renamed.
Expand All @@ -27,6 +27,14 @@ internal struct GitMergeOpts
/// </summary>
public IntPtr SimilarityMetric;

/// <summary>
/// Maximum number of times to merge common ancestors to build a
/// virtual merge base when faced with criss-cross merges. When this
/// limit is reached, the next ancestor will simply be used instead of
/// attempting to merge it. The default is unlimited.
/// </summary>
public uint RecursionLimit;

/// <summary>
/// Flags for automerging content.
/// </summary>
Expand All @@ -35,7 +43,7 @@ internal struct GitMergeOpts
/// <summary>
/// File merging flags.
/// </summary>
public GitMergeFileFlags FileFlags;
public GitMergeFileFlag FileFlags;
Copy link
Member

Choose a reason for hiding this comment

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

This struct needs a public uint RenameThreshold between the SimilarityMetric and the MergeFileFavorFlags. (Sorry for the breaking change.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, I missed that in the diff.

}

/// <summary>
Expand Down Expand Up @@ -98,30 +106,43 @@ internal enum GitMergePreference
}

[Flags]
internal enum GitMergeTreeFlags
internal enum GitMergeFlag
{
/// <summary>
/// No options.
/// </summary>
GIT_MERGE_TREE_NORMAL = 0,
GIT_MERGE_NORMAL = 0,

/// <summary>
/// Detect renames that occur between the common ancestor and the "ours"
/// side or the common ancestor and the "theirs" side. This will enable
/// the ability to merge between a modified and renamed file.
/// </summary>
GIT_MERGE_TREE_FIND_RENAMES = (1 << 0),
GIT_MERGE_FIND_RENAMES = (1 << 0),

/// <summary>
/// If a conflict occurs, exit immediately instead of attempting to
/// continue resolving conflicts. The merge operation will fail with
/// GIT_EMERGECONFLICT and no index will be returned.
///</summary>
GIT_MERGE_TREE_FAIL_ON_CONFLICT = (1 << 1),
GIT_MERGE_FAIL_ON_CONFLICT = (1 << 1),

/// <summary>
/// Do not write the REUC extension on the generated index
/// </summary>
GIT_MERGE_SKIP_REUC = (1 << 2),

/// <summary>
/// If the commits being merged have multiple merge bases, do not build
/// a recursive merge base (by merging the multiple merge bases),
/// instead simply use the first base. This flag provides a similar
/// merge base to `git-merge-resolve`.
/// </summary>
GIT_MERGE_NO_RECURSIVE = (1 << 3),
}

[Flags]
internal enum GitMergeFileFlags
internal enum GitMergeFileFlag
Copy link

Choose a reason for hiding this comment

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

Why drop the 's' from GitMergeFileFlags, won't this be a breaking change with no benefit?

Copy link
Member

Choose a reason for hiding this comment

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

won't this be a breaking change with no benefit?

I think this type isn't publicly exposed 😉

Copy link
Member Author

Choose a reason for hiding this comment

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

It's to mach the libgit2 rename, which brings this type into compliance with the naming structure.

{
/// <summary>
/// Defaults
Expand Down
24 changes: 20 additions & 4 deletions LibGit2Sharp/Core/Proxy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1101,11 +1101,19 @@ public static void git_index_clear(Index index)

#region git_merge_

public static IndexSafeHandle git_merge_commits(RepositorySafeHandle repo, GitObjectSafeHandle ourCommit, GitObjectSafeHandle theirCommit, GitMergeOpts opts)
public static IndexSafeHandle git_merge_commits(RepositorySafeHandle repo, GitObjectSafeHandle ourCommit, GitObjectSafeHandle theirCommit, GitMergeOpts opts, out bool earlyStop)
{
IndexSafeHandle index;
int res = NativeMethods.git_merge_commits(out index, repo, ourCommit, theirCommit, ref opts);
Ensure.ZeroResult(res);
if (res == (int)GitErrorCode.MergeConflict)
{
earlyStop = true;
}
else
{
earlyStop = false;
Ensure.ZeroResult(res);
}

return index;
}
Expand Down Expand Up @@ -1189,7 +1197,7 @@ public static ObjectId git_annotated_commit_id(GitAnnotatedCommitHandle mergeHea
return NativeMethods.git_annotated_commit_id(mergeHead).MarshalAsObjectId();
}

public static void git_merge(RepositorySafeHandle repo, GitAnnotatedCommitHandle[] heads, GitMergeOpts mergeOptions, GitCheckoutOpts checkoutOptions)
public static void git_merge(RepositorySafeHandle repo, GitAnnotatedCommitHandle[] heads, GitMergeOpts mergeOptions, GitCheckoutOpts checkoutOptions, out bool earlyStop)
{
IntPtr[] their_heads = heads.Select(head => head.DangerousGetHandle()).ToArray();

Expand All @@ -1199,7 +1207,15 @@ public static void git_merge(RepositorySafeHandle repo, GitAnnotatedCommitHandle
ref mergeOptions,
ref checkoutOptions);

Ensure.ZeroResult(res);
if (res == (int)GitErrorCode.MergeConflict)
{
earlyStop = true;
}
else
{
earlyStop = false;
Ensure.ZeroResult(res);
}
}

public static void git_merge_analysis(
Expand Down
24 changes: 17 additions & 7 deletions LibGit2Sharp/CurrentOperation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,39 +21,49 @@ public enum CurrentOperation
/// </summary>
Revert = 2,

/// <summary>
/// A sequencer revert is in progress.
/// </summary>
RevertSequence = 3,

/// <summary>
/// A cherry-pick is in progress.
/// </summary>
CherryPick = 3,
CherryPick = 4,

/// <summary>
/// A sequencer cherry-pick is in progress.
/// </summary>
CherryPickSequence = 5,

/// <summary>
/// A bisect is in progress.
/// </summary>
Bisect = 4,
Bisect = 6,

/// <summary>
/// A rebase is in progress.
/// </summary>
Rebase = 5,
Rebase = 7,

/// <summary>
/// A rebase --interactive is in progress.
/// </summary>
RebaseInteractive = 6,
RebaseInteractive = 8,

/// <summary>
/// A rebase --merge is in progress.
/// </summary>
RebaseMerge = 7,
RebaseMerge = 9,

/// <summary>
/// A mailbox application (am) is in progress.
/// </summary>
ApplyMailbox = 8,
ApplyMailbox = 10,

/// <summary>
/// A mailbox application (am) or rebase is in progress.
/// </summary>
ApplyMailboxOrRebase = 9,
ApplyMailboxOrRebase = 11,
}
}
4 changes: 2 additions & 2 deletions LibGit2Sharp/LibGit2Sharp.csproj
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="4.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Import Project="..\packages\LibGit2Sharp.NativeBinaries.1.0.106\build\LibGit2Sharp.NativeBinaries.props" Condition="Exists('..\packages\LibGit2Sharp.NativeBinaries.1.0.106\build\LibGit2Sharp.NativeBinaries.props')" />
<Import Project="..\packages\LibGit2Sharp.NativeBinaries.1.0.119\build\LibGit2Sharp.NativeBinaries.props" Condition="Exists('..\packages\LibGit2Sharp.NativeBinaries.1.0.119\build\LibGit2Sharp.NativeBinaries.props')" />
<PropertyGroup>
<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
<Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
Expand Down Expand Up @@ -405,7 +405,7 @@
<PropertyGroup>
<ErrorText>This project references NuGet package(s) that are missing on this computer. Enable NuGet Package Restore to download them. For more information, see http://go.microsoft.com/fwlink/?LinkID=322105. The missing file is {0}.</ErrorText>
</PropertyGroup>
<Error Condition="!Exists('..\packages\LibGit2Sharp.NativeBinaries.1.0.106\build\LibGit2Sharp.NativeBinaries.props')" Text="$([System.String]::Format('$(ErrorText)', '..\packages\LibGit2Sharp.NativeBinaries.1.0.106\build\LibGit2Sharp.NativeBinaries.props'))" />
<Error Condition="!Exists('..\packages\LibGit2Sharp.NativeBinaries.1.0.119\build\LibGit2Sharp.NativeBinaries.props')" Text="$([System.String]::Format('$(ErrorText)', '..\packages\LibGit2Sharp.NativeBinaries.1.0.119\build\LibGit2Sharp.NativeBinaries.props'))" />
</Target>
<!-- To modify your build process, add your task inside one of the targets below and uncomment it.
Other similar extension points exist, see Microsoft.Common.targets.
Expand Down
13 changes: 13 additions & 0 deletions LibGit2Sharp/MergeOptionsBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,19 @@ protected MergeOptionsBase()
/// </summary>
public bool FindRenames { get; set; }

/// <summary>
/// If set, do not create or return conflict entries, but stop and return
/// an error result after finding the first conflict.
/// </summary>
public bool FailOnConflict { get; set; }
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 switch actually triggers a throw, maybe should we describe the thrown exception in the xml doc? and maybe rename this member as ThrowsOnFirstConflict ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done (explanation), but I'd rather keep the libgit2 name for the option. It gets harder to know what you're doing if they don't match.

Copy link
Member

Choose a reason for hiding this comment

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

In order to help to user, shouldn't we describe the type of exception that's going to be thrown?

It 106D2 gets harder to know what you're doing if they don't match.

I agree, from a troubleshooting perspective, and we've done this for internal types.

However, Throws means something very clear in .NET land. Fail doesn't, and I'd rather stick with the idioms of the language when we're exposing a public type.

Copy link
Member Author

Choose a reason for hiding this comment

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

The new text does show which exception we throw. But there's of course the issue of whether we should throw at all, or if this should just be part of the merge result we return.


/// <summary>
/// Do not write the Resolve Undo Cache extension on the generated index. This can
/// be useful when no merge resolution will be presented to the user (e.g. a server-side
/// merge attempt).
/// </summary>
public bool SkipReuc { get; set; }
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 not sure that many people are at ease with REUC (Currently the very first result Google returns for "git reuc" is the source code of reuc.c in libgit2. Could we try and enhance a tad the xml doc to quickly describe what's the impact of setting (or not) this switch? Similarly, in order to be a little more expressive, how about renaming it to a less terse DoNotStoreResolvedConflictDataIntoTheIndexReucSection... ok maybe something shorter then ;-)

Copy link
Member Author

Choose a reason for hiding this comment

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

Nobody needs to be at ease with it, they can just ignore it. I've added some copy to use the full name and explain when you might want to disable it. It shouldn't need more than that to recognise that you're not in an environment which matches when this option is meant to be used.

Copy link
Member

Choose a reason for hiding this comment

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

Cool doco. Thanks!

I've added some copy to use the full name and explain when you might want to disable it

"disable"? But it's false by default, isn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Disable writing the extension, as in set this option to true.


/// <summary>
/// Similarity to consider a file renamed.
/// </summary>
Expand Down
33 changes: 29 additions & 4 deletions LibGit2Sharp/ObjectDatabase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,13 @@ public virtual bool CanMergeWithoutConflict(Commit one, Commit another)
Ensure.ArgumentNotNull(one, "one");
Ensure.ArgumentNotNull(another, "another");

var result = repo.ObjectDatabase.MergeCommits(one, another, null);
var opts = new MergeTreeOptions()
{
SkipReuc = true,
FailOnConflict = true,
};

var result = repo.ObjectDatabase.MergeCommits(one, another, opts);
return (result.Status == MergeTreeStatus.Succeeded);
}

Expand Down Expand Up @@ -603,22 +609,41 @@ public virtual MergeTreeResult MergeCommits(Commit ours, Commit theirs, MergeTre

options = options ?? new MergeTreeOptions();

// We throw away the index after looking at the conflicts, so we'll never need the REUC
// entries to be there
GitMergeFlag mergeFlags = GitMergeFlag.GIT_MERGE_NORMAL | GitMergeFlag.GIT_MERGE_SKIP_REUC;
if (options.FindRenames)
{
mergeFlags |= GitMergeFlag.GIT_MERGE_FIND_RENAMES;
}
if (options.FailOnConflict)
{
mergeFlags |= GitMergeFlag.GIT_MERGE_FAIL_ON_CONFLICT;
}


var mergeOptions = new GitMergeOpts
{
Version = 1,
MergeFileFavorFlags = options.MergeFileFavor,
MergeTreeFlags = options.FindRenames ? GitMergeTreeFlags.GIT_MERGE_TREE_FIND_RENAMES
: GitMergeTreeFlags.GIT_MERGE_TREE_NORMAL,
MergeTreeFlags = mergeFlags,
RenameThreshold = (uint)options.RenameThreshold,
TargetLimit = (uint)options.TargetLimit,
};

bool earlyStop;
using (var oneHandle = Proxy.git_object_lookup(repo.Handle, ours.Id, GitObjectType.Commit))
using (var twoHandle = Proxy.git_object_lookup(repo.Handle, theirs.Id, GitObjectType.Commit))
using (var indexHandle = Proxy.git_merge_commits(repo.Handle, oneHandle, twoHandle, mergeOptions))
using (var indexHandle = Proxy.git_merge_commits(repo.Handle, oneHandle, twoHandle, mergeOptions, out earlyStop))
{
MergeTreeResult mergeResult;

// Stopped due to FailOnConflict so there's no index or conflict list
if (earlyStop)
{
return new MergeTreeResult(new Conflict[] { });
}

if (Proxy.git_index_has_conflicts(indexHandle))
{
List<Conflict> conflicts = new List<Conflict>();
Expand Down
32 changes: 25 additions & 7 deletions LibGit2Sharp/Repository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1329,8 +1329,8 @@ public RevertResult Revert(Commit commit, Signature reverter, RevertOptions opti
{
Version = 1,
MergeFileFavorFlags = options.MergeFileFavor,
MergeTreeFlags = options.FindRenames ? GitMergeTreeFlags.GIT_MERGE_TREE_FIND_RENAMES :
GitMergeTreeFlags.GIT_MERGE_TREE_NORMAL,
MergeTreeFlags = options.FindRenames ? GitMergeFlag.GIT_MERGE_FIND_RENAMES :
GitMergeFlag.GIT_MERGE_NORMAL,
RenameThreshold = (uint)options.RenameThreshold,
TargetLimit = (uint)options.TargetLimit,
};
Expand Down Expand Up @@ -1413,8 +1413,8 @@ public CherryPickResult CherryPick(Commit commit, Signature committer, CherryPic
{
Version = 1,
MergeFileFavorFlags = options.MergeFileFavor,
MergeTreeFlags = options.FindRenames ? GitMergeTreeFlags.GIT_MERGE_TREE_FIND_RENAMES :
GitMergeTreeFlags.GIT_MERGE_TREE_NORMAL,
MergeTreeFlags = options.FindRenames ? GitMergeFlag.GIT_MERGE_FIND_RENAMES :
GitMergeFlag.GIT_MERGE_NORMAL,
RenameThreshold = (uint)options.RenameThreshold,
TargetLimit = (uint)options.TargetLimit,
};
Expand Down Expand Up @@ -1553,21 +1553,39 @@ private MergeResult Merge(GitAnnotatedCommitHandle[] annotatedCommits, Signature
private MergeResult NormalMerge(GitAnnotatedCommitHandle[] annotatedCommits, Signature merger, MergeOptions options)
{
MergeResult mergeResult;
GitMergeFlag treeFlags = options.FindRenames ? GitMergeFlag.GIT_MERGE_FIND_RENAMES
: GitMergeFlag.GIT_MERGE_NORMAL;

if (options.FailOnConflict)
{
treeFlags |= GitMergeFlag.GIT_MERGE_FAIL_ON_CONFLICT;
}

if (options.SkipReuc)
{
treeFlags |= GitMergeFlag.GIT_MERGE_SKIP_REUC;
}

var mergeOptions = new GitMergeOpts
{
Version = 1,
MergeFileFavorFlags = options.MergeFileFavor,
MergeTreeFlags = options.FindRenames ? GitMergeTreeFlags.GIT_MERGE_TREE_FIND_RENAMES
: GitMergeTreeFlags.GIT_MERGE_TREE_NORMAL,
MergeTreeFlags = treeFlags,
RenameThreshold = (uint)options.RenameThreshold,
TargetLimit = (uint)options.TargetLimit,
};

bool earlyStop;
using (GitCheckoutOptsWrapper checkoutOptionsWrapper = new GitCheckoutOptsWrapper(options))
{
var checkoutOpts = checkoutOptionsWrapper.Options;

Proxy.git_merge(Handle, annotatedCommits, mergeOptions, checkoutOpts);
Proxy.git_merge(Handle, annotatedCommits, mergeOptions, checkoutOpts, out earlyStop);
}

if (earlyStop)
{
return new MergeResult(MergeStatus.Conflicts);
}

if (Index.IsFullyMerged)
Expand Down
Loading
0