From 85429d76a34835445ac413a501d5040c49fcd646 Mon Sep 17 00:00:00 2001 From: Kirill Likhodedov Date: Sun, 1 Apr 2012 14:18:11 +0400 Subject: [PATCH] IDEA-83604 Deleting branch unmerged to upstream If branch is merged to the HEAD, but not merged to the upstream, it also can't be deleted by 'git branch -d'. Capture this case with additional detector, and use baseBranch parameter (which is either current branch or the upstream branch) to make the list of unmerged commits. The pattern is checked for 1.7.0.0 and 1.7.9.2 Write a test for this situation. --- .../GitBranchIsNotFullyMergedDialog.java | 21 +++--- .../branch/GitDeleteBranchOperation.java | 65 +++++++++++++++---- .../src/git4idea/i18n/GitBundle.properties | 4 +- .../branch/GitBranchOperationsTest.java | 38 +++++++++++ .../git4idea/tests/git4idea/test/GitExec.java | 26 ++++++++ 5 files changed, 128 insertions(+), 26 deletions(-) diff --git a/plugins/git4idea/src/git4idea/branch/GitBranchIsNotFullyMergedDialog.java b/plugins/git4idea/src/git4idea/branch/GitBranchIsNotFullyMergedDialog.java index e7a6aa7ab715..ff360ae119e7 100644 --- a/plugins/git4idea/src/git4idea/branch/GitBranchIsNotFullyMergedDialog.java +++ b/plugins/git4idea/src/git4idea/branch/GitBranchIsNotFullyMergedDialog.java @@ -54,7 +54,7 @@ public class GitBranchIsNotFullyMergedDialog extends DialogWrapper { private final Project myProject; private final Map> myCommits; private final String myBranchToDelete; - private final String myCurrentBranch; + private final String myBaseBranch; private final List myMergedToBranches; private final GitCommitListWithDiffPanel myCommitListWithDiffPanel; @@ -68,15 +68,15 @@ public class GitBranchIsNotFullyMergedDialog extends DialogWrapper { * grouped by repository. * @param branchToDelete the name of the branch which user chose to delete. * @param mergedToBranches the list of branches which the branch is merged to (returned by {@code git branch --merged } command. - * @param currentBranch + * @param baseBranch branch which branchToDelete is not merged to. It is either current branch, or the upstream branch. * @return true if user decided to delete the branch. */ public static boolean showAndGetAnswer(@NotNull Project project, @NotNull Map> commits, @NotNull String branchToDelete, @NotNull List mergedToBranches, - @Nullable String currentBranch) { - GitBranchIsNotFullyMergedDialog dialog = new GitBranchIsNotFullyMergedDialog(project, commits, branchToDelete, currentBranch, mergedToBranches); + @Nullable String baseBranch) { + GitBranchIsNotFullyMergedDialog dialog = new GitBranchIsNotFullyMergedDialog(project, commits, branchToDelete, baseBranch, mergedToBranches); ServiceManager.getService(project, PlatformFacade.class).showDialog(dialog); return dialog.isOK(); } @@ -84,13 +84,13 @@ public class GitBranchIsNotFullyMergedDialog extends DialogWrapper { private GitBranchIsNotFullyMergedDialog(@NotNull Project project, @NotNull Map> commits, @NotNull String branchToDelete, - @Nullable String currentBranch, + @Nullable String baseBranch, @NotNull List mergedToBranches) { super(project, false); myProject = project; myCommits = commits; myBranchToDelete = branchToDelete; - myCurrentBranch = currentBranch; + myBaseBranch = baseBranch; myMergedToBranches = mergedToBranches; myRepositories = commits.keySet(); @@ -118,8 +118,8 @@ public class GitBranchIsNotFullyMergedDialog extends DialogWrapper { String currentBranchOrRev; boolean onBranch; if (myRepositories.size() > 1) { - LOG.assertTrue(myCurrentBranch != null, "Branches have unexpectedly diverged"); - currentBranchOrRev = myCurrentBranch; + LOG.assertTrue(myBaseBranch != null, "Branches have unexpectedly diverged"); + currentBranchOrRev = myBaseBranch; onBranch = true; } else { @@ -138,9 +138,10 @@ public class GitBranchIsNotFullyMergedDialog extends DialogWrapper { StringBuilder description = new StringBuilder(); if (onBranch) { - description.append(GitBundle.message("branch.delete.not_fully_merged.description", myBranchToDelete, currentBranchOrRev)); + description.append(GitBundle.message("branch.delete.not_fully_merged.description", myBranchToDelete, myBaseBranch)); } else { - description.append(GitBundle.message("branch.delete.not_fully_merged.description.not_on_branch", myBranchToDelete, currentBranchOrRev)); + description.append(GitBundle.message("branch.delete.not_fully_merged.description.not_on_branch", myBranchToDelete, currentBranchOrRev, + myBaseBranch)); } if (!myMergedToBranches.isEmpty()) { String listOfMergedBranches = StringUtil.join(StringUtil.surround(ArrayUtil.toStringArray(myMergedToBranches), "", ""), ", "); diff --git a/plugins/git4idea/src/git4idea/branch/GitDeleteBranchOperation.java b/plugins/git4idea/src/git4idea/branch/GitDeleteBranchOperation.java index 1ff5492dfb34..b1e9ec5c0fc6 100644 --- a/plugins/git4idea/src/git4idea/branch/GitDeleteBranchOperation.java +++ b/plugins/git4idea/src/git4idea/branch/GitDeleteBranchOperation.java @@ -19,15 +19,13 @@ import com.intellij.notification.NotificationType; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.progress.ProgressIndicator; import com.intellij.openapi.project.Project; +import com.intellij.openapi.util.Key; import com.intellij.openapi.vcs.VcsException; import com.intellij.util.ArrayUtil; import com.intellij.util.ui.UIUtil; -import git4idea.GitVcs; -import git4idea.commands.Git; import git4idea.GitExecutionException; -import git4idea.commands.GitCommandResult; -import git4idea.commands.GitCompoundResult; -import git4idea.commands.GitSimpleEventDetector; +import git4idea.GitVcs; +import git4idea.commands.*; import git4idea.history.GitHistoryUtils; import git4idea.history.browser.GitCommit; import git4idea.repo.GitRepository; @@ -37,6 +35,8 @@ import org.jetbrains.annotations.Nullable; import java.util.*; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.regex.Matcher; +import java.util.regex.Pattern; /** * Deletes a branch. @@ -66,15 +66,21 @@ class GitDeleteBranchOperation extends GitBranchOperation { final GitRepository repository = next(); GitSimpleEventDetector notFullyMergedDetector = new GitSimpleEventDetector(GitSimpleEventDetector.Event.BRANCH_NOT_FULLY_MERGED); - GitCommandResult result = myGit.branchDelete(repository, myBranchName, false, notFullyMergedDetector); + GitBranchNotMergedToUpstreamDetector notMergedToUpstreamDetector = new GitBranchNotMergedToUpstreamDetector(); + GitCommandResult result = myGit.branchDelete(repository, myBranchName, false, notFullyMergedDetector, notMergedToUpstreamDetector); if (result.success()) { refresh(repository); markSuccessful(repository); } else if (notFullyMergedDetector.hasHappened()) { + String baseBranch = notMergedToUpstreamDetector.getBaseBranch(); + if (baseBranch == null) { // GitBranchNotMergedToUpstreamDetector didn't happen + baseBranch = myCurrentBranch; + } + Collection remainingRepositories = getRemainingRepositories(); - boolean forceDelete = showNotFullyMergedDialog(myBranchName, remainingRepositories); + boolean forceDelete = showNotFullyMergedDialog(myBranchName, baseBranch, remainingRepositories); if (forceDelete) { GitCompoundResult compoundResult = forceDelete(myBranchName, remainingRepositories); if (compoundResult.totalSuccess()) { @@ -88,7 +94,7 @@ class GitDeleteBranchOperation extends GitBranchOperation { } } else { - fatalError(getErrorTitle(), "This branch is not fully merged to " + myCurrentBranch + "."); + fatalError(getErrorTitle(), "This branch is not fully merged to " + baseBranch + "."); fatalErrorHappened = true; } } @@ -164,14 +170,15 @@ class GitDeleteBranchOperation extends GitBranchOperation { * In multi-repository setup collects unmerged commits for all given repositories. * @return true if the branch should be force deleted. */ - private boolean showNotFullyMergedDialog(@NotNull final String branchName, @NotNull Collection repositories) { - final List mergedToBranches = getMergedToBranches(branchName); + private boolean showNotFullyMergedDialog(@NotNull final String unmergedBranch, @NotNull final String baseBranch, + @NotNull Collection repositories) { + final List mergedToBranches = getMergedToBranches(unmergedBranch); final Map> history = new HashMap>(); for (GitRepository repository : getRepositories()) { // we don't confuse user with the absence of repositories that have succeeded, just show no commits for them (and don't query for log) if (repositories.contains(repository)) { - history.put(repository, getUnmergedCommits(repository, branchName)); + history.put(repository, getUnmergedCommits(repository, unmergedBranch, baseBranch)); } else { history.put(repository, Collections.emptyList()); @@ -182,17 +189,17 @@ class GitDeleteBranchOperation extends GitBranchOperation { UIUtil.invokeAndWaitIfNeeded(new Runnable() { @Override public void run() { - forceDelete.set(GitBranchIsNotFullyMergedDialog.showAndGetAnswer(myProject, history, branchName, mergedToBranches, myCurrentBranch)); + forceDelete.set(GitBranchIsNotFullyMergedDialog.showAndGetAnswer(myProject, history, unmergedBranch, mergedToBranches, baseBranch)); } }); return forceDelete.get(); } @NotNull - private List getUnmergedCommits(@NotNull GitRepository repository, @NotNull String branchName) { + private List getUnmergedCommits(@NotNull GitRepository repository, @NotNull String branchName, @NotNull String baseBranch) { List history; try { - history = GitHistoryUtils.history(myProject, repository.getRoot(), ".." + branchName); + history = GitHistoryUtils.history(myProject, repository.getRoot(), baseBranch + ".." + branchName); } catch (VcsException e) { // this is critical, because we need to show the list of unmerged commits, and it shouldn't happen => inform user and developer throw new GitExecutionException("Couldn't get [git log .." + branchName + "] on repository [" + repository.getRoot() + "]", e); @@ -261,4 +268,34 @@ class GitDeleteBranchOperation extends GitBranchOperation { return Collections.emptyList(); } + // warning: not deleting branch 'feature' that is not yet merged to + // 'refs/remotes/origin/feature', even though it is merged to HEAD. + // error: The branch 'feature' is not fully merged. + // If you are sure you want to delete it, run 'git branch -D feature'. + private static class GitBranchNotMergedToUpstreamDetector implements GitLineHandlerListener { + + private static final Pattern PATTERN = Pattern.compile(".*'(.*)', even though it is merged to.*"); + @Nullable private String myBaseBranch; + + @Override + public void onLineAvailable(String line, Key outputType) { + Matcher matcher = PATTERN.matcher(line); + if (matcher.matches()) { + myBaseBranch = matcher.group(1); + } + } + + @Override + public void processTerminated(int exitCode) { + } + + @Override + public void startFailed(Throwable exception) { + } + + @Nullable + public String getBaseBranch() { + return myBaseBranch; + } + } } diff --git a/plugins/git4idea/src/git4idea/i18n/GitBundle.properties b/plugins/git4idea/src/git4idea/i18n/GitBundle.properties index 241557ad2493..50dff3b2c277 100644 --- a/plugins/git4idea/src/git4idea/i18n/GitBundle.properties +++ b/plugins/git4idea/src/git4idea/i18n/GitBundle.properties @@ -510,9 +510,9 @@ git.unstash.clear.confirmation.title=Remove all stashes? git.unstash.drop.confirmation.message=Do you want to remove {0}?
"{1}" git.unstash.drop.confirmation.title=Remove stash {0}? -branch.delete.not_fully_merged.description=The branch {0} is not fully merged to the current branch {1}.
Below is the list of unmerged commits. +branch.delete.not_fully_merged.description=The branch {0} is not fully merged to the branch {1}.
Below is the list of unmerged commits. branch.delete.not_fully_merged.description.not_on_branch=You are currently not on the branch ({1}).
\ - The branch {0} is not fully merged.
Below is the list of unmerged commits. + The branch {0} is not fully merged to {2}.
Below is the list of unmerged commits. branch.delete.merged_to.many=The branch {0} is however fully merged to the following branches: {1}. branch.delete.merged_to.one=The branch {0} is however fully merged to the branch {1}. branch.delete.warning=You may still delete the branch {0}, but beware that it cannot be undone. diff --git a/plugins/git4idea/tests/git4idea/branch/GitBranchOperationsTest.java b/plugins/git4idea/tests/git4idea/branch/GitBranchOperationsTest.java index a26cbfd47c8b..c02a68bd1c02 100644 --- a/plugins/git4idea/tests/git4idea/branch/GitBranchOperationsTest.java +++ b/plugins/git4idea/tests/git4idea/branch/GitBranchOperationsTest.java @@ -38,6 +38,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import static git4idea.test.GitExec.*; import static git4idea.util.GitUIUtil.getShortRepositoryName; +import static java.util.Collections.singletonList; import static org.testng.Assert.*; /** @@ -431,6 +432,43 @@ public class GitBranchOperationsTest extends AbstractVcsTestCase { assertTrue(dialogShown.get()); } + @Test + public void delete_branch_merged_to_head_but_unmerged_to_upstream_should_show_dialog() throws Exception { + // inspired by IDEA-83604 + // dealing with a single myCommunity repository here + + // prepare parent repository + final File parentDir = new File(myTempDirFixture.getTempDirPath(), "parent.git"); + GitExec.clone(myProject, myCommunity.getRoot().getPath(), parentDir.getPath(), true); + + // initialize feature branch and push to make origin/feature, set up tracking + checkout(myCommunity, "-b", "feature"); + remoteAdd(myCommunity, "origin", parentDir.getPath()); + push(myCommunity, "-u", "origin", "feature"); + + // create a commit and merge it to master, but not to feature's upstream + createAddCommit(myCommunity, "file"); + checkout(myCommunity, "master"); + merge(myCommunity, "feature"); + refresh(myCommunity); + + // delete feature fully merged to current HEAD, but not to the upstream + final AtomicBoolean dialogShown = new AtomicBoolean(); + myDialogManager.registerDialogHandler(GitBranchIsNotFullyMergedDialog.class, new TestDialogHandler() { + @Override + public int handleDialog(GitBranchIsNotFullyMergedDialog dialog) { + dialogShown.set(true); + return DialogWrapper.CANCEL_EXIT_CODE; + } + }); + GitBranchOperationsProcessor processor = new GitBranchOperationsProcessor(myProject, singletonList(myCommunity), myCommunity); + Method method = GitBranchOperationsProcessor.class.getDeclaredMethod("doDelete", String.class, ProgressIndicator.class); + method.setAccessible(true); + method.invoke(processor, "feature", new EmptyProgressIndicator()); + + assertTrue(dialogShown.get()); + } + @Test public void ok_in_unmerged_branch_dialog_should_force_delete_branch() throws Exception { prepareBranchWithCommit("unmerged_branch", myUltimate, myCommunity, myContrib); diff --git a/plugins/git4idea/tests/git4idea/test/GitExec.java b/plugins/git4idea/tests/git4idea/test/GitExec.java index 31e2a0cc11fa..de61dc575ee3 100644 --- a/plugins/git4idea/tests/git4idea/test/GitExec.java +++ b/plugins/git4idea/tests/git4idea/test/GitExec.java @@ -16,9 +16,11 @@ package git4idea.test; import com.intellij.openapi.project.Project; +import com.intellij.openapi.vfs.LocalFileSystem; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.testFramework.AbstractVcsTestCase; import com.intellij.testFramework.VfsTestUtil; +import com.intellij.util.ArrayUtil; import com.intellij.util.ui.UIUtil; import git4idea.repo.GitRepository; import org.jetbrains.annotations.NotNull; @@ -42,6 +44,30 @@ public class GitExec { return GitRepository.getLightInstance(root, project, project); } + /** + * Returns null in case of bare repository, because GitRepository instance for a bare repository can't be created. + */ + @Nullable + public static GitRepository clone(@NotNull Project project, @NotNull String sourcePath, @NotNull String destinationPath, boolean bare) + throws IOException + { + + String[] args = bare ? new String[]{"--bare", sourcePath, destinationPath} : new String[]{sourcePath, destinationPath}; + new GitTestRunEnv(new File(sourcePath)).run("clone", args); + VirtualFile root = LocalFileSystem.getInstance().refreshAndFindFileByIoFile(new File(destinationPath)); + assert root != null; + root.refresh(false, true); + return bare ? null : GitRepository.getLightInstance(root, project, project); + } + + public static String push(@NotNull GitRepository repository, String... args) throws IOException { + return run(repository, "push", args); + } + + public static String remoteAdd(@NotNull GitRepository repository, String... args) throws IOException { + return run(repository, "remote", ArrayUtil.mergeArrays(new String[]{"add"}, args)); + } + public static void create(@NotNull GitRepository repository, @NotNull String filePath) { create(repository, filePath, "content"); }