From 7776cdba52e5d740677eb5eea3518c9fbf56ae75 Mon Sep 17 00:00:00 2001 From: Jameson Miller Date: Fri, 5 Sep 2014 16:51:59 -0400 Subject: [PATCH 1/2] Revert should clean up repository state when there is nothing to revert If reverting with the option to commit on success, and the revert completes successfully but there are no changes to commit, then revert will clean up the revert operation in progress state and the RevertStatus will indicate that there was nothing to revert. --- LibGit2Sharp.Tests/RevertFixture.cs | 43 +++++++++++++++++++++++++++++ LibGit2Sharp/Repository.cs | 38 +++++++++++++++++++++++-- LibGit2Sharp/RevertOptions.cs | 12 +++++++- LibGit2Sharp/RevertResult.cs | 7 ++++- 4 files changed, 96 insertions(+), 4 deletions(-) diff --git a/LibGit2Sharp.Tests/RevertFixture.cs b/LibGit2Sharp.Tests/RevertFixture.cs index 046179b17..39e64d53d 100644 --- a/LibGit2Sharp.Tests/RevertFixture.cs +++ b/LibGit2Sharp.Tests/RevertFixture.cs @@ -391,5 +391,48 @@ public void CanNotRevertAMergeCommitWithoutSpecifyingTheMainlineBranch() Assert.Throws(() => repo.Revert(commitToRevert, Constants.Signature)); } } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void RevertWithNothingToRevert(bool commitOnSuccess) + { + // The branch name to perform the revert on + const string revertBranchName = "refs/heads/revert"; + + string path = CloneRevertTestRepo(); + using (var repo = new Repository(path)) + { + // Checkout the revert branch. + Branch branch = repo.Checkout(revertBranchName); + Assert.NotNull(branch); + + Commit commitToRevert = repo.Head.Tip; + + // Revert tip commit. + RevertResult result = repo.Revert(commitToRevert, Constants.Signature); + Assert.NotNull(result); + Assert.Equal(RevertStatus.Reverted, result.Status); + + // Revert the same commit a second time + result = repo.Revert( + commitToRevert, + Constants.Signature, + new RevertOptions() { CommitOnSuccess = commitOnSuccess }); + + Assert.NotNull(result); + Assert.Equal(null, result.Commit); + Assert.Equal(RevertStatus.NothingToRevert, result.Status); + + if (commitOnSuccess) + { + Assert.Equal(CurrentOperation.None, repo.Info.CurrentOperation); + } + else + { + Assert.Equal(CurrentOperation.Revert, repo.Info.CurrentOperation); + } + } + } } } diff --git a/LibGit2Sharp/Repository.cs b/LibGit2Sharp/Repository.cs index 6773fe503..fb249390f 100644 --- a/LibGit2Sharp/Repository.cs +++ b/LibGit2Sharp/Repository.cs @@ -1084,6 +1084,14 @@ internal MergeResult MergeFetchHeads(Signature merger, MergeOptions options) /// /// Revert the specified commit. + /// + /// If the revert is successful but there are no changes to commit, + /// then the will be . + /// If the revert is successful and there are changes to revert, then + /// the will be . + /// If the revert resulted in conflicts, then the + /// will be . + /// /// /// The to revert. /// The of who is performing the revert. @@ -1123,12 +1131,38 @@ public RevertResult Revert(Commit commit, Signature reverter, RevertOptions opti if (Index.IsFullyMerged) { Commit revertCommit = null; + + // Check if the revert generated any changes + // and set the revert status accordingly + bool anythingToRevert = Index.RetrieveStatus( + new StatusOptions() + { + DetectRenamesInIndex = false, + Show = StatusShowOption.IndexOnly + }).Any(); + + RevertStatus revertStatus = anythingToRevert ? + RevertStatus.Reverted : RevertStatus.NothingToRevert; + if (options.CommitOnSuccess) { - revertCommit = this.Commit(Info.Message, author: reverter, committer: reverter); + if (!anythingToRevert) + { + // If there were no changes to revert, and we are + // asked to commit the changes, then cleanup + // the repository state (following command line behavior). + Proxy.git_repository_state_cleanup(handle); + } + else + { + revertCommit = this.Commit( + Info.Message, + author: reverter, + committer: reverter); + } } - result = new RevertResult(RevertStatus.Reverted, revertCommit); + result = new RevertResult(revertStatus, revertCommit); } else { diff --git a/LibGit2Sharp/RevertOptions.cs b/LibGit2Sharp/RevertOptions.cs index 2e48f04ed..8b1a103bc 100644 --- a/LibGit2Sharp/RevertOptions.cs +++ b/LibGit2Sharp/RevertOptions.cs @@ -43,7 +43,17 @@ public RevertOptions() public CheckoutNotifyHandler OnCheckoutNotify { get; set; } /// - /// Commit the revert if the revert is successful. + /// Commit changes if there are no conflicts and the revert results + /// in changes. + /// + /// Following command line behavior, if the revert results in no + /// changes, then Revert will cleanup the repository state if + /// is true (i.e. the repository + /// will not be left in a "revert in progress" state). + /// If is false and there are no + /// changes to revert, then the repository will be left in + /// the "revert in progress" state. + /// /// public bool CommitOnSuccess { get; set; } diff --git a/LibGit2Sharp/RevertResult.cs b/LibGit2Sharp/RevertResult.cs index bb8700c79..813903a76 100644 --- a/LibGit2Sharp/RevertResult.cs +++ b/LibGit2Sharp/RevertResult.cs @@ -52,6 +52,11 @@ public enum RevertStatus /// /// The revert resulted in conflicts. /// - Conflicts + Conflicts, + + /// + /// Revert was run, but there were no changes to commit. + /// + NothingToRevert, } } From 329c65204c5ece249e8b98b9d895039de58fcb94 Mon Sep 17 00:00:00 2001 From: Jameson Miller Date: Sat, 6 Sep 2014 22:15:06 -0400 Subject: [PATCH 2/2] Revert should throw if HEAD branch is orphaned --- LibGit2Sharp.Tests/RevertFixture.cs | 24 ++++++++++++++++++++++++ LibGit2Sharp/Repository.cs | 5 +++++ 2 files changed, 29 insertions(+) diff --git a/LibGit2Sharp.Tests/RevertFixture.cs b/LibGit2Sharp.Tests/RevertFixture.cs index 39e64d53d..13ab2c3a4 100644 --- a/LibGit2Sharp.Tests/RevertFixture.cs +++ b/LibGit2Sharp.Tests/RevertFixture.cs @@ -434,5 +434,29 @@ public void RevertWithNothingToRevert(bool commitOnSuccess) } } } + + [Fact] + public void RevertOrphanedBranchThrows() + { + // The branch name to perform the revert on + const string revertBranchName = "refs/heads/revert"; + + string path = CloneRevertTestRepo(); + using (var repo = new Repository(path)) + { + // Checkout the revert branch. + Branch branch = repo.Checkout(revertBranchName); + Assert.NotNull(branch); + + Commit commitToRevert = repo.Head.Tip; + + // Move the HEAD to an orphaned branch. + repo.Refs.UpdateTarget("HEAD", "refs/heads/orphan"); + Assert.True(repo.Info.IsHeadUnborn); + + // Revert the tip of the refs/heads/revert branch. + Assert.Throws(() => repo.Revert(commitToRevert, Constants.Signature)); + } + } } } diff --git a/LibGit2Sharp/Repository.cs b/LibGit2Sharp/Repository.cs index fb249390f..8e811ca57 100644 --- a/LibGit2Sharp/Repository.cs +++ b/LibGit2Sharp/Repository.cs @@ -1102,6 +1102,11 @@ public RevertResult Revert(Commit commit, Signature reverter, RevertOptions opti Ensure.ArgumentNotNull(commit, "commit"); Ensure.ArgumentNotNull(reverter, "reverter"); + if (Info.IsHeadUnborn) + { + throw new UnbornBranchException("Can not revert the commit. The Head doesn't point at a commit."); + } + options = options ?? new RevertOptions(); RevertResult result = null;