From ec652e0cc8eeea18e28b1578672c358106f99708 Mon Sep 17 00:00:00 2001 From: Kirill Likhodedov Date: Sat, 25 Mar 2017 11:29:02 +0000 Subject: [PATCH] During update project check if the tracked branch is deleted after fetch And display a notification in that case, instead of failing later. Also don't store the tracked branch information in a field: it is enough to pass it in a local variable. Relates to IDEA-169877. --- .../src/git4idea/update/GitUpdateProcess.java | 75 +++++++++++-------- .../git4idea/update/GitMultiRepoUpdateTest.kt | 32 +++++++- 2 files changed, 73 insertions(+), 34 deletions(-) diff --git a/plugins/git4idea/src/git4idea/update/GitUpdateProcess.java b/plugins/git4idea/src/git4idea/update/GitUpdateProcess.java index 1db26a7d7090..fe90eaecd949 100644 --- a/plugins/git4idea/src/git4idea/update/GitUpdateProcess.java +++ b/plugins/git4idea/src/git4idea/update/GitUpdateProcess.java @@ -15,6 +15,7 @@ */ package git4idea.update; +import com.google.common.annotations.VisibleForTesting; import com.intellij.dvcs.DvcsUtil; import com.intellij.openapi.application.AccessToken; import com.intellij.openapi.diagnostic.Logger; @@ -57,6 +58,7 @@ import java.util.Map; import static com.intellij.dvcs.DvcsUtil.getShortRepositoryName; import static git4idea.GitUtil.getRootsFromRepositories; +import static git4idea.GitUtil.mention; import static git4idea.util.GitUIUtil.*; /** @@ -78,8 +80,6 @@ public class GitUpdateProcess { @NotNull private final ProgressIndicator myProgressIndicator; @NotNull private final GitMerger myMerger; - private final Map myTrackedBranches = new HashMap<>(); - public GitUpdateProcess(@NotNull Project project, @Nullable ProgressIndicator progressIndicator, @NotNull Collection repositories, @@ -122,7 +122,10 @@ public class GitUpdateProcess { } // check if update is possible - if (checkRebaseInProgress() || isMergeInProgress() || areUnmergedFiles() || !checkTrackedBranchesConfigured()) { + if (checkRebaseInProgress() || isMergeInProgress() || areUnmergedFiles()) { + return GitUpdateResult.NOT_READY; + } + if (checkTrackedBranchesConfiguration() == null) { return GitUpdateResult.NOT_READY; } @@ -144,9 +147,14 @@ public class GitUpdateProcess { @NotNull private GitUpdateResult updateImpl(@NotNull UpdateMethod updateMethod) { + Map trackedBranches = checkTrackedBranchesConfiguration(); + if (trackedBranches == null) { + return GitUpdateResult.NOT_READY; + } + Map updaters; try { - updaters = defineUpdaters(updateMethod); + updaters = defineUpdaters(updateMethod, trackedBranches); } catch (VcsException e) { LOG.info(e); @@ -171,8 +179,7 @@ public class GitUpdateProcess { GitRebaseOverMergeProblem.Decision decision = GitRebaseOverMergeProblem.showDialog(); if (decision == GitRebaseOverMergeProblem.Decision.MERGE_INSTEAD) { for (GitRepository repo : problematicRoots) { - updaters.put(repo, new GitMergeUpdater(myProject, myGit, repo.getRoot(), myTrackedBranches, - myProgressIndicator, myUpdatedFiles)); + updaters.put(repo, new GitMergeUpdater(myProject, myGit, repo.getRoot(), trackedBranches, myProgressIndicator, myUpdatedFiles)); } } else if (decision == GitRebaseOverMergeProblem.Decision.CANCEL_OPERATION) { @@ -272,12 +279,13 @@ public class GitUpdateProcess { } @NotNull - private Map defineUpdaters(@NotNull UpdateMethod updateMethod) throws VcsException { + private Map defineUpdaters(@NotNull UpdateMethod updateMethod, + @NotNull Map trackedBranches) throws VcsException { final Map updaters = new HashMap<>(); LOG.info("updateImpl: defining updaters..."); for (GitRepository repository : myRepositories) { VirtualFile root = repository.getRoot(); - GitUpdater updater = GitUpdater.getUpdater(myProject, myGit, myTrackedBranches, root, myProgressIndicator, myUpdatedFiles, + GitUpdater updater = GitUpdater.getUpdater(myProject, myGit, trackedBranches, root, myProgressIndicator, myUpdatedFiles, updateMethod); if (updater.isUpdateNeeded()) { updaters.put(repository, updater); @@ -301,12 +309,13 @@ public class GitUpdateProcess { } /** - * For each root check that the repository is on branch, and this branch is tracking a remote branch, - * and the remote branch exists. - * If it is not true for at least one of roots, notify and return false. - * If branch configuration is OK for all roots, return true. + * For each root check that the repository is on branch, and this branch is tracking a remote branch, and the remote branch exists. + * If it is not true for at least one of roots, notify and return null. + * If branch configuration is OK for all roots, return the collected tracking branch information. */ - private boolean checkTrackedBranchesConfigured() { + @Nullable + private Map checkTrackedBranchesConfiguration() { + Map trackedBranches = ContainerUtil.newHashMap(); LOG.info("checking tracked branch configuration..."); for (GitRepository repository : myRepositories) { VirtualFile root = repository.getRoot(); @@ -315,34 +324,38 @@ public class GitUpdateProcess { LOG.info("checkTrackedBranchesConfigured: current branch is null in " + repository); notifyImportantError(myProject, "Can't update: no current branch", "You are in 'detached HEAD' state, which means that you're not on any branch" + - rootStringIfNeeded(root) + + mention(repository) + "
" + "Checkout a branch to make update possible."); - return false; + return null; } 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)); - String recommendedCommand = String.format(GitVersionSpecialty.KNOWS_SET_UPSTREAM_TO.existsIn(repository.getVcs().getVersion()) ? - "git branch --set-upstream-to origin/%1$s %1$s" : - "git branch --set-upstream %1$s origin/%1$s", branchName); - notifyImportantError(myProject, "Can't update: no tracked branch", - "No tracked branch configured for branch " + code(branchName) + - rootStringIfNeeded(root) + - "To make your branch track a remote branch call, for example,
" + - "" + recommendedCommand + ""); - return false; + notifyImportantError(repository.getProject(), "Can't Update", getNoTrackedBranchError(repository, branchName)); + return null; } - myTrackedBranches.put(root, new GitBranchPair(branch, trackInfo.getRemoteBranch())); + trackedBranches.put(root, new GitBranchPair(branch, trackInfo.getRemoteBranch())); } - return true; + return trackedBranches; } - private String rootStringIfNeeded(@NotNull VirtualFile root) { - if (myRepositories.size() < 2) { - return ".
"; - } - return "
in Git repository " + code(root.getPresentableUrl()) + "
"; + @VisibleForTesting + @NotNull + static String getNoTrackedBranchError(@NotNull GitRepository repository, @NotNull String branchName) { + String recommendedCommand = recommendSetupTrackingCommand(repository, branchName); + return "No tracked branch configured for branch " + code(branchName) + + mention(repository) + + " or the branch doesn't exist.
" + + "To make your branch track a remote branch call, for example,
" + + "" + recommendedCommand + ""; + } + + @NotNull + private static String recommendSetupTrackingCommand(@NotNull GitRepository repository, @NotNull String branchName) { + return String.format(GitVersionSpecialty.KNOWS_SET_UPSTREAM_TO.existsIn(repository.getVcs().getVersion()) ? + "git branch --set-upstream-to origin/%1$s %1$s" : + "git branch --set-upstream %1$s origin/%1$s", branchName); } /** diff --git a/plugins/git4idea/tests/git4idea/update/GitMultiRepoUpdateTest.kt b/plugins/git4idea/tests/git4idea/update/GitMultiRepoUpdateTest.kt index ed8bac088f1b..fd276ecbb363 100644 --- a/plugins/git4idea/tests/git4idea/update/GitMultiRepoUpdateTest.kt +++ b/plugins/git4idea/tests/git4idea/update/GitMultiRepoUpdateTest.kt @@ -20,9 +20,7 @@ import com.intellij.openapi.vcs.Executor.cd import com.intellij.openapi.vcs.update.UpdatedFiles import git4idea.config.UpdateMethod import git4idea.repo.GitRepository -import git4idea.test.git -import git4idea.test.last -import git4idea.test.tacp +import git4idea.test.* import java.io.File class GitMultiRepoUpdateTest : GitUpdateBaseTest() { @@ -66,6 +64,34 @@ class GitMultiRepoUpdateTest : GitUpdateBaseTest() { assertEquals("Couldn't find the hash from bro", hash, git("log -1 --no-merges --pretty=%H")) } + fun `test update fails if branch is deleted in one of repositories`() { + listOf(bro, bromunity).forEach { + cd(it) + git("checkout -b feature") + git("push -u origin feature") + } + listOf(repository, community).forEach { + cd(it) + git("pull") + git("checkout -b feature origin/feature") + it.update() + } + + // commit in one repo to let update work + cd(bro) + tac("bro.txt") + git("push") + // remove branch in another repo + cd(bromunity) + git("push origin :feature") + + val updateProcess = GitUpdateProcess(myProject, EmptyProgressIndicator(), repositories(), UpdatedFiles.create(), false) + val result = updateProcess.update(UpdateMethod.MERGE) + + assertEquals("Update result is incorrect", GitUpdateResult.NOT_READY, result) + assertErrorNotification("Can't Update", GitUpdateProcess.getNoTrackedBranchError(community, "feature")) + } + private fun updateWithMerge(): GitUpdateResult { return GitUpdateProcess(myProject, EmptyProgressIndicator(), repositories(), UpdatedFiles.create(), false).update(UpdateMethod.MERGE) }