git: stash-unstash just once during the push-with-update IDEA-76774

plugins/git4idea/src/git4idea/push/GitPushOperation.java

@@ -28,6 +28,7 @@ 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;
@@ -55,6 +56,7 @@ 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;
@@ -137,6 +139,7 @@ public class GitPushOperation {
 
     final Map<GitRepository, GitPushRepoResult> results = ContainerUtil.newHashMap();
     Map<GitRepository, GitUpdateResult> updatedRoots = ContainerUtil.newHashMap();
+    GitPreservingProcess preservingProcess = null;
 
     try {
       Collection<GitRepository> remainingRoots = myPushSpecs.keySet();
@@ -182,7 +185,13 @@ public class GitPushOperation {
             beforePushLabel = LocalHistory.getInstance().putSystemLabel(myProject, "Before push");
           }
           Collection<GitRepository> rootsToUpdate = getRootsToUpdate(updateSettings, result.rejected.keySet());
-          GitUpdateResult updateResult = update(rootsToUpdate, updateSettings.getUpdateMethod(), rebaseOverMergeProblemDetected == null);
+          Pair<GitUpdateResult, GitPreservingProcess> 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;
           for (GitRepository repository : rootsToUpdate) {
             updatedRoots.put(repository, updateResult); // TODO update result in GitUpdateProcess is a single for several roots
           }
@@ -195,6 +204,9 @@ 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");
       }
@@ -453,17 +465,30 @@ public class GitPushOperation {
   }
 
   @NotNull
-  protected GitUpdateResult update(@NotNull Collection<GitRepository> rootsToUpdate,
+  protected Pair<GitUpdateResult, GitPreservingProcess> update(@NotNull Collection<GitRepository> rootsToUpdate,
-                                   @NotNull UpdateMethod updateMethod,
+                                                               @NotNull UpdateMethod updateMethod,
-                                   boolean checkForRebaseOverMergeProblem) {
+                                                               boolean checkForRebaseOverMergeProblem,
-    GitUpdateResult updateResult = new GitUpdateProcess(myProject, myProgressIndicator,
+                                                               boolean stashBeforeUpdate) {
-                                                        new HashSet<>(rootsToUpdate), UpdatedFiles.create(),
+    GitUpdateProcess updateProcess = new GitUpdateProcess(myProject, myProgressIndicator,
-                                                        checkForRebaseOverMergeProblem, false).update(updateMethod);
+                                                          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;
     for (GitRepository repository : rootsToUpdate) {
       repository.getRoot().refresh(true, true);
       repository.update();
     }
-    return updateResult;
+    return Pair.create(updateResult, preservingProcess);
   }
 
   private static class ResultWithOutput {

plugins/git4idea/src/git4idea/update/GitUpdateProcess.java

@@ -62,8 +62,6 @@ 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);
@@ -80,6 +78,8 @@ 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<GitRepository> repositories,
@@ -143,6 +143,14 @@ public class GitUpdateProcess {
     return result;
   }
 
+  protected boolean unstashAfterUpdate() {
+    return true;
+  }
+
+  protected boolean stashBeforeUpdate() {
+    return true;
+  }
+
   @NotNull
   private GitUpdateResult updateImpl(@NotNull UpdateMethod updateMethod) {
     Map<VirtualFile, GitBranchPair> trackedBranches = checkTrackedBranchesConfiguration();
@@ -195,12 +203,14 @@ public class GitUpdateProcess {
     // save local changes if needed (update via merge may perform without saving).
     final Collection<VirtualFile> myRootsToSave = ContainerUtil.newArrayList();
     LOG.info("updateImpl: identifying if save is needed...");
-    for (Map.Entry<GitRepository, GitUpdater> entry : updaters.entrySet()) {
+    if (stashBeforeUpdate()) {
-      GitRepository repo = entry.getKey();
+      for (Map.Entry<GitRepository, GitUpdater> entry : updaters.entrySet()) {
-      GitUpdater updater = entry.getValue();
+        GitRepository repo = entry.getKey();
-      if (updater.isSaveNeeded()) {
+        GitUpdater updater = entry.getValue();
-        myRootsToSave.add(repo.getRoot());
+        if (updater.isSaveNeeded()) {
-        LOG.info("update| root " + repo + " needs save");
+          myRootsToSave.add(repo.getRoot());
+          LOG.info("update| root " + repo + " needs save");
+        }
       }
     }
 
@@ -208,30 +218,33 @@ public class GitUpdateProcess {
     final Ref<Boolean> incomplete = Ref.create(false);
     final Ref<GitUpdateResult> compoundResult = Ref.create();
     final Map<GitRepository, GitUpdater> finalUpdaters = updaters;
-    new GitPreservingProcess(myProject, myGit, myRootsToSave, "Update", "Remote",
+    myPreservingProcess = new GitPreservingProcess(myProject, myGit, myRootsToSave, "Update", "Remote",
-                             GitVcsSettings.getInstance(myProject).updateChangesPolicy(), myProgressIndicator, () -> {
+                                                   GitVcsSettings.getInstance(myProject).updateChangesPolicy(), myProgressIndicator, () -> {
-                               LOG.info("updateImpl: updating...");
+      LOG.info("updateImpl: updating...");
-                               GitRepository currentlyUpdatedRoot = null;
+      GitRepository currentlyUpdatedRoot = null;
-                               try {
+      try {
-                                 for (GitRepository repo : myRepositories) {
+        for (GitRepository repo : myRepositories) {
-                                   GitUpdater updater = finalUpdaters.get(repo);
+          GitUpdater updater = finalUpdaters.get(repo);
-                                   if (updater == null) continue;
+          if (updater == null) continue;
-                                   currentlyUpdatedRoot = repo;
+          currentlyUpdatedRoot = repo;
-                                   GitUpdateResult res = updater.update();
+          GitUpdateResult res = updater.update();
-                                   LOG.info("updating root " + currentlyUpdatedRoot + " finished: " + res);
+          LOG.info("updating root " + currentlyUpdatedRoot + " finished: " + res);
-                                   if (res == GitUpdateResult.INCOMPLETE) {
+          if (res == GitUpdateResult.INCOMPLETE) {
-                                     incomplete.set(true);
+            incomplete.set(true);
-                                   }
+          }
-                                   compoundResult.set(joinResults(compoundResult.get(), res));
+          compoundResult.set(joinResults(compoundResult.get(), res));
-                                 }
+        }
-                               }
+      }
-                               catch (VcsException e) {
+      catch (VcsException e) {
-                                 String rootName = (currentlyUpdatedRoot == null) ? "" : getShortRepositoryName(currentlyUpdatedRoot);
+        String rootName = (currentlyUpdatedRoot == null) ? "" : getShortRepositoryName(currentlyUpdatedRoot);
-                                 LOG.info("Error updating changes for root " + currentlyUpdatedRoot, e);
+        LOG.info("Error updating changes for root " + currentlyUpdatedRoot, e);
-                                 notifyImportantError(myProject, "Error updating " + rootName,
+        notifyImportantError(myProject, "Error updating " + rootName,
-                                                      "Updating " + rootName + " failed with an error: " + e.getLocalizedMessage());
+                             "Updating " + rootName + " failed with an error: " + e.getLocalizedMessage());
-                               }
+      }
-                             }).execute(() -> {
+    });
+    myPreservingProcess.execute(() -> {
+      if (!unstashAfterUpdate()) return false;
+
       // 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.
@@ -295,6 +308,11 @@ public class GitUpdateProcess {
     return updaters;
   }
 
+  @Nullable
+  public GitPreservingProcess getPreservingProcess() {
+    return myPreservingProcess;
+  }
+
   @NotNull
   private static GitUpdateResult joinResults(@Nullable GitUpdateResult compoundResult, GitUpdateResult result) {
     if (compoundResult == null) {

plugins/git4idea/tests/git4idea/push/GitPushOperationSingleRepoTest.kt

@@ -23,7 +23,6 @@ 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
@@ -33,6 +32,7 @@ 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,10 +141,11 @@ 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: Collection<GitRepository>,
+      override fun update(rootsToUpdate: MutableCollection<GitRepository>,
                           updateMethod: UpdateMethod,
-                          checkForRebaseOverMergeProblem: Boolean): GitUpdateResult {
+                          checkForRebaseOverMergeProblem: Boolean,
-        val updateResult = super.update(rootsToUpdate, updateMethod, checkForRebaseOverMergeProblem)
+                          stashBeforeUpdate: Boolean): Pair<GitUpdateResult, GitPreservingProcess> {
+        val updateResult = super.update(rootsToUpdate, updateMethod, checkForRebaseOverMergeProblem, stashBeforeUpdate)
         pushCommitFromBro()
         return updateResult
       }
@@ -169,10 +170,11 @@ class GitPushOperationSingleRepoTest : GitPushOperationBaseTest() {
     val result = object : GitPushOperation(project, pushSupport, singletonMap(repository, pushSpec), null, false, false) {
       internal var updateHappened: Boolean = false
 
-      override fun update(rootsToUpdate: Collection<GitRepository>,
+      override fun update(rootsToUpdate: MutableCollection<GitRepository>,
                           updateMethod: UpdateMethod,
-                          checkForRebaseOverMergeProblem: Boolean): GitUpdateResult {
+                          checkForRebaseOverMergeProblem: Boolean,
-        val updateResult = super.update(rootsToUpdate, updateMethod, checkForRebaseOverMergeProblem)
+                          stashBeforeUpdate: Boolean): Pair<GitUpdateResult, GitPreservingProcess> {
+        val updateResult = super.update(rootsToUpdate, updateMethod, checkForRebaseOverMergeProblem, stashBeforeUpdate)
         if (!updateHappened) {
           updateHappened = true
           pushCommitFromBro()
@@ -426,6 +428,49 @@ 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<GitRepository>,
+                          updateMethod: UpdateMethod,
+                          checkForRebaseOverMergeProblem: Boolean,
+                          stashBeforeUpdate: Boolean): Pair<GitUpdateResult, GitPreservingProcess> {
+        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)
@@ -464,8 +509,8 @@ class GitPushOperationSingleRepoTest : GitPushOperationBaseTest() {
                            updatedFiles: List<String>?,
                            actualResult: GitPushResult) {
     assertResult(type, pushedCommits, from, to, updateResult, actualResult.results[repository]!!)
-    UsefulTestCase.assertSameElements("Updated files set is incorrect",
+    if (updatedFiles != null)
-        getUpdatedFiles(actualResult.updatedFiles), ContainerUtil.notNullize(updatedFiles))
+      assertSameElements("Updated files set is incorrect", getUpdatedFiles(actualResult.updatedFiles), updatedFiles)
   }
 
   private fun getUpdatedFiles(updatedFiles: UpdatedFiles): Collection<String> {