Rebase: Handle case where patch has already been applied

Author: jamill

LibGit2Sharp.Tests/RebaseFixture.cs

@@ -17,6 +17,7 @@ public class RebaseFixture : BaseFixture
         const string topicBranch1Name = "T1";
         const string topicBranch2Name = "T2";
         const string conflictBranch1Name = "C1";
+        const string topicBranch1PrimeName = "T1Prime";
 
         [Theory]
         [InlineData(topicBranch2Name, topicBranch2Name, topicBranch1Name, masterBranch1Name, 3)]
@@ -50,7 +51,7 @@ public void CanRebase(string initialBranchName,
                 int afterStepCallCount = 0;
 
                 List<ObjectId> PreRebaseCommits = new List<ObjectId>();
-                List<ObjectId> PostRebaseCommits = new List<ObjectId>();
+                List<CompletedRebaseStepInfo> PostRebaseResults = new List<CompletedRebaseStepInfo>();
                 ObjectId expectedParentId = upstream.Tip.Id;
 
                 RebaseOptions options = new RebaseOptions()
@@ -63,7 +64,7 @@ public void CanRebase(string initialBranchName,
                     RebaseStepCompleted = x =>
                     {
                         afterStepCallCount++;
-                        PostRebaseCommits.Add(x.CommitId);
+                        PostRebaseResults.Add(new CompletedRebaseStepInfo(x.CommitId, x.WasPatchAlreadyApplied));
                     },
                 };
 
@@ -91,19 +92,58 @@ public void CanRebase(string initialBranchName,
 
                 // Verify the chain of commits that resulted from the rebase.
                 Commit expectedParent = expectedOntoCommit;
-                foreach(Commit rebasedCommit in PostRebaseCommits.Select(id => repo.Lookup<Commit>(id)))
+                foreach(CompletedRebaseStepInfo stepInfo in PostRebaseResults)
                 {
+                    Commit rebasedCommit = repo.Lookup<Commit>(stepInfo.ObjectId);
                     Assert.Equal(expectedParent.Id, rebasedCommit.Parents.First().Id);
+                    Assert.False(stepInfo.WasPatchAlreadyApplied);
                     expectedParent = rebasedCommit;
                 }
 
-                Assert.Equal(repo.Head.Tip.Id, PostRebaseCommits.Last());
+                Assert.Equal(repo.Head.Tip.Id, PostRebaseResults.Last().ObjectId);
             }
         }
 
-        private class rebaseStepInfo
+        private class CompletedRebaseStepInfo : IEqualityComparer<CompletedRebaseStepInfo>
         {
-            public Commit Commit { get; set; }
+            public CompletedRebaseStepInfo(ObjectId objectId, bool wasPatchAlreadyApplied)
+            {
+                ObjectId = objectId;
+                WasPatchAlreadyApplied = wasPatchAlreadyApplied;
+            }
+
+            public ObjectId ObjectId { get; set; }
+
+            public bool WasPatchAlreadyApplied { get; set; }
+
+            bool IEqualityComparer<CompletedRebaseStepInfo>.Equals(CompletedRebaseStepInfo x, CompletedRebaseStepInfo y)
+            {
+                if (x == null && y == null)
+                {
+                    return true;
+                }
+
+                if ((x == null && y != null ) ||
+                    (x != null && y == null ))
+                {
+                    return false;
+                }
+
+                return x.WasPatchAlreadyApplied == y.WasPatchAlreadyApplied &&
+                       ObjectId.Equals(x.ObjectId, y.ObjectId);
+            }
+
+            int IEqualityComparer<CompletedRebaseStepInfo>.GetHashCode(CompletedRebaseStepInfo obj)
+            {
+                int hashCode = obj.WasPatchAlreadyApplied.GetHashCode();
+
+                if (obj.ObjectId != null)
+                {
+                    hashCode += obj.ObjectId.GetHashCode();
+                }
+
+                return hashCode;
+            }
         }
 
         /// <summary>
@@ -335,7 +375,7 @@ public void RebaseWhileAlreadyRebasingThrows()
 
                 Assert.Throws<LibGit2SharpException>(() =>
                     repo.Rebase.Start(branch, upstream, onto, Constants.Signature, null));
-                }
+            }
         }
 
         [Fact]
@@ -371,6 +411,57 @@ public void CurrentStepInfoIsNullWhenNotRebasing()
             }
         }
 
+        [Fact]
+        public void CanHandlePatchAlreadyApplied()
+        {
+            SelfCleaningDirectory scd = BuildSelfCleaningDirectory();
+            var path = Repository.Init(scd.DirectoryPath);
+            using (Repository repo = new Repository(path))
+            {
+                ConstructRebaseTestRepository(repo);
+
+                repo.Checkout(topicBranch1Name);
+
+                 Branch topicBranch1Prime = repo.CreateBranch(topicBranch1PrimeName, masterBranch1Name);
+
+                string newFileRelativePath = "new_file.txt";
+                Touch(repo.Info.WorkingDirectory, newFileRelativePath, "New Content");
+                repo.Stage(newFileRelativePath);
+                Commit commit = repo.Commit("new commit 1", Constants.Signature, Constants.Signature, new CommitOptions());
+
+                repo.Checkout(topicBranch1Prime);
+                var cherryPickResult = repo.CherryPick(commit, Constants.Signature2);
+                Assert.Equal(CherryPickStatus.CherryPicked, cherryPickResult.Status);
+
+                string newFileRelativePath2 = "new_file_2.txt";
+                Touch(repo.Info.WorkingDirectory, newFileRelativePath2, "New Content for path 2");
+                repo.Stage(newFileRelativePath2);
+                repo.Commit("new commit 2", Constants.Signature, Constants.Signature, new CommitOptions());
+
+                Branch upstreamBranch = repo.Branches[topicBranch1Name];
+
+                List<CompletedRebaseStepInfo> rebaseResults = new List<CompletedRebaseStepInfo>();
+
+                RebaseOptions options = new RebaseOptions()
+                {
+                    RebaseStepCompleted = x => 
+                    {
+                        rebaseResults.Add(new CompletedRebaseStepInfo(x.CommitId, x.WasPatchAlreadyApplied));
+                    }
+                };
+
+                repo.Rebase.Start(null, upstreamBranch, null, Constants.Signature2, options);
+
+                List<CompletedRebaseStepInfo> expectedRebaseResults = new List<CompletedRebaseStepInfo>()
+                {
+                    new CompletedRebaseStepInfo(null, true),
+                    new CompletedRebaseStepInfo(new ObjectId("ebdea37ecf583fb7fa5c806a1c00b82f3987fbaa"), false),
+                };
+
+                Assert.Equal(expectedRebaseResults, rebaseResults);
+            }
+        }
+
         private void ConstructRebaseTestRepository(Repository repo)
         {
             // Constructs a graph that looks like:
@@ -425,7 +516,7 @@ private void ConstructRebaseTestRepository(Repository repo)
             repo.Stage(filePathC);
             commit = repo.Commit("commit 3", Constants.Signature, Constants.Signature, new CommitOptions());
 
-            repo.CreateBranch(masterBranch1Name, commit, Constants.Signature);
+            Branch masterBranch1 = repo.CreateBranch(masterBranch1Name, commit, Constants.Signature);
 
             Touch(workdir, filePathB, string.Join(lineEnding, fileContentB1, fileContentB2));
             repo.Stage(filePathB);
@@ -455,7 +546,7 @@ private void ConstructRebaseTestRepository(Repository repo)
 
             repo.CreateBranch(topicBranch2Name, commit, Constants.Signature);
 
-            repo.Checkout(masterBranch1Name);
+            repo.Checkout(masterBranch1.Tip);
             Touch(workdir, filePathD, fileContentD1);
             repo.Stage(filePathD);
             commit = repo.Commit("commit 10", Constants.Signature, Constants.Signature, new CommitOptions());

LibGit2Sharp/AfterRebaseStepInfo.cs

@@ -20,6 +20,17 @@ internal AfterRebaseStepInfo(RebaseStepInfo stepInfo, ObjectId commitId)
         {
             StepInfo = stepInfo;
             CommitId = commitId;
+            WasPatchAlreadyApplied = false;
+        }
+
+        /// <summary>
+        /// Constructor to call when the patch has already been applied for this step.
+        /// </summary>
+        /// <param name="stepInfo"></param>
+        internal AfterRebaseStepInfo(RebaseStepInfo stepInfo)
+            : this (stepInfo, null)
+        {
+            WasPatchAlreadyApplied = true;
         }
 
         /// <summary>
@@ -31,5 +42,11 @@ internal AfterRebaseStepInfo(RebaseStepInfo stepInfo, ObjectId commitId)
         /// The ID of the commit generated by the step, if any.
         /// </summary>
         public virtual ObjectId CommitId { get; private set; }
+
+        /// <summary>
+        /// Was the changes for this step already applied. If so,
+        /// <see cref="AfterRebaseStepInfo.CommitId"/> will be null.
+        /// </summary>
+        public virtual bool WasPatchAlreadyApplied { get; private set; }
     }
 }

LibGit2Sharp/Core/GitOid.cs

@@ -27,5 +27,13 @@ public static implicit operator ObjectId(GitOid? oid)
         {
             return oid == null ? null : new ObjectId(oid.Value);
         }
+
+        /// <summary>
+        /// Static convenience property to return an id (all zeros).
+        /// </summary>
+        public static GitOid Empty
+        {
+            get { return new GitOid(); }
+        }
     }
 }

LibGit2Sharp/Core/Proxy.cs

@@ -1661,15 +1661,15 @@ public static GitRebaseOperation git_rebase_next(RebaseSafeHandle rebase,
             return operation;
         }
 
-        public static GitOid git_rebase_commit(
+        public static GitRebaseCommitResult git_rebase_commit(
             RebaseSafeHandle rebase,
             Signature author,
             Signature committer)
         {
             Ensure.ArgumentNotNull(rebase, "rebase");
             Ensure.ArgumentNotNull(committer, "committer");
 
-            GitOid commitId = new GitOid();
+            GitRebaseCommitResult commitResult = new GitRebaseCommitResult();
 
             using (ThreadAffinity())
             {
@@ -1682,8 +1682,17 @@ public static GitOid git_rebase_commit(
                     authorHandle = author == null ? 
                         new SignatureSafeHandle() : author.BuildHandle();
 
-                    int result = NativeMethods.git_rebase_commit(ref commitId, rebase, authorHandle, committerHandle, IntPtr.Zero, IntPtr.Zero);
-                    Ensure.ZeroResult(result);
+                    int result = NativeMethods.git_rebase_commit(ref commitResult.CommitId, rebase, authorHandle, committerHandle, IntPtr.Zero, IntPtr.Zero);
+
+                    if (result == (int)GitErrorCode.Applied)
+                    {
+                        commitResult.CommitId = GitOid.Empty;
+                        commitResult.WasPatchAlreadyApplied = true;
+                    }
+                    else
+                    {
+                        Ensure.ZeroResult(result);
+                    }
                 }
                 finally
                 {
@@ -1695,7 +1704,24 @@ public static GitOid git_rebase_commit(
                 }
             }
 
-            return commitId;
+            return commitResult;
+        }
+
+        /// <summary>
+        /// Struct to report the result of calling git_rebase_commit.
+        /// </summary>
+        public struct GitRebaseCommitResult
+        {
+            /// <summary>
+            /// The ID of the commit that was generated, if any
+            /// </summary>
+            public GitOid CommitId;
+
+            /// <summary>
+            /// bool to indicate if the patch was already applied.
+            /// If Patch was already applied, then CommitId will be empty (all zeros).
+            /// </summary>
+            public bool WasPatchAlreadyApplied;
         }
 
         public static void git_rebase_abort(

LibGit2Sharp/Rebase.cs

@@ -173,6 +173,7 @@ public virtual RebaseResult Continue(Signature committer, RebaseOptions options)
             try
             {
                 rebase = Proxy.git_rebase_open(repository.Handle);
+                var rebaseCommitResult = Proxy.git_rebase_commit(rebase, null, committer);
 
                 // Report that we just completed the step
                 if (options.RebaseStepCompleted != null)
@@ -188,8 +189,14 @@ public virtual RebaseResult Continue(Signature committer, RebaseOptions options)
                                                       currentStepIndex,
                                                       totalStepCount);
 
-                    GitOid id = Proxy.git_rebase_commit(rebase, null, committer);
-                    options.RebaseStepCompleted(new AfterRebaseStepInfo(stepInfo, new ObjectId(id)));
+                    if (rebaseCommitResult.WasPatchAlreadyApplied)
+                    {
+                        options.RebaseStepCompleted(new AfterRebaseStepInfo(stepInfo));
+                    }
+                    else
+                    {
+                        options.RebaseStepCompleted(new AfterRebaseStepInfo(stepInfo, new ObjectId(rebaseCommitResult.CommitId)));
+                    }
                 }
 
                 RebaseResult rebaseResult = RebaseOperationImpl.Run(rebase, repository, committer, options);

LibGit2Sharp/RebaseOperationImpl.cs

@@ -78,12 +78,19 @@ public static RebaseResult Run(RebaseSafeHandle rebaseOperationHandle,
                                 // commit and continue.
                                 if (repository.Index.IsFullyMerged)
                                 {
-                                    GitOid id = Proxy.git_rebase_commit(rebaseOperationHandle, null, committer);
+                                    Proxy.GitRebaseCommitResult rebase_commit_result = Proxy.git_rebase_commit(rebaseOperationHandle, null, committer);
 
                                     // Report that we just completed the step
                                     if (options.RebaseStepCompleted != null)
                                     {
-                                        options.RebaseStepCompleted(new AfterRebaseStepInfo(stepInfo, new ObjectId(id)));
+                                        if (rebase_commit_result.WasPatchAlreadyApplied)
+                                        {
+                                            options.RebaseStepCompleted(new AfterRebaseStepInfo(stepInfo));
+                                        }
+                                        else
+                                        {
+                                            options.RebaseStepCompleted(new AfterRebaseStepInfo(stepInfo, new ObjectId(rebase_commit_result.CommitId)));
+                                        }
                                     }
                                 }
                                 else