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

This reverts commit 9d9c015 This change breaks the CLM freezing, which leads to IDEA-192906.
2026-09-27 10:03:11 +07:00 · 2018-05-29 21:19:16 +03:00
parent 0e0a4e003e
commit 534714dbb7
3 changed files with 49 additions and 137 deletions
@@ -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<GitRepository, GitPushRepoResult> results = ContainerUtil.newHashMap();
    Map<GitRepository, GitUpdateResult> updatedRoots = ContainerUtil.newHashMap();
-    GitPreservingProcess preservingProcess = null;

    try {
      Collection<GitRepository> remainingRoots = myPushSpecs.keySet();
@@ -185,13 +182,7 @@ public class GitPushOperation {
            beforePushLabel = LocalHistory.getInstance().putSystemLabel(myProject, "Before push");
          }
          Collection<GitRepository> rootsToUpdate = getRootsToUpdate(updateSettings, result.rejected.keySet());
-          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;
+          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<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;
+  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);
    for (GitRepository repository : rootsToUpdate) {
      repository.getRoot().refresh(true, true);
      repository.update();
    }
-    return Pair.create(updateResult, preservingProcess);
+    return updateResult;
  }

  private static class ResultWithOutput {
@@ -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<GitRepository> 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<VirtualFile, GitBranchPair> trackedBranches = checkTrackedBranchesConfiguration();
@@ -203,14 +195,12 @@ 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...");
-    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");
-        }
+    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");
      }
    }

@@ -218,33 +208,30 @@ public class GitUpdateProcess {
    final Ref<Boolean> incomplete = Ref.create(false);
    final Ref<GitUpdateResult> compoundResult = Ref.create();
    final Map<GitRepository, GitUpdater> 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) {
@@ -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<GitRepository>,
+      override fun update(rootsToUpdate: Collection<GitRepository>,
                          updateMethod: UpdateMethod,
-                          checkForRebaseOverMergeProblem: Boolean,
-                          stashBeforeUpdate: Boolean): Pair<GitUpdateResult, GitPreservingProcess> {
-        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<GitRepository>,
+      override fun update(rootsToUpdate: Collection<GitRepository>,
                          updateMethod: UpdateMethod,
-                          checkForRebaseOverMergeProblem: Boolean,
-                          stashBeforeUpdate: Boolean): Pair<GitUpdateResult, GitPreservingProcess> {
-        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<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)
@@ -509,8 +464,8 @@ class GitPushOperationSingleRepoTest : GitPushOperationBaseTest() {
                           updatedFiles: List<String>?,
                           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<String> {