From f1c398141a88822d897653d4aa85280d0100373d Mon Sep 17 00:00:00 2001 From: Kirill Likhodedov Date: Tue, 22 Jul 2014 21:10:11 +0400 Subject: [PATCH] [git] IDEA-127472 Correctly checkout from diverged branches to common branch + tests * Before branch operation starts, remember current branches for each repository, without assuming that the repositories are on the same branch. Use proper branch for each repository during rollback. * Simplify some user messages in a couple of places to avoid forming complex messages like "branch A in repository B", etc. However, use such complex message format to notify about merge result. --- .../git4idea/branch/GitBranchOperation.java | 65 +++++++++++++++++-- .../branch/GitCheckoutNewBranchOperation.java | 6 +- .../git4idea/branch/GitCheckoutOperation.java | 12 ++-- .../branch/GitDeleteBranchOperation.java | 2 +- .../git4idea/branch/GitMergeOperation.java | 6 +- .../branch/GitBranchWorkerTest.groovy | 40 ++++++++++-- .../tests/git4idea/test/GitScenarios.groovy | 7 +- 7 files changed, 114 insertions(+), 24 deletions(-) diff --git a/plugins/git4idea/src/git4idea/branch/GitBranchOperation.java b/plugins/git4idea/src/git4idea/branch/GitBranchOperation.java index 7c2f77eeebd2..ca8469c79b4d 100644 --- a/plugins/git4idea/src/git4idea/branch/GitBranchOperation.java +++ b/plugins/git4idea/src/git4idea/branch/GitBranchOperation.java @@ -25,6 +25,9 @@ import com.intellij.openapi.vcs.VcsNotifier; import com.intellij.openapi.vcs.changes.Change; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.util.Function; +import com.intellij.util.containers.ContainerUtil; +import com.intellij.util.containers.MultiMap; +import git4idea.GitLocalBranch; import git4idea.GitPlatformFacade; import git4idea.GitUtil; import git4idea.commands.Git; @@ -32,6 +35,7 @@ import git4idea.commands.GitMessageWithFilesDetector; import git4idea.config.GitVcsSettings; import git4idea.repo.GitRepository; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.util.*; @@ -50,7 +54,7 @@ abstract class GitBranchOperation { @NotNull protected final Git myGit; @NotNull protected final GitBranchUiHandler myUiHandler; @NotNull private final Collection myRepositories; - @NotNull protected final String myCurrentBranchOrRev; + @NotNull protected final Map myCurrentHeads; private final GitVcsSettings mySettings; @NotNull private final Collection mySuccessfulRepositories; @@ -63,7 +67,13 @@ abstract class GitBranchOperation { myGit = git; myUiHandler = uiHandler; myRepositories = repositories; - myCurrentBranchOrRev = GitBranchUtil.getCurrentBranchOrRev(repositories); + myCurrentHeads = ContainerUtil.map2Map(repositories, new Function>() { + @Override + public Pair fun(GitRepository repository) { + GitLocalBranch currentBranch = repository.getCurrentBranch(); + return Pair.create(repository, currentBranch == null ? repository.getCurrentRevision() : currentBranch.getName()); + } + }); mySuccessfulRepositories = new ArrayList(); myRemainingRepositories = new ArrayList(myRepositories); mySettings = myFacade.getSettings(myProject); @@ -220,13 +230,30 @@ abstract class GitBranchOperation { protected void updateRecentBranch() { if (getRepositories().size() == 1) { GitRepository repository = myRepositories.iterator().next(); - mySettings.setRecentBranchOfRepository(repository.getRoot().getPath(), myCurrentBranchOrRev); + mySettings.setRecentBranchOfRepository(repository.getRoot().getPath(), myCurrentHeads.get(repository)); } else { - mySettings.setRecentCommonBranch(myCurrentBranchOrRev); + String recentCommonBranch = getRecentCommonBranch(); + if (recentCommonBranch != null) { + mySettings.setRecentCommonBranch(recentCommonBranch); + } } } + @Nullable + private String getRecentCommonBranch() { + String recentCommonBranch = null; + for (String branch : myCurrentHeads.values()) { + if (recentCommonBranch == null) { + recentCommonBranch = branch; + } + else if (!recentCommonBranch.equals(branch)) { + return null; + } + } + return recentCommonBranch; + } + private void showUnmergedFilesDialogWithRollback() { boolean ok = myUiHandler.showUnmergedFilesMessageWithRollback(getOperationName(), getRollbackProposal()); if (ok) { @@ -340,4 +367,34 @@ abstract class GitBranchOperation { return Pair.create(allConflictingRepositories, affectedChanges); } + + @NotNull + protected static String stringifyBranchesByRepos(@NotNull Map heads) { + MultiMap grouped = groupByBranches(heads); + if (grouped.size() == 1) { + return grouped.keySet().iterator().next(); + } + return StringUtil.join(grouped.entrySet(), new Function>, String>() { + @Override + public String fun(Map.Entry> entry) { + String roots = StringUtil.join(entry.getValue(), new Function() { + @Override + public String fun(VirtualFile file) { + return file.getName(); + } + }, ", "); + return entry.getKey() + " (in " + roots + ")"; + } + }, "
"); + } + + @NotNull + private static MultiMap groupByBranches(@NotNull Map heads) { + MultiMap result = MultiMap.create(); + for (Map.Entry entry : heads.entrySet()) { + result.putValue(entry.getValue(), entry.getKey().getRoot()); + } + return result; + } + } diff --git a/plugins/git4idea/src/git4idea/branch/GitCheckoutNewBranchOperation.java b/plugins/git4idea/src/git4idea/branch/GitCheckoutNewBranchOperation.java index 22e486e6eab0..cdc4f7711801 100644 --- a/plugins/git4idea/src/git4idea/branch/GitCheckoutNewBranchOperation.java +++ b/plugins/git4idea/src/git4idea/branch/GitCheckoutNewBranchOperation.java @@ -89,7 +89,7 @@ class GitCheckoutNewBranchOperation extends GitBranchOperation { protected String getRollbackProposal() { return "However checkout has succeeded for the following " + repositories() + ":
" + successfulRepositoriesJoined() + - "
You may rollback (checkout back to " + myCurrentBranchOrRev + " and delete " + myNewBranchName + ") not to let branches diverge."; + "
You may rollback (checkout previous branch back, and delete " + myNewBranchName + ") not to let branches diverge."; } @NotNull @@ -104,7 +104,7 @@ class GitCheckoutNewBranchOperation extends GitBranchOperation { GitCompoundResult deleteResult = new GitCompoundResult(myProject); Collection repositories = getSuccessfulRepositories(); for (GitRepository repository : repositories) { - GitCommandResult result = myGit.checkout(repository, myCurrentBranchOrRev, null, true); + GitCommandResult result = myGit.checkout(repository, myCurrentHeads.get(repository), null, true); checkoutResult.append(repository, result); if (result.success()) { deleteResult.append(repository, myGit.branchDelete(repository, myNewBranchName, false)); @@ -113,7 +113,7 @@ class GitCheckoutNewBranchOperation extends GitBranchOperation { } if (checkoutResult.totalSuccess() && deleteResult.totalSuccess()) { VcsNotifier.getInstance(myProject).notifySuccess("Rollback successful", String - .format("Checked out %s and deleted %s on %s %s", code(myCurrentBranchOrRev), code(myNewBranchName), + .format("Checked out %s and deleted %s on %s %s", stringifyBranchesByRepos(myCurrentHeads), code(myNewBranchName), StringUtil.pluralize("root", repositories.size()), successfulRepositoriesJoined())); } else { diff --git a/plugins/git4idea/src/git4idea/branch/GitCheckoutOperation.java b/plugins/git4idea/src/git4idea/branch/GitCheckoutOperation.java index 2441bf7d46cf..e69f5ecfe8fc 100644 --- a/plugins/git4idea/src/git4idea/branch/GitCheckoutOperation.java +++ b/plugins/git4idea/src/git4idea/branch/GitCheckoutOperation.java @@ -47,7 +47,7 @@ import static git4idea.util.GitUIUtil.code; */ class GitCheckoutOperation extends GitBranchOperation { - public static final String ROLLBACK_PROPOSAL_FORMAT = "You may rollback (checkout back to %s) not to let branches diverge."; + public static final String ROLLBACK_PROPOSAL_FORMAT = "You may rollback (checkout back to previous branch) not to let branches diverge."; @NotNull private final String myStartPointReference; @Nullable private final String myNewBranch; @@ -115,7 +115,8 @@ class GitCheckoutOperation extends GitBranchOperation { private boolean smartCheckoutOrNotify(@NotNull GitRepository repository, @NotNull GitMessageWithFilesDetector localChangesOverwrittenByCheckout) { Pair, List> conflictingRepositoriesAndAffectedChanges = - getConflictingRepositoriesAndAffectedChanges(repository, localChangesOverwrittenByCheckout, myCurrentBranchOrRev, myStartPointReference); + getConflictingRepositoriesAndAffectedChanges(repository, localChangesOverwrittenByCheckout, myCurrentHeads.get(repository), + myStartPointReference); List allConflictingRepositories = conflictingRepositoriesAndAffectedChanges.getFirst(); List affectedChanges = conflictingRepositoriesAndAffectedChanges.getSecond(); @@ -153,8 +154,7 @@ class GitCheckoutOperation extends GitBranchOperation { @Override protected String getRollbackProposal() { return "However checkout has succeeded for the following " + repositories() + ":
" + - successfulRepositoriesJoined() + - "
" + String.format(ROLLBACK_PROPOSAL_FORMAT, myCurrentBranchOrRev); + successfulRepositoriesJoined() + "
" + ROLLBACK_PROPOSAL_FORMAT; } @NotNull @@ -168,7 +168,7 @@ class GitCheckoutOperation extends GitBranchOperation { GitCompoundResult checkoutResult = new GitCompoundResult(myProject); GitCompoundResult deleteResult = new GitCompoundResult(myProject); for (GitRepository repository : getSuccessfulRepositories()) { - GitCommandResult result = myGit.checkout(repository, myCurrentBranchOrRev, null, true); + GitCommandResult result = myGit.checkout(repository, myCurrentHeads.get(repository), null, true); checkoutResult.append(repository, result); if (result.success() && myNewBranch != null) { /* @@ -183,7 +183,7 @@ class GitCheckoutOperation extends GitBranchOperation { if (!checkoutResult.totalSuccess() || !deleteResult.totalSuccess()) { StringBuilder message = new StringBuilder(); if (!checkoutResult.totalSuccess()) { - message.append("Errors during checking out ").append(myCurrentBranchOrRev).append(": "); + message.append("Errors during checkout: "); message.append(checkoutResult.getErrorOutputWithReposIndication()); } if (!deleteResult.totalSuccess()) { diff --git a/plugins/git4idea/src/git4idea/branch/GitDeleteBranchOperation.java b/plugins/git4idea/src/git4idea/branch/GitDeleteBranchOperation.java index d3501caf7089..1dba1e07f6ac 100644 --- a/plugins/git4idea/src/git4idea/branch/GitDeleteBranchOperation.java +++ b/plugins/git4idea/src/git4idea/branch/GitDeleteBranchOperation.java @@ -68,7 +68,7 @@ class GitDeleteBranchOperation extends GitBranchOperation { else if (notFullyMergedDetector.hasHappened()) { String baseBranch = notMergedToUpstreamDetector.getBaseBranch(); if (baseBranch == null) { // GitBranchNotMergedToUpstreamDetector didn't happen - baseBranch = myCurrentBranchOrRev; + baseBranch = myCurrentHeads.get(repository); } Collection remainingRepositories = getRemainingRepositories(); diff --git a/plugins/git4idea/src/git4idea/branch/GitMergeOperation.java b/plugins/git4idea/src/git4idea/branch/GitMergeOperation.java index 89a50cfd6054..bf14a6050cac 100644 --- a/plugins/git4idea/src/git4idea/branch/GitMergeOperation.java +++ b/plugins/git4idea/src/git4idea/branch/GitMergeOperation.java @@ -189,7 +189,8 @@ class GitMergeOperation extends GitBranchOperation { private boolean proposeSmartMergePerformAndNotify(@NotNull GitRepository repository, @NotNull GitMessageWithFilesDetector localChangesOverwrittenByMerge) { Pair, List> conflictingRepositoriesAndAffectedChanges = - getConflictingRepositoriesAndAffectedChanges(repository, localChangesOverwrittenByMerge, myCurrentBranchOrRev, myBranchToMerge); + getConflictingRepositoriesAndAffectedChanges(repository, localChangesOverwrittenByMerge, myCurrentHeads.get(repository), + myBranchToMerge); List allConflictingRepositories = conflictingRepositoriesAndAffectedChanges.getFirst(); List affectedChanges = conflictingRepositoriesAndAffectedChanges.getSecond(); @@ -339,7 +340,8 @@ class GitMergeOperation extends GitBranchOperation { @NotNull @Override public String getSuccessMessage() { - return String.format("Merged %s to %s", myBranchToMerge, myCurrentBranchOrRev); + return String.format("Merged %s to %s", + myBranchToMerge, stringifyBranchesByRepos(myCurrentHeads)); } @NotNull diff --git a/plugins/git4idea/tests/git4idea/branch/GitBranchWorkerTest.groovy b/plugins/git4idea/tests/git4idea/branch/GitBranchWorkerTest.groovy index 491054197285..d838310c41c5 100644 --- a/plugins/git4idea/tests/git4idea/branch/GitBranchWorkerTest.groovy +++ b/plugins/git4idea/tests/git4idea/branch/GitBranchWorkerTest.groovy @@ -14,16 +14,13 @@ * limitations under the License. */ package git4idea.branch -import com.intellij.dvcs.test.MockVirtualFile import com.intellij.openapi.progress.ProgressIndicator import com.intellij.openapi.progress.util.ProgressIndicatorBase import com.intellij.openapi.project.Project import com.intellij.openapi.ui.DialogWrapper import com.intellij.openapi.util.io.FileUtil import com.intellij.openapi.util.text.StringUtil -import com.intellij.openapi.vcs.FilePathImpl import com.intellij.openapi.vcs.changes.Change -import com.intellij.openapi.vcs.changes.CurrentContentRevision import com.intellij.openapi.vfs.VirtualFile import com.intellij.util.Function import com.intellij.util.LineSeparator @@ -38,9 +35,7 @@ import org.jetbrains.annotations.NotNull import java.util.regex.Matcher -import static com.intellij.openapi.vcs.Executor.* -import static git4idea.test.GitExecutor.cd -import static git4idea.test.GitExecutor.git +import static git4idea.test.GitExecutor.* import static git4idea.test.GitScenarios.* class GitBranchWorkerTest extends GitPlatformTest { @@ -600,6 +595,39 @@ class GitBranchWorkerTest extends GitPlatformTest { assertEquals "Merge in ultimate should have been reset", ultimateTipAfterMerge, tip(myUltimate) } + public void test_checkout_in_detached_head() { + cd(myCommunity); + touch("file.txt", "some content"); + add("file.txt"); + commit("msg"); + git(myCommunity, "checkout HEAD^"); + + checkoutBranch("master", []); + assertCurrentBranch("master"); + } + + // inspired by IDEA-127472 + public void test_checkout_to_common_branch_when_branches_have_diverged() { + branchWithCommit(myUltimate, "feature", "feature-file.txt", "feature_content", false); + branchWithCommit(myCommunity, "newbranch", "newbranch-file.txt", "newbranch_content", false); + checkoutBranch("master", []) + assertCurrentBranch("master"); + } + + public void test_rollback_checkout_from_diverged_branches_should_return_to_proper_branches() { + branchWithCommit(myUltimate, "feature", "feature-file.txt", "feature_content", false); + branchWithCommit(myCommunity, "newbranch", "newbranch-file.txt", "newbranch_content", false); + unmergedFiles(myContrib) + + checkoutBranch "master", [ + showUnmergedFilesMessageWithRollback: { String s1, String s2 -> true }, + ] + + assertCurrentBranch(myUltimate, "feature"); + assertCurrentBranch(myCommunity, "newbranch"); + assertCurrentBranch(myContrib, "master"); + } + static def assertCurrentBranch(GitRepository repository, String name) { def curBranch = git(repository, "branch").split("\n").find { it -> it.contains("*") }.replace('*', ' ').trim() assertEquals("Current branch is incorrect in ${repository}", name, curBranch) diff --git a/plugins/git4idea/tests/git4idea/test/GitScenarios.groovy b/plugins/git4idea/tests/git4idea/test/GitScenarios.groovy index 77ecf0bc9e20..1fe574ad863d 100644 --- a/plugins/git4idea/tests/git4idea/test/GitScenarios.groovy +++ b/plugins/git4idea/tests/git4idea/test/GitScenarios.groovy @@ -38,14 +38,17 @@ class GitScenarios { /** * Create a branch with a commit and return back to master. */ - static def branchWithCommit(GitRepository repository, String name, String file = "branch_file.txt", String content = "branch content") { + static def branchWithCommit(GitRepository repository, String name, String file = "branch_file.txt", String content = "branch content", + boolean returnToMaster = true) { cd repository git("checkout -b $name") touch(file, content) git("add $file") git("commit -m branch_content") - git("checkout master") + if (returnToMaster) { + git("checkout master") + } } /**