diff --git a/plugins/git4idea/src/git4idea/push/GitPushOperation.java b/plugins/git4idea/src/git4idea/push/GitPushOperation.java index e24e642decc3..9655119a16ae 100644 --- a/plugins/git4idea/src/git4idea/push/GitPushOperation.java +++ b/plugins/git4idea/src/git4idea/push/GitPushOperation.java @@ -28,7 +28,6 @@ import com.intellij.openapi.progress.ProgressManager; import com.intellij.openapi.progress.util.BackgroundTaskUtil; import com.intellij.openapi.project.Project; import com.intellij.openapi.ui.DialogWrapper; -import com.intellij.openapi.util.Pair; import com.intellij.openapi.util.Ref; import com.intellij.openapi.vcs.VcsException; import com.intellij.openapi.vcs.update.UpdatedFiles; @@ -56,7 +55,6 @@ import git4idea.update.GitRebaseOverMergeProblem; import git4idea.update.GitUpdateProcess; import git4idea.update.GitUpdateResult; import git4idea.update.GitUpdater; -import git4idea.util.GitPreservingProcess; import one.util.streamex.StreamEx; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -139,7 +137,6 @@ public class GitPushOperation { final Map results = ContainerUtil.newHashMap(); Map updatedRoots = ContainerUtil.newHashMap(); - GitPreservingProcess preservingProcess = null; try { Collection remainingRoots = myPushSpecs.keySet(); @@ -185,13 +182,7 @@ public class GitPushOperation { beforePushLabel = LocalHistory.getInstance().putSystemLabel(myProject, "Before push"); } Collection rootsToUpdate = getRootsToUpdate(updateSettings, result.rejected.keySet()); - Pair pair = update(rootsToUpdate, updateSettings.getUpdateMethod(), - rebaseOverMergeProblemDetected == null, - preservingProcess == null); - if (preservingProcess == null) { // multiple updates => multiple preserving processes, but all are fake ones except the first - preservingProcess = pair.second; - } - GitUpdateResult updateResult = pair.first; + GitUpdateResult updateResult = update(rootsToUpdate, updateSettings.getUpdateMethod(), rebaseOverMergeProblemDetected == null); for (GitRepository repository : rootsToUpdate) { updatedRoots.put(repository, updateResult); // TODO update result in GitUpdateProcess is a single for several roots } @@ -204,9 +195,6 @@ public class GitPushOperation { if (myPushProcessCustomization != null) myPushProcessCustomization.executeAfterPush(results); } finally { - if (preservingProcess != null) { - preservingProcess.load(); - } if (beforePushLabel != null) { afterPushLabel = LocalHistory.getInstance().putSystemLabel(myProject, "After push"); } @@ -465,30 +453,17 @@ public class GitPushOperation { } @NotNull - protected Pair update(@NotNull Collection rootsToUpdate, - @NotNull UpdateMethod updateMethod, - boolean checkForRebaseOverMergeProblem, - boolean stashBeforeUpdate) { - GitUpdateProcess updateProcess = new GitUpdateProcess(myProject, myProgressIndicator, - new HashSet<>(rootsToUpdate), UpdatedFiles.create(), - checkForRebaseOverMergeProblem, false) { - @Override - protected boolean stashBeforeUpdate() { - return stashBeforeUpdate; - } - - @Override - protected boolean unstashAfterUpdate() { - return false; - } - }; - GitUpdateResult updateResult = updateProcess.update(updateMethod); - GitPreservingProcess preservingProcess = stashBeforeUpdate ? updateProcess.getPreservingProcess() : null; + protected GitUpdateResult update(@NotNull Collection rootsToUpdate, + @NotNull UpdateMethod updateMethod, + boolean checkForRebaseOverMergeProblem) { + GitUpdateResult updateResult = new GitUpdateProcess(myProject, myProgressIndicator, + new HashSet<>(rootsToUpdate), UpdatedFiles.create(), + checkForRebaseOverMergeProblem, false).update(updateMethod); for (GitRepository repository : rootsToUpdate) { repository.getRoot().refresh(true, true); repository.update(); } - return Pair.create(updateResult, preservingProcess); + return updateResult; } private static class ResultWithOutput { diff --git a/plugins/git4idea/src/git4idea/update/GitUpdateProcess.java b/plugins/git4idea/src/git4idea/update/GitUpdateProcess.java index 4b2df75d76fb..358c167588ee 100644 --- a/plugins/git4idea/src/git4idea/update/GitUpdateProcess.java +++ b/plugins/git4idea/src/git4idea/update/GitUpdateProcess.java @@ -62,6 +62,8 @@ import static git4idea.util.GitUIUtil.*; /** * Handles update process (pull via merge or rebase) for several roots. + * + * @author Kirill Likhodedov */ public class GitUpdateProcess { private static final Logger LOG = Logger.getInstance(GitUpdateProcess.class); @@ -78,8 +80,6 @@ public class GitUpdateProcess { @NotNull private final ProgressIndicator myProgressIndicator; @NotNull private final GitMerger myMerger; - @Nullable private GitPreservingProcess myPreservingProcess; - public GitUpdateProcess(@NotNull Project project, @Nullable ProgressIndicator progressIndicator, @NotNull Collection repositories, @@ -143,14 +143,6 @@ public class GitUpdateProcess { return result; } - protected boolean unstashAfterUpdate() { - return true; - } - - protected boolean stashBeforeUpdate() { - return true; - } - @NotNull private GitUpdateResult updateImpl(@NotNull UpdateMethod updateMethod) { Map trackedBranches = checkTrackedBranchesConfiguration(); @@ -203,14 +195,12 @@ public class GitUpdateProcess { // save local changes if needed (update via merge may perform without saving). final Collection myRootsToSave = ContainerUtil.newArrayList(); LOG.info("updateImpl: identifying if save is needed..."); - if (stashBeforeUpdate()) { - for (Map.Entry entry : updaters.entrySet()) { - GitRepository repo = entry.getKey(); - GitUpdater updater = entry.getValue(); - if (updater.isSaveNeeded()) { - myRootsToSave.add(repo.getRoot()); - LOG.info("update| root " + repo + " needs save"); - } + for (Map.Entry entry : updaters.entrySet()) { + GitRepository repo = entry.getKey(); + GitUpdater updater = entry.getValue(); + if (updater.isSaveNeeded()) { + myRootsToSave.add(repo.getRoot()); + LOG.info("update| root " + repo + " needs save"); } } @@ -218,33 +208,30 @@ public class GitUpdateProcess { final Ref incomplete = Ref.create(false); final Ref compoundResult = Ref.create(); final Map finalUpdaters = updaters; - myPreservingProcess = new GitPreservingProcess(myProject, myGit, myRootsToSave, "Update", "Remote", - GitVcsSettings.getInstance(myProject).updateChangesPolicy(), myProgressIndicator, () -> { - LOG.info("updateImpl: updating..."); - GitRepository currentlyUpdatedRoot = null; - try { - for (GitRepository repo : myRepositories) { - GitUpdater updater = finalUpdaters.get(repo); - if (updater == null) continue; - currentlyUpdatedRoot = repo; - GitUpdateResult res = updater.update(); - LOG.info("updating root " + currentlyUpdatedRoot + " finished: " + res); - if (res == GitUpdateResult.INCOMPLETE) { - incomplete.set(true); - } - compoundResult.set(joinResults(compoundResult.get(), res)); - } - } - catch (VcsException e) { - String rootName = (currentlyUpdatedRoot == null) ? "" : getShortRepositoryName(currentlyUpdatedRoot); - LOG.info("Error updating changes for root " + currentlyUpdatedRoot, e); - notifyImportantError(myProject, "Error updating " + rootName, - "Updating " + rootName + " failed with an error: " + e.getLocalizedMessage()); - } - }); - myPreservingProcess.execute(() -> { - if (!unstashAfterUpdate()) return false; - + new GitPreservingProcess(myProject, myGit, myRootsToSave, "Update", "Remote", + GitVcsSettings.getInstance(myProject).updateChangesPolicy(), myProgressIndicator, () -> { + LOG.info("updateImpl: updating..."); + GitRepository currentlyUpdatedRoot = null; + try { + for (GitRepository repo : myRepositories) { + GitUpdater updater = finalUpdaters.get(repo); + if (updater == null) continue; + currentlyUpdatedRoot = repo; + GitUpdateResult res = updater.update(); + LOG.info("updating root " + currentlyUpdatedRoot + " finished: " + res); + if (res == GitUpdateResult.INCOMPLETE) { + incomplete.set(true); + } + compoundResult.set(joinResults(compoundResult.get(), res)); + } + } + catch (VcsException e) { + String rootName = (currentlyUpdatedRoot == null) ? "" : getShortRepositoryName(currentlyUpdatedRoot); + LOG.info("Error updating changes for root " + currentlyUpdatedRoot, e); + notifyImportantError(myProject, "Error updating " + rootName, + "Updating " + rootName + " failed with an error: " + e.getLocalizedMessage()); + } + }).execute(() -> { // Note: compoundResult normally should not be null, because the updaters map was checked for non-emptiness. // But if updater.update() fails with exception for the first root, then the value would not be assigned. // In this case we don't restore local changes either, because update failed. @@ -308,11 +295,6 @@ public class GitUpdateProcess { return updaters; } - @Nullable - public GitPreservingProcess getPreservingProcess() { - return myPreservingProcess; - } - @NotNull private static GitUpdateResult joinResults(@Nullable GitUpdateResult compoundResult, GitUpdateResult result) { if (compoundResult == null) { diff --git a/plugins/git4idea/tests/git4idea/push/GitPushOperationSingleRepoTest.kt b/plugins/git4idea/tests/git4idea/push/GitPushOperationSingleRepoTest.kt index cdd310e40292..89b9f8cf3bc7 100644 --- a/plugins/git4idea/tests/git4idea/push/GitPushOperationSingleRepoTest.kt +++ b/plugins/git4idea/tests/git4idea/push/GitPushOperationSingleRepoTest.kt @@ -23,6 +23,7 @@ import com.intellij.openapi.util.text.StringUtil import com.intellij.openapi.vcs.Executor import com.intellij.openapi.vcs.update.FileGroup import com.intellij.openapi.vcs.update.UpdatedFiles +import com.intellij.testFramework.UsefulTestCase import com.intellij.util.containers.ContainerUtil import git4idea.branch.GitBranchUtil import git4idea.config.GitVersionSpecialty @@ -32,7 +33,6 @@ import git4idea.repo.GitRepository import git4idea.test.* import git4idea.update.GitRebaseOverMergeProblem import git4idea.update.GitUpdateResult -import git4idea.util.GitPreservingProcess import org.junit.Assume.assumeTrue import java.io.File import java.util.Collections.singletonMap @@ -141,11 +141,10 @@ class GitPushOperationSingleRepoTest : GitPushOperationBaseTest() { val pushSpec = makePushSpec(repository, "master", "origin/master") val result = object : GitPushOperation(project, pushSupport, singletonMap(repository, pushSpec), null, false, false) { - override fun update(rootsToUpdate: MutableCollection, + override fun update(rootsToUpdate: Collection, updateMethod: UpdateMethod, - checkForRebaseOverMergeProblem: Boolean, - stashBeforeUpdate: Boolean): Pair { - val updateResult = super.update(rootsToUpdate, updateMethod, checkForRebaseOverMergeProblem, stashBeforeUpdate) + checkForRebaseOverMergeProblem: Boolean): GitUpdateResult { + val updateResult = super.update(rootsToUpdate, updateMethod, checkForRebaseOverMergeProblem) pushCommitFromBro() return updateResult } @@ -170,11 +169,10 @@ class GitPushOperationSingleRepoTest : GitPushOperationBaseTest() { val result = object : GitPushOperation(project, pushSupport, singletonMap(repository, pushSpec), null, false, false) { internal var updateHappened: Boolean = false - override fun update(rootsToUpdate: MutableCollection, + override fun update(rootsToUpdate: Collection, updateMethod: UpdateMethod, - checkForRebaseOverMergeProblem: Boolean, - stashBeforeUpdate: Boolean): Pair { - val updateResult = super.update(rootsToUpdate, updateMethod, checkForRebaseOverMergeProblem, stashBeforeUpdate) + checkForRebaseOverMergeProblem: Boolean): GitUpdateResult { + val updateResult = super.update(rootsToUpdate, updateMethod, checkForRebaseOverMergeProblem) if (!updateHappened) { updateHappened = true pushCommitFromBro() @@ -428,49 +426,6 @@ class GitPushOperationSingleRepoTest : GitPushOperationBaseTest() { assertEquals(UpdateMethod.BRANCH_DEFAULT, settings.updateType) } - // IDEA-76774 - fun `test local changes are stashed-unstashed only once for several push rejections`() { - pushCommitFromBro() - cd(repository) - makeCommit("afile.txt") - val localFile = repository.file("local-changes.txt") - localFile.create("Local changes").add() - - updateRepositories() - refresh() - updateChangeListManager() - - agreeToUpdate(GitRejectedPushUpdateDialog.REBASE_EXIT_CODE) - - val pushSpec = makePushSpec(repository, "master", "origin/master") - - var stashCalled = 0 - git.stashListener = { - stashCalled++ - } - - var pushRejected = 0 - - val result = object : GitPushOperation(project, pushSupport, singletonMap(repository, pushSpec), null, false, false) { - override fun update(rootsToUpdate: MutableCollection, - updateMethod: UpdateMethod, - checkForRebaseOverMergeProblem: Boolean, - stashBeforeUpdate: Boolean): Pair { - val updateResult = super.update(rootsToUpdate, updateMethod, checkForRebaseOverMergeProblem, stashBeforeUpdate) - if (pushRejected < 2) { - pushCommitFromBro() - pushRejected++ - } - return updateResult - } - }.execute() - - assertResult(SUCCESS, 1, "master", "origin/master", result) - assertTrue("Locally changed file wasn't unstashed", localFile.exists()) - repository.assertStatus(localFile.file, 'A') - assertEquals("Stash should have been called only once", 1, stashCalled) - } - private fun generateUpdateNeeded() { pushCommitFromBro() cd(repository) @@ -509,8 +464,8 @@ class GitPushOperationSingleRepoTest : GitPushOperationBaseTest() { updatedFiles: List?, actualResult: GitPushResult) { assertResult(type, pushedCommits, from, to, updateResult, actualResult.results[repository]!!) - if (updatedFiles != null) - assertSameElements("Updated files set is incorrect", getUpdatedFiles(actualResult.updatedFiles), updatedFiles) + UsefulTestCase.assertSameElements("Updated files set is incorrect", + getUpdatedFiles(actualResult.updatedFiles), ContainerUtil.notNullize(updatedFiles)) } private fun getUpdatedFiles(updatedFiles: UpdatedFiles): Collection {