summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSasa Zivkov <sasa.zivkov@sap.com>2012-12-17 17:00:48 +0100
committerEdwin Kempin <edwin.kempin@sap.com>2012-12-28 10:14:21 +0100
commit68fc92e0817f8958a042bfab5c034bcafebf3cdc (patch)
treef53dba2b0359460784530319bdccaf19fd3a2abf
parentc9e11de77a48c12d8130e5d93e2ea9284c98edb5 (diff)
Fix: don't display all files from a merge-commit when auto-merge fails
The 60848f89959d41de83dfac2a0ee20d336cd7b3f1 tried to address the issue that the Change Screen couldn't open for a patch set with merge commit when the creation of an auto-merge tree failed in JGit (which happens when the parents of the merge commit have multiple merge bases as JGit doesn't support this case yet). It considered the failure to create the auto-merge tree as if the tree creation succeeded with conflicts. In fact, the auto merge tree was empty and Gerrit considered all paths as unmerged. This caused several issues: - the file list was too large for a project with large number of files - Gerrit would send too many false notification emails to those watching changes under certain paths - both client and server needed a lot of resources in order to handle such a large list of files With this change the file list will be empty when creation of auto-merge commit fails. This is not worse than showing a list of all files and it resolves the issues listed above. JGit will be able to auto-merge commits with multiple merge bases in a near future [1] which will further reduce the number of cases where Gerrit will incorrectly show an empty list of files for a patch-set with merge commit. [1] https://git.eclipse.org/r/8113 Change-Id: I76c51802f70d8d7f3d7cb797034ad19b77d84aa4 Signed-off-by: Sasa Zivkov <sasa.zivkov@sap.com> Signed-off-by: Christian Halstrick <christian.halstrick@sap.com>
-rw-r--r--gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java6
1 files changed, 5 insertions, 1 deletions
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java
index d27c205e47..bb231a505f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java
@@ -278,7 +278,11 @@ class PatchListLoader extends CacheLoader<PatchListKey, PatchList> {
try {
couldMerge = m.merge(b.getParents());
} catch (IOException e) {
- //
+ // It is not safe to continue further down in this method as throwing
+ // an exception most likely means that the merge tree was not created
+ // and m.getMergeResults() is empty. This would mean that all paths are
+ // unmerged and Gerrit UI would show all paths in the patch list.
+ return null;
}
if (couldMerge) {