From fe623342687b381a1cabe3d7f39f95d77f38243d Mon Sep 17 00:00:00 2001 From: Kirill Likhodedov Date: Sun, 11 Jun 2017 12:21:44 +0300 Subject: [PATCH] Prohibit undo git reword if HEAD changes, or commit was pushed Expire the notification if the repository has changed, and check when "Undo" is pressed as well. The reworded commit changes its hash on reword, so we have to find the commit: * Get the number of rebased commits: `git log parent..newHead` (parent didn't change) * The last commit in the row is the one we started rebase from, i.e. the one which was reworded. * Double-check it by comparing the commit message. IDEA-173634 --- .../git4idea/rebase/GitCommitEditingAction.kt | 13 +--- .../src/git4idea/rebase/GitRebaseUtils.kt | 33 ++++++++ .../src/git4idea/rebase/GitRewordOperation.kt | 78 ++++++++++++++++++- .../tests/git4idea/rebase/GitRewordTest.kt | 40 ++++++++++ .../tests/git4idea/test/GitTestAssertions.kt | 14 +++- 5 files changed, 163 insertions(+), 15 deletions(-) create mode 100644 plugins/git4idea/src/git4idea/rebase/GitRebaseUtils.kt diff --git a/plugins/git4idea/src/git4idea/rebase/GitCommitEditingAction.kt b/plugins/git4idea/src/git4idea/rebase/GitCommitEditingAction.kt index 92e655c8e9c9..59e6b255f673 100644 --- a/plugins/git4idea/src/git4idea/rebase/GitCommitEditingAction.kt +++ b/plugins/git4idea/src/git4idea/rebase/GitCommitEditingAction.kt @@ -26,8 +26,6 @@ import com.intellij.vcs.log.VcsLogDataKeys import com.intellij.vcs.log.data.VcsLogData import git4idea.GitUtil.HEAD import git4idea.GitUtil.getRepositoryManager -import git4idea.config.GitSharedSettings -import git4idea.repo.GitRepository /** * Base class for Git action which is going to edit existing commits, @@ -133,15 +131,6 @@ abstract class GitCommitEditingAction : DumbAwareAction() { }, "Searching for branches containing the selected commit", true, data.project) } - protected fun findProtectedRemoteBranch(repository: GitRepository, branches: Collection): String? { - val settings = GitSharedSettings.getInstance(repository.project) - // protected branches hold patterns for branch names without remote names - return repository.branches.remoteBranches. - filter { settings.isBranchProtected(it.nameForRemoteOperations) }. - map { it.nameForLocalOperations }. - filter { branches.contains(it) }.firstOrNull() - } - private fun commitPushedToProtectedBranchError(protectedBranch: String) = "The commit is already pushed to protected branch '$protectedBranch'" -} \ No newline at end of file +} diff --git a/plugins/git4idea/src/git4idea/rebase/GitRebaseUtils.kt b/plugins/git4idea/src/git4idea/rebase/GitRebaseUtils.kt new file mode 100644 index 000000000000..1db935bbe8d4 --- /dev/null +++ b/plugins/git4idea/src/git4idea/rebase/GitRebaseUtils.kt @@ -0,0 +1,33 @@ +/* + * Copyright 2000-2017 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package git4idea.rebase + +import git4idea.config.GitSharedSettings +import git4idea.repo.GitRepository + +/** + * Checks if there is a protected remote branch among the given branches, and returns one of them, or `null` otherwise. + * + * `branches` are given in the "local" format, e.g. `origin/master`. + */ +fun findProtectedRemoteBranch(repository: GitRepository, branches: Collection): String? { + val settings = GitSharedSettings.getInstance(repository.project) + // protected branches hold patterns for branch names without remote names + return repository.branches.remoteBranches. + filter { settings.isBranchProtected(it.nameForRemoteOperations) }. + map { it.nameForLocalOperations }. + filter { branches.contains(it) }.firstOrNull() +} diff --git a/plugins/git4idea/src/git4idea/rebase/GitRewordOperation.kt b/plugins/git4idea/src/git4idea/rebase/GitRewordOperation.kt index 50041bd1e70a..e53532a25f3f 100644 --- a/plugins/git4idea/src/git4idea/rebase/GitRewordOperation.kt +++ b/plugins/git4idea/src/git4idea/rebase/GitRewordOperation.kt @@ -19,19 +19,26 @@ import com.intellij.notification.Notification import com.intellij.notification.NotificationAction import com.intellij.notification.NotificationType import com.intellij.openapi.actionSystem.AnActionEvent +import com.intellij.openapi.application.ApplicationManager +import com.intellij.openapi.diagnostic.logger import com.intellij.openapi.progress.EmptyProgressIndicator import com.intellij.openapi.progress.ProgressIndicator import com.intellij.openapi.progress.ProgressManager import com.intellij.openapi.progress.Task +import com.intellij.openapi.project.Project import com.intellij.openapi.vcs.VcsNotifier import com.intellij.openapi.vcs.VcsNotifier.STANDARD_NOTIFICATION import com.intellij.util.containers.MultiMap +import com.intellij.vcs.log.Hash import com.intellij.vcs.log.VcsCommitMetadata +import git4idea.branch.GitBranchUtil import git4idea.branch.GitRebaseParams import git4idea.commands.Git +import git4idea.history.GitHistoryUtils import git4idea.rebase.GitRebaseEntry.Action.pick import git4idea.rebase.GitRebaseEntry.Action.reword import git4idea.repo.GitRepository +import git4idea.repo.GitRepositoryChangeListener import git4idea.reset.GitResetMode class GitRewordOperation(private val repository: GitRepository, @@ -40,8 +47,14 @@ class GitRewordOperation(private val repository: GitRepository, init { repository.update() } + + private val LOG = logger() private val project = repository.project + private val notifier = VcsNotifier.getInstance(project) + private val initialHeadPosition = repository.currentRevision!! + private var headAfterReword: String? = null + private var rewordedCommit: Hash? = null fun execute() { val rebaseEditor = GitAutomaticRebaseEditor(project, commit.root, @@ -52,12 +65,26 @@ class GitRewordOperation(private val repository: GitRepository, val indicator = ProgressManager.getInstance().progressIndicator ?: EmptyProgressIndicator() val spec = GitRebaseSpec.forNewRebase(project, params, listOf(repository), indicator) RewordProcess(spec).rebase() + headAfterReword = repository.currentRevision + rewordedCommit = findNewHashOfRewordedCommit(headAfterReword!!) } internal fun undo() { + val possibility = checkUndoPossibility(project) + val errorTitle = "Can't Undo Reword" + when (possibility) { + is UndoPossibility.HeadMoved -> notifier.notifyError(errorTitle, "Repository has already been changed") + is UndoPossibility.PushedToProtectedBranch -> + notifier.notifyError(errorTitle, "Commit has already been pushed to ${possibility.branch}") + is Error -> notifier.notifyError(errorTitle, "") + else -> doUndo() + } + } + + private fun doUndo() { val res = Git.getInstance().reset(repository, GitResetMode.KEEP, initialHeadPosition) if (!res.success()) { - VcsNotifier.getInstance(project).notifyError("Undo Reword Failed", res.errorOutputAsHtmlString) + notifier.notifyError("Undo Reword Failed", res.errorOutputAsHtmlString) } } @@ -78,6 +105,44 @@ class GitRewordOperation(private val repository: GitRepository, } } + private fun findNewHashOfRewordedCommit(newHead: String): Hash? { + val newCommitsRange = "${commit.parents.first().asString()}..$newHead" + val newCommits = GitHistoryUtils.loadMetadata(project, repository.root, newCommitsRange).commits + if (newCommits.isEmpty()) { + LOG.error("Couldn't find commits after reword in range $newCommitsRange") + return null + } + val newCommit = newCommits.last() + if (newCommit.fullMessage != newMessage) { + LOG.error("Couldn't find the reworded commit. Expected message: \n[$newMessage]\nActual message: \n[${newCommit.fullMessage}]") + return null + } + return newCommit.id + } + + private fun checkUndoPossibility(project: Project): UndoPossibility { + repository.update() + if (repository.currentRevision != headAfterReword) { + return UndoPossibility.HeadMoved + } + + if (rewordedCommit == null) { + LOG.error("Couldn't find the reworded commit") + return UndoPossibility.Error + } + val containingBranches = GitBranchUtil.getBranches(project, repository.root, false, true, rewordedCommit!!.asString()) + val protectedBranch = findProtectedRemoteBranch(repository, containingBranches) + if (protectedBranch != null) return UndoPossibility.PushedToProtectedBranch(protectedBranch) + return UndoPossibility.Possible + } + + private sealed class UndoPossibility { + object Possible : UndoPossibility() + object HeadMoved : UndoPossibility() + class PushedToProtectedBranch(val branch: String) : UndoPossibility() + object Error : UndoPossibility() + } + private inner class RewordProcess(spec: GitRebaseSpec) : GitRebaseProcess(project, spec, null) { override fun notifySuccess(successful: MutableMap, skippedCommits: MultiMap) { @@ -88,7 +153,16 @@ class GitRewordOperation(private val repository: GitRepository, undoInBackground() } }) - VcsNotifier.getInstance(project).notify(notification) + + val connection = project.messageBus.connect() + notification.whenExpired { connection.disconnect() } + connection.subscribe(GitRepository.GIT_REPO_CHANGE, GitRepositoryChangeListener { + ApplicationManager.getApplication().executeOnPooledThread { + if (checkUndoPossibility(project) !is UndoPossibility.Possible) notification.expire() + } + }) + + notifier.notify(notification) } override fun shouldRefreshOnSuccess(successType: GitSuccessfulRebase.SuccessType) = false diff --git a/plugins/git4idea/tests/git4idea/rebase/GitRewordTest.kt b/plugins/git4idea/tests/git4idea/rebase/GitRewordTest.kt index 555376cbb896..687e8abab4bd 100644 --- a/plugins/git4idea/tests/git4idea/rebase/GitRewordTest.kt +++ b/plugins/git4idea/tests/git4idea/rebase/GitRewordTest.kt @@ -16,6 +16,7 @@ package git4idea.rebase import git4idea.test.GitSingleRepoTest +import git4idea.test.assertLatestHistory import git4idea.test.file import git4idea.test.git @@ -50,4 +51,43 @@ class GitRewordTest : GitSingleRepoTest() { assertEquals("Message reworded incorrectly", "Wrong message", git("log HEAD --no-walk --pretty=%B")) } + + fun `test undo is not possible if HEAD moved`() { + val commit = file("a").create("initial").addCommit("Wrong message").details() + + val operation = GitRewordOperation(myRepo, commit, "Correct message") + operation.execute() + + file("b").create().addCommit("New commit") + + operation.undo() + + myRepo.assertLatestHistory( + "New commit", + "Correct message" + ) + assertErrorNotification("Can't Undo Reword", "Repository has already been changed") + } + + fun `test undo is not possible if commit was pushed`() { + git("remote add origin http://example.git") + val file = file("a").create("initial") + file.append("First commit\n").addCommit("First commit") + val commit = file.append("To reword\n").addCommit("Wrong message").details() + file.append("Third commit").addCommit("Third commit") + + val operation = GitRewordOperation(myRepo, commit, "Correct message") + operation.execute() + + git("update-ref refs/remotes/origin/master HEAD") + + operation.undo() + + myRepo.assertLatestHistory( + "Third commit", + "Correct message", + "First commit" + ) + assertErrorNotification("Can't Undo Reword", "Commit has already been pushed to origin/master") + } } \ No newline at end of file diff --git a/plugins/git4idea/tests/git4idea/test/GitTestAssertions.kt b/plugins/git4idea/tests/git4idea/test/GitTestAssertions.kt index 314191627c34..fe929f65dc53 100644 --- a/plugins/git4idea/tests/git4idea/test/GitTestAssertions.kt +++ b/plugins/git4idea/tests/git4idea/test/GitTestAssertions.kt @@ -17,7 +17,9 @@ package git4idea.test import com.intellij.openapi.vcs.FilePath import com.intellij.openapi.vfs.VirtualFile +import com.intellij.testFramework.PlatformTestCase.assertOrderedEquals import com.intellij.vcsUtil.VcsUtil.getFilePath +import git4idea.history.GitHistoryUtils import git4idea.repo.GitRepository import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue @@ -35,4 +37,14 @@ fun GitRepository.assertStatus(file: File, status: Char) { val actualStatus = git(this, "status --porcelain ${file.path}") assertTrue("File status is not-changed: $actualStatus", !actualStatus.isEmpty()) assertEquals("File status is incorrect: $actualStatus", status, actualStatus[0]) -} \ No newline at end of file +} + +/** + * Checks the latest part of git history by commit messages. + */ +fun GitRepository.assertLatestHistory(vararg expectedMessages: String) { + val actualMessages = GitHistoryUtils.loadMetadata(this.project, this.root).commits + .map { it.fullMessage } + .subList(0, expectedMessages.size) + assertOrderedEquals("History is incorrect", actualMessages, expectedMessages.asList()) +}