[git] preserve order of merge commits when necessary

For merge commits that are not different to one of the parents collecting changes requires additional "git log" call in order to get tree hashes. In order to avoid multiple extra git invocations, these calls were bundled together. This led to merge commits appear at the end of the list, which was fine for indexing purposes, but led to problems in other cases, such as branches comparison. This commit introduces a parameter that allows to preserve commits order.

Issue was introduced in eed01c21.

IDEA-186772

(cherry picked from commit 9bb1be68)

plugins/git4idea/src/git4idea/history/GitHistoryUtils.java

@@ -237,7 +237,7 @@ public class GitHistoryUtils {
   @NotNull
   public static List<GitCommit> history(@NotNull Project project, @NotNull VirtualFile root, String... parameters)
     throws VcsException {
-    final VcsLogObjectsFactory factory = GitLogUtil.getObjectsFactoryWithDisposeCheck(project);
+    VcsLogObjectsFactory factory = GitLogUtil.getObjectsFactoryWithDisposeCheck(project);
     if (factory == null) {
       return Collections.emptyList();
     }
@@ -258,7 +258,7 @@ public class GitHistoryUtils {
                                  @NotNull VirtualFile root,
                                  @NotNull Consumer<? super GitCommit> commitConsumer,
                                  @NotNull String... parameters) throws VcsException {
-    GitLogUtil.readFullDetails(project, root, commitConsumer, true, parameters);
+    GitLogUtil.readFullDetails(project, root, commitConsumer, true, true, parameters);
   }
 
   public static long getAuthorTime(@NotNull Project project, @NotNull FilePath path, @NotNull String commitsId) throws VcsException {

plugins/git4idea/src/git4idea/history/GitLogRecordCollector.java

-/*
- * Copyright 2000-2017 JetBrains s.r.o.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
+// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
 package git4idea.history;
 
 import com.intellij.openapi.diagnostic.Logger;
@@ -27,8 +13,8 @@ import git4idea.GitVcs;
 import git4idea.commands.Git;
 import git4idea.commands.GitCommand;
 import git4idea.commands.GitLineHandler;
-import git4idea.util.GitUIUtil;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 import java.util.*;
 
@@ -40,35 +26,56 @@ import static git4idea.history.GitLogParser.GitLogOption.TREE;
  * This class collects records for one commit and sends them together for processing.
  * It also deals with problems with `-m` flag, when `git log` does not provide empty records when a commit is not different from one of the parents.
  */
-abstract class GitLogRecordCollector implements Consumer<GitLogRecord> {
+class GitLogRecordCollector implements Consumer<GitLogRecord> {
   private static final Logger LOG = Logger.getInstance(GitLogRecordCollector.class);
-  private static final int STATUS_LINES_THRESHOLD = 20_000;
-  @NotNull private final Project myProject;
-  @NotNull private final VirtualFile myRoot;
-  @NotNull private final MultiMap<String, GitLogRecord> myHashToRecord = MultiMap.create();
-  @NotNull private final MultiMap<String, GitLogRecord> myHashToIncompleteRecords = MultiMap.create();
-  private int myIncompleteStatusLinesCount = 0;
-
-  protected GitLogRecordCollector(@NotNull Project project, @NotNull VirtualFile root) {
+
+  @NotNull protected final Project myProject;
+  @NotNull protected final VirtualFile myRoot;
+  @NotNull protected final Consumer<List<GitLogRecord>> myConsumer;
+
+  @NotNull private final MultiMap<String, GitLogRecord> myHashToRecord = MultiMap.createLinked();
+  @Nullable private String myLastHash = null;
+
+  protected GitLogRecordCollector(@NotNull Project project,
+                                  @NotNull VirtualFile root,
+                                  @NotNull Consumer<List<GitLogRecord>> consumer) {
     myProject = project;
     myRoot = root;
+    myConsumer = consumer;
   }
 
   @Override
   public void consume(@NotNull GitLogRecord record) {
-    String[] parents = record.getParentsHashes();
-    if (parents.length <= 1) {
-      consume(Collections.singletonList(record));
+    if (!record.getHash().equals(myLastHash)) {
+      processCollectedRecords();
     }
-    else {
-      myHashToRecord.putValue(record.getHash(), record);
-      if (parents.length == myHashToRecord.get(record.getHash()).size()) {
-        processCollectedRecords(false);
+    myLastHash = record.getHash();
+
+    myHashToRecord.putValue(record.getHash(), record);
+    if (record.getParentsHashes().length == myHashToRecord.get(record.getHash()).size()) {
+      processCollectedRecords();
+    }
+  }
+
+  public void finish() {
+    processCollectedRecords();
+  }
+
+  protected void processCollectedRecords() {
+    for (String hash : myHashToRecord.keySet()) {
+      ArrayList<GitLogRecord> records = ContainerUtil.newArrayList(notNull(myHashToRecord.get(hash)));
+      GitLogRecord firstRecord = records.get(0);
+      if (firstRecord.getParentsHashes().length != 0 && records.size() != firstRecord.getParentsHashes().length) {
+        processIncompleteRecord(hash, records);
+      }
+      else {
+        myConsumer.consume(records);
       }
     }
+    myHashToRecord.clear();
   }
 
-  private void processCollectedRecords(boolean processIncompleteRecords) {
+  protected void processIncompleteRecord(@NotNull String hash, @NotNull List<GitLogRecord> records) {
     // there is a surprising (or not really surprising, depending how to look at it) problem with `-m` option
     // despite what is written in git-log documentation, it does not always output a record for each parent of a merge commit
     // if a merge commit has no changes with one of the parents, nothing is output for that parent
@@ -78,55 +85,37 @@ abstract class GitLogRecordCollector implements Consumer<GitLogRecord> {
     // and there is no format option to display it
     // so the solution is to run another git log command and get tree hashes for all participating commits
     // tree hashes allow to determine, which parent of the commit in question is the same as the commit itself and create an empty record for it
-    for (String hash : myHashToRecord.keySet()) {
-      ArrayList<GitLogRecord> records = ContainerUtil.newArrayList(notNull(myHashToRecord.get(hash)));
-      GitLogRecord firstRecord = records.get(0);
-      if (firstRecord.getParentsHashes().length != 0 && records.size() != firstRecord.getParentsHashes().length) {
-        myHashToIncompleteRecords.put(hash, records);
-        records.forEach(r -> myIncompleteStatusLinesCount += r.getStatusInfos().size());
-      }
-      else {
-        consume(records);
-      }
+    MultiMap<String, GitLogRecord> incompleteRecords = MultiMap.create();
+    incompleteRecords.put(hash, records);
+    try {
+      processIncompleteRecords(incompleteRecords, myProject, myRoot, myConsumer);
     }
-    myHashToRecord.clear();
-
-    // we want to avoid executing a lot of "git log" commands
-    // at the same time we do not want to waste too much memory on records
-    // so we process "incomplete" records when we accumulate too much of them in terms of status lines
-    // or at the very end
-    if (!myHashToIncompleteRecords.isEmpty() && (processIncompleteRecords || myIncompleteStatusLinesCount >= STATUS_LINES_THRESHOLD)) {
-      try {
-        Map<String, String> hashToTreeMap = getHashToTreeMap(ContainerUtil.map(myHashToIncompleteRecords.entrySet(),
-                                                                               e -> ContainerUtil.getFirstItem(e.getValue())));
-        for (String hash : myHashToIncompleteRecords.keySet()) {
-          ArrayList<GitLogRecord> records = ContainerUtil.newArrayList(notNull(myHashToIncompleteRecords.get(hash)));
-          fillWithEmptyRecords(records, hashToTreeMap);
-          consume(records);
-        }
-      }
-      catch (VcsException e) {
-        LOG.error(e);
-      }
-      finally {
-        myHashToIncompleteRecords.clear();
-        myIncompleteStatusLinesCount = 0;
-        // do not keep records on error
-      }
+    catch (VcsException e) {
+      LOG.error(e);
     }
   }
 
-  public void finish() {
-    processCollectedRecords(true);
+  public static void processIncompleteRecords(@NotNull MultiMap<String, GitLogRecord> incompleteRecords,
+                                              @NotNull Project project,
+                                              @NotNull VirtualFile root,
+                                              @NotNull Consumer<List<GitLogRecord>> consumer) throws VcsException {
+    List<GitLogRecord> firstRecords = ContainerUtil.map(incompleteRecords.entrySet(), e -> ContainerUtil.getFirstItem(e.getValue()));
+    Map<String, String> hashToTreeMap = getHashToTreeMap(project, root, firstRecords);
+    for (String hash : incompleteRecords.keySet()) {
+      ArrayList<GitLogRecord> records = ContainerUtil.newArrayList(notNull(incompleteRecords.get(hash)));
+      fillWithEmptyRecords(records, hashToTreeMap);
+      consumer.consume(records);
+    }
   }
 
-  public abstract void consume(@NotNull List<GitLogRecord> records);
-
   /*
    * This method calculates tree hashes for commits and their parents.
    */
   @NotNull
-  private Map<String, String> getHashToTreeMap(@NotNull Collection<GitLogRecord> records) throws VcsException {
+  private static Map<String, String> getHashToTreeMap(@NotNull Project project,
+                                                      @NotNull VirtualFile root,
+                                                      @NotNull Collection<GitLogRecord> records)
+    throws VcsException {
     Set<String> hashes = ContainerUtil.newHashSet();
 
     for (GitLogRecord r : records) {
@@ -134,9 +123,9 @@ abstract class GitLogRecordCollector implements Consumer<GitLogRecord> {
       ContainerUtil.addAll(hashes, r.getParentsHashes());
     }
 
-    GitLineHandler handler = new GitLineHandler(myProject, myRoot, GitCommand.LOG);
-    GitLogParser parser = new GitLogParser(myProject, GitLogParser.NameStatus.NONE, HASH, TREE);
-    GitVcs vcs = GitVcs.getInstance(myProject);
+    GitLineHandler handler = new GitLineHandler(project, root, GitCommand.LOG);
+    GitLogParser parser = new GitLogParser(project, GitLogParser.NameStatus.NONE, HASH, TREE);
+    GitVcs vcs = GitVcs.getInstance(project);
     handler.setStdoutSuppressed(true);
     handler.addParameters(parser.getPretty());
     handler.addParameters(GitLogUtil.getNoWalkParameter(vcs));

plugins/git4idea/src/git4idea/history/GitLogUnorderedRecordCollector.java

+/*
+ * Copyright 2000-2017 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package git4idea.history;
+
+import com.intellij.openapi.diagnostic.Logger;
+import com.intellij.openapi.project.Project;
+import com.intellij.openapi.vcs.VcsException;
+import com.intellij.openapi.vfs.VirtualFile;
+import com.intellij.util.Consumer;
+import com.intellij.util.containers.MultiMap;
+import org.jetbrains.annotations.NotNull;
+
+import java.util.List;
+
+class GitLogUnorderedRecordCollector extends GitLogRecordCollector {
+  private static final Logger LOG = Logger.getInstance(GitLogUnorderedRecordCollector.class);
+  private static final int STATUS_LINES_THRESHOLD = 20_000;
+
+  @NotNull private final MultiMap<String, GitLogRecord> myHashToIncompleteRecords = MultiMap.createLinked();
+  private int myIncompleteStatusLinesCount = 0;
+
+  protected GitLogUnorderedRecordCollector(@NotNull Project project,
+                                           @NotNull VirtualFile root,
+                                           @NotNull Consumer<List<GitLogRecord>> consumer) {
+    super(project, root, consumer);
+  }
+
+  @Override
+  protected void processCollectedRecords() {
+    processCollectedRecords(false);
+  }
+
+  private void processCollectedRecords(boolean processIncompleteRecords) {
+    super.processCollectedRecords();
+
+    // we want to avoid executing a lot of "git log" commands
+    // at the same time we do not want to waste too much memory on records
+    // so we process "incomplete" records when we accumulate too much of them in terms of status lines
+    // or at the very end
+    if (!myHashToIncompleteRecords.isEmpty() && (processIncompleteRecords || myIncompleteStatusLinesCount >= STATUS_LINES_THRESHOLD)) {
+      try {
+        processIncompleteRecords(myHashToIncompleteRecords, myProject, myRoot, myConsumer);
+      }
+      catch (VcsException e) {
+        LOG.error(e);
+      }
+      finally {
+        myHashToIncompleteRecords.clear();
+        myIncompleteStatusLinesCount = 0;
+        // do not keep records on error
+      }
+    }
+  }
+
+  @Override
+  protected void processIncompleteRecord(@NotNull String hash, @NotNull List<GitLogRecord> records) {
+    myHashToIncompleteRecords.put(hash, records);
+    records.forEach(r -> myIncompleteStatusLinesCount += r.getStatusInfos().size());
+  }
+
+  @Override
+  public void finish() {
+    processCollectedRecords(true);
+  }
+}

plugins/git4idea/src/git4idea/history/GitLogUtil.java

@@ -228,7 +228,7 @@ public class GitLogUtil {
 
     List<GitCommit> commits = ContainerUtil.newArrayList();
     try {
-      readFullDetails(project, root, commits::add, true, parameters);
+      readFullDetails(project, root, commits::add, true, true, parameters);
     }
     catch (VcsException e) {
       if (commits.isEmpty()) {
@@ -243,6 +243,7 @@ public class GitLogUtil {
                                      @NotNull VirtualFile root,
                                      @NotNull Consumer<? super GitCommit> commitConsumer,
                                      boolean includeRootChanges,
+                                     boolean preserverOrder,
                                      @NotNull String... parameters) throws VcsException {
     VcsLogObjectsFactory factory = getObjectsFactoryWithDisposeCheck(project);
     if (factory == null) {
@@ -250,13 +251,15 @@ public class GitLogUtil {
     }
 
     DiffRenameLimit renameLimit = DiffRenameLimit.REGISTRY;
-    GitLogRecordCollector recordCollector = new GitLogRecordCollector(project, root) {
-      @Override
-      public void consume(@NotNull List<GitLogRecord> records) {
-        assertCorrectNumberOfRecords(records);
-        commitConsumer.consume(createCommit(project, root, records, factory, renameLimit));
-      }
+
+    Consumer<List<GitLogRecord>> consumer = records -> {
+      assertCorrectNumberOfRecords(records);
+      commitConsumer.consume(createCommit(project, root, records, factory, renameLimit));
     };
+
+    GitLogRecordCollector recordCollector = preserverOrder ? new GitLogRecordCollector(project, root, consumer)
+                                                           : new GitLogUnorderedRecordCollector(project, root, consumer);
+
     readRecords(project, root, false, true, includeRootChanges, renameLimit, recordCollector, parameters);
     recordCollector.finish();
   }
@@ -344,13 +347,10 @@ public class GitLogUtil {
       return;
     }
 
-    GitLogRecordCollector recordCollector = new GitLogRecordCollector(project, root) {
-      @Override
-      public void consume(@NotNull List<GitLogRecord> records) {
-        assertCorrectNumberOfRecords(records);
-        commitConsumer.consume(createCommit(project, root, records, factory, renameLimit));
-      }
-    };
+    GitLogRecordCollector recordCollector = new GitLogUnorderedRecordCollector(project, root, records -> {
+      assertCorrectNumberOfRecords(records);
+      commitConsumer.consume(createCommit(project, root, records, factory, renameLimit));
+    });
     GitLineHandler handler = createGitHandler(project, root, createConfigParameters(true, includeRootChanges, renameLimit));
     sendHashesToStdin(vcs, hashes, handler);
 

plugins/git4idea/src/git4idea/log/GitLogProvider.java

@@ -330,8 +330,8 @@ public class GitLogProvider implements VcsLogProvider {
       return;
     }
 
-    GitLogUtil
-      .readFullDetails(myProject, root, commitConsumer, shouldIncludeRootChanges(root), ArrayUtil.toStringArray(GitLogUtil.LOG_ALL));
+    GitLogUtil.readFullDetails(myProject, root, commitConsumer, shouldIncludeRootChanges(root),
+                               false, ArrayUtil.toStringArray(GitLogUtil.LOG_ALL));
   }
 
   @Override