diff options
author | Sasa Zivkov <sasa.zivkov@sap.com> | 2012-12-17 17:00:48 +0100 |
---|---|---|
committer | Edwin Kempin <edwin.kempin@sap.com> | 2012-12-28 10:14:21 +0100 |
commit | 68fc92e0817f8958a042bfab5c034bcafebf3cdc (patch) | |
tree | f53dba2b0359460784530319bdccaf19fd3a2abf | |
parent | c9e11de77a48c12d8130e5d93e2ea9284c98edb5 (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.java | 6 |
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) { |