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

2026-09-27 10:03:11 +07:00 · 2018-05-28 19:22:40 +03:00
parent 0a51d9ee96
commit 9d9c015d95
3 changed files with 137 additions and 49 deletions
@@ -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,
-                                   @NotNull UpdateMethod updateMethod,
-                                   boolean checkForRebaseOverMergeProblem) {
-    GitUpdateResult updateResult = new GitUpdateProcess(myProject, myProgressIndicator,
-                                                        new HashSet<>(rootsToUpdate), UpdatedFiles.create(),
-                                                        checkForRebaseOverMergeProblem, false).update(updateMethod);
+  protected Pair<GitUpdateResult, GitPreservingProcess> update(@NotNull Collection<GitRepository> 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;
    for (GitRepository repository : rootsToUpdate) {
      repository.getRoot().refresh(true, true);
      repository.update();
    }
-    return updateResult;
+    return Pair.create(updateResult, preservingProcess);
  }

  private static class ResultWithOutput {
@@ -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()) {
-      GitRepository repo = entry.getKey();
-      GitUpdater updater = entry.getValue();
-      if (updater.isSaveNeeded()) {
-        myRootsToSave.add(repo.getRoot());
-        LOG.info("update| root " + repo + " needs save");
+    if (stashBeforeUpdate()) {
+      for (Map.Entry<GitRepository, GitUpdater> 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");
+        }
      }
    }

@@ -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",
-                             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(() -> {
+    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;
+
      // 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) {
@@ -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 {
-        val updateResult = super.update(rootsToUpdate, updateMethod, checkForRebaseOverMergeProblem)
+                          checkForRebaseOverMergeProblem: Boolean,
+                          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 {
-        val updateResult = super.update(rootsToUpdate, updateMethod, checkForRebaseOverMergeProblem)
+                          checkForRebaseOverMergeProblem: Boolean,
+                          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",
-        getUpdatedFiles(actualResult.updatedFiles), ContainerUtil.notNullize(updatedFiles))
+    if (updatedFiles != null)
+      assertSameElements("Updated files set is incorrect", getUpdatedFiles(actualResult.updatedFiles), updatedFiles)
  }

  private fun getUpdatedFiles(updatedFiles: UpdatedFiles): Collection<String> {