From 7edbebca10d91eb111eb25fd61a466d12129ac7d Mon Sep 17 00:00:00 2001 From: Kirill Likhodedov Date: Sat, 25 Mar 2017 11:29:17 +0000 Subject: [PATCH] Support rejected multi-repo push if the branch in other repo is deleted Suppose the branch 'feature' is removed in repoA, and got a commit in repoB. Then push to repoB is rejected. Subsequent update fails, because the branch is removed in repoA. That shouldn't happen, because this fact doesn't affect pushing to repoB. The fix is to provide a flag not to check for such removed branches during update. Fixes IDEA-169877. --- .../src/git4idea/push/GitPushOperation.java | 2 +- .../git4idea/update/GitUpdateEnvironment.java | 2 +- .../src/git4idea/update/GitUpdateProcess.java | 17 ++++-- .../push/GitPushOperationMultiRepoTest.kt | 54 +++++++++++++++++-- .../git4idea/update/GitMultiRepoUpdateTest.kt | 4 +- .../update/GitSingleRepoUpdateTest.kt | 2 +- .../tests/git4idea/update/GitSubmoduleTest.kt | 2 +- 7 files changed, 67 insertions(+), 16 deletions(-) diff --git a/plugins/git4idea/src/git4idea/push/GitPushOperation.java b/plugins/git4idea/src/git4idea/push/GitPushOperation.java index a5f8cd9758fd..84b3c93ccbbd 100644 --- a/plugins/git4idea/src/git4idea/push/GitPushOperation.java +++ b/plugins/git4idea/src/git4idea/push/GitPushOperation.java @@ -412,7 +412,7 @@ public class GitPushOperation { boolean checkForRebaseOverMergeProblem) { GitUpdateResult updateResult = new GitUpdateProcess(myProject, myProgressIndicator, new HashSet<>(rootsToUpdate), UpdatedFiles.create(), - checkForRebaseOverMergeProblem).update(updateMethod); + checkForRebaseOverMergeProblem, false).update(updateMethod); for (GitRepository repository : rootsToUpdate) { repository.getRoot().refresh(true, true); repository.update(); diff --git a/plugins/git4idea/src/git4idea/update/GitUpdateEnvironment.java b/plugins/git4idea/src/git4idea/update/GitUpdateEnvironment.java index f2ad35e42ac8..2772ff9d92d1 100644 --- a/plugins/git4idea/src/git4idea/update/GitUpdateEnvironment.java +++ b/plugins/git4idea/src/git4idea/update/GitUpdateEnvironment.java @@ -56,7 +56,7 @@ public class GitUpdateEnvironment implements UpdateEnvironment { GitRepositoryManager repositoryManager = getRepositoryManager(myProject); final GitUpdateProcess gitUpdateProcess = new GitUpdateProcess(myProject, progressIndicator, getRepositoriesFromRoots(repositoryManager, roots), - updatedFiles, true); + updatedFiles, true, true); boolean result = gitUpdateProcess.update(mySettings.getUpdateType()).isSuccess(); return new GitUpdateSession(result); } diff --git a/plugins/git4idea/src/git4idea/update/GitUpdateProcess.java b/plugins/git4idea/src/git4idea/update/GitUpdateProcess.java index fe90eaecd949..6f0a1494b1fa 100644 --- a/plugins/git4idea/src/git4idea/update/GitUpdateProcess.java +++ b/plugins/git4idea/src/git4idea/update/GitUpdateProcess.java @@ -76,6 +76,7 @@ public class GitUpdateProcess { @NotNull private final List myRepositories; private final boolean myCheckRebaseOverMergeProblem; + private final boolean myCheckForTrackedBranchExistance; private final UpdatedFiles myUpdatedFiles; @NotNull private final ProgressIndicator myProgressIndicator; @NotNull private final GitMerger myMerger; @@ -84,9 +85,11 @@ public class GitUpdateProcess { @Nullable ProgressIndicator progressIndicator, @NotNull Collection repositories, @NotNull UpdatedFiles updatedFiles, - boolean checkRebaseOverMergeProblem) { + boolean checkRebaseOverMergeProblem, + boolean checkForTrackedBranchExistance) { myProject = project; myCheckRebaseOverMergeProblem = checkRebaseOverMergeProblem; + myCheckForTrackedBranchExistance = checkForTrackedBranchExistance; myGit = Git.getInstance(); myChangeListManager = ChangeListManager.getInstance(project); myVcsManager = ProjectLevelVcsManager.getInstance(project); @@ -285,6 +288,7 @@ public class GitUpdateProcess { LOG.info("updateImpl: defining updaters..."); for (GitRepository repository : myRepositories) { VirtualFile root = repository.getRoot(); + if (trackedBranches.get(root) == null) continue; GitUpdater updater = GitUpdater.getUpdater(myProject, myGit, trackedBranches, root, myProgressIndicator, myUpdatedFiles, updateMethod); if (updater.isUpdateNeeded()) { @@ -330,12 +334,15 @@ public class GitUpdateProcess { } GitBranchTrackInfo trackInfo = GitBranchUtil.getTrackInfoForBranch(repository, branch); if (trackInfo == null) { - final String branchName = branch.getName(); LOG.info(String.format("checkTrackedBranchesConfigured: no track info for current branch %s in %s", branch, repository)); - notifyImportantError(repository.getProject(), "Can't Update", getNoTrackedBranchError(repository, branchName)); - return null; + if (myCheckForTrackedBranchExistance) { + notifyImportantError(repository.getProject(), "Can't Update", getNoTrackedBranchError(repository, branch.getName())); + return null; + } + } + else { + trackedBranches.put(root, new GitBranchPair(branch, trackInfo.getRemoteBranch())); } - trackedBranches.put(root, new GitBranchPair(branch, trackInfo.getRemoteBranch())); } return trackedBranches; } diff --git a/plugins/git4idea/tests/git4idea/push/GitPushOperationMultiRepoTest.kt b/plugins/git4idea/tests/git4idea/push/GitPushOperationMultiRepoTest.kt index 6d2da6c7769b..483e51437c17 100644 --- a/plugins/git4idea/tests/git4idea/push/GitPushOperationMultiRepoTest.kt +++ b/plugins/git4idea/tests/git4idea/push/GitPushOperationMultiRepoTest.kt @@ -16,7 +16,7 @@ package git4idea.push import com.intellij.dvcs.push.PushSpec -import com.intellij.openapi.vcs.Executor +import com.intellij.openapi.vcs.Executor.cd import com.intellij.util.containers.ContainerUtil import git4idea.commands.GitCommandResult import git4idea.repo.GitRepository @@ -47,7 +47,7 @@ class GitPushOperationMultiRepoTest : GitPushOperationBaseTest() { community = enclosingRepo.projectRepo brommunity = enclosingRepo.bro - Executor.cd(myProjectPath) + cd(myProjectPath) refresh() updateRepositories() } @@ -80,10 +80,10 @@ class GitPushOperationMultiRepoTest : GitPushOperationBaseTest() { } fun test_update_all_roots_on_reject_when_needed_even_if_only_one_in_push_spec() { - Executor.cd(brultimate) + cd(brultimate) val broHash = makeCommit("bro.txt") git("push") - Executor.cd(brommunity) + cd(brommunity) val broCommunityHash = makeCommit("bro_com.txt") git("push") @@ -108,7 +108,51 @@ class GitPushOperationMultiRepoTest : GitPushOperationBaseTest() { cd(ultimate) val lastCommitParents = git("log -1 --pretty=%P").split(" ".toRegex()).dropLastWhile { it.isEmpty() }.toTypedArray() assertEquals("Merge didn't happen in main repository", 2, lastCommitParents.size) - assertEquals("Commit from bro repository didn't arrive", broHash, git("log --no-walk HEAD^2 --pretty=%H")) + assertRemoteCommitMerged("Commit from bro repository didn't arrive", broHash) } + // IDEA-169877 + fun `test push rejected in one repo when branch is deleted in another, should finally succeed`() { + listOf(brultimate, brommunity).forEach { + cd(it) + git("checkout -b feature") + git("push -u origin feature") + } + listOf(ultimate, community).forEach { + cd(it) + git("pull") + git("checkout -b feature origin/feature") + } + + // commit in one repo to reject the push + cd(brultimate) + val broHash = tac("bro.txt") + git("push") + // remove branch in another repo + cd(brommunity) + git("push origin :feature") + + cd(ultimate) + val commitToPush = tac("file.txt") + + listOf(ultimate, community).forEach { it.update() } + + agreeToUpdate(GitRejectedPushUpdateDialog.MERGE_EXIT_CODE) // auto-update-all-roots is selected by default + + // push only to 1 repo, otherwise the push would recreate the deleted branch, and the error won't reproduce + val pushSpecs = mapOf(ultimate to makePushSpec(ultimate, "feature", "origin/feature")) + val result = GitPushOperation(myProject, pushSupport, pushSpecs, null, false).execute() + + val result1 = result.results[ultimate]!! + assertResult(GitPushRepoResult.Type.SUCCESS, 2, "feature", "origin/feature", GitUpdateResult.SUCCESS, result1) + assertRemoteCommitMerged("Commit from bro repository didn't arrive", broHash) + + cd(brultimate) + git("pull origin feature") + assertEquals("Commit from ultimate repository wasn't pushed", commitToPush, git("log --no-walk HEAD^1 --pretty=%H")) + } + + private fun assertRemoteCommitMerged(message: String, expectedHash: String) { + assertEquals(message, expectedHash, git("log --no-walk HEAD^2 --pretty=%H")) + } } diff --git a/plugins/git4idea/tests/git4idea/update/GitMultiRepoUpdateTest.kt b/plugins/git4idea/tests/git4idea/update/GitMultiRepoUpdateTest.kt index fd276ecbb363..b8a6ace3cabc 100644 --- a/plugins/git4idea/tests/git4idea/update/GitMultiRepoUpdateTest.kt +++ b/plugins/git4idea/tests/git4idea/update/GitMultiRepoUpdateTest.kt @@ -85,7 +85,7 @@ class GitMultiRepoUpdateTest : GitUpdateBaseTest() { cd(bromunity) git("push origin :feature") - val updateProcess = GitUpdateProcess(myProject, EmptyProgressIndicator(), repositories(), UpdatedFiles.create(), false) + val updateProcess = GitUpdateProcess(myProject, EmptyProgressIndicator(), repositories(), UpdatedFiles.create(), false, true) val result = updateProcess.update(UpdateMethod.MERGE) assertEquals("Update result is incorrect", GitUpdateResult.NOT_READY, result) @@ -93,7 +93,7 @@ class GitMultiRepoUpdateTest : GitUpdateBaseTest() { } private fun updateWithMerge(): GitUpdateResult { - return GitUpdateProcess(myProject, EmptyProgressIndicator(), repositories(), UpdatedFiles.create(), false).update(UpdateMethod.MERGE) + return GitUpdateProcess(myProject, EmptyProgressIndicator(), repositories(), UpdatedFiles.create(), false, true).update(UpdateMethod.MERGE) } private fun repositories() = listOf(repository, community) diff --git a/plugins/git4idea/tests/git4idea/update/GitSingleRepoUpdateTest.kt b/plugins/git4idea/tests/git4idea/update/GitSingleRepoUpdateTest.kt index c603d5ea34e0..ee857b5b0be3 100644 --- a/plugins/git4idea/tests/git4idea/update/GitSingleRepoUpdateTest.kt +++ b/plugins/git4idea/tests/git4idea/update/GitSingleRepoUpdateTest.kt @@ -96,7 +96,7 @@ class GitSingleRepoUpdateTest : GitUpdateBaseTest() { } private fun updateWithRebase(): GitUpdateResult { - return GitUpdateProcess(myProject, EmptyProgressIndicator(), listOf(repo), UpdatedFiles.create(), false).update(UpdateMethod.REBASE) + return GitUpdateProcess(myProject, EmptyProgressIndicator(), listOf(repo), UpdatedFiles.create(), false, true).update(UpdateMethod.REBASE) } private fun File.commitAndPush() { diff --git a/plugins/git4idea/tests/git4idea/update/GitSubmoduleTest.kt b/plugins/git4idea/tests/git4idea/update/GitSubmoduleTest.kt index 37e96d3ba4f7..8b52d24f7421 100644 --- a/plugins/git4idea/tests/git4idea/update/GitSubmoduleTest.kt +++ b/plugins/git4idea/tests/git4idea/update/GitSubmoduleTest.kt @@ -81,7 +81,7 @@ class GitSubmoduleTest : GitPlatformTest() { reposInActualOrder.add(it) } - val updateProcess = GitUpdateProcess(myProject, EmptyProgressIndicator(), allRepositories(), UpdatedFiles.create(), false) + val updateProcess = GitUpdateProcess(myProject, EmptyProgressIndicator(), allRepositories(), UpdatedFiles.create(), false, true) val result = updateProcess.update(UpdateMethod.MERGE) assertEquals("Incorrect update result", GitUpdateResult.SUCCESS, result) assertOrder(reposInActualOrder)