Show submodule difference properly in side by side view

Previous versions of JGit incorrectly include the submodule 'Subproject
commit' diff lines as part of the diff header instead of as part of a
diff hunk. Newer versions of JGit fix this. However, the way that
submodule differences are displayed depends on this old broken behavior.

The extra header lines are shown in place of the normal side by side
view. In addition, gerrit doesn't know how to show gitlink files. Fix
this by removing the locations hard coded to check for GITLINK files.
Modify the PatchFile output to be able to show the C git style
'Subproject commit <sha1sum>' format. Add the same format conditioned on
OBJ_COMMIT into PatchScriptBuilder.

The end result will be a proper side-by-side view of Submodules. It does
not currently include any extra information. A future enhancement shall
use a Repository object to perform lookups on the actual submodule
repository to determine the extend of what commits were added or
removed.

Bug: Issue 3071
Change-Id: Icf62d74c27ed81bf2c07b5ce0e18a527705f1631
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>

4 files changed, 12 insertions, 18 deletions

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchFile.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchFile.java
index e479ba9880..2704be8746 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchFile.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchFile.java
@@ -150,9 +150,13 @@ public class PatchFile {
     if (tw == null) {
       return Text.EMPTY;
     }
-    if (tw.getFileMode(0).getObjectType() != Constants.OBJ_BLOB) {
+    if (tw.getFileMode(0).getObjectType() == Constants.OBJ_BLOB) {
+      return new Text(repo.open(tw.getObjectId(0), Constants.OBJ_BLOB));
+    } else if (tw.getFileMode(0).getObjectType() == Constants.OBJ_COMMIT) {
+      String str = "Subproject commit " + ObjectId.toString(tw.getObjectId(0));
+      return new Text(str.getBytes());
+    } else {
       return Text.EMPTY;
     }
-    return new Text(repo.open(tw.getObjectId(0), Constants.OBJ_BLOB));
   }
 }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListEntry.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListEntry.java
index 5de28f3e87..319483a7c9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListEntry.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListEntry.java
@@ -32,7 +32,6 @@ import com.google.gerrit.reviewdb.client.PatchSet;
 
 import org.eclipse.jgit.diff.Edit;
 import org.eclipse.jgit.lib.Constants;
-import org.eclipse.jgit.lib.FileMode;
 import org.eclipse.jgit.patch.CombinedFileHeader;
 import org.eclipse.jgit.patch.FileHeader;
 import org.eclipse.jgit.util.IntList;
@@ -94,10 +93,7 @@ public class PatchListEntry {
 
     header = compact(hdr);
 
-    if (hdr instanceof CombinedFileHeader
-        || hdr.getHunks().isEmpty() //
-        || hdr.getOldMode() == FileMode.GITLINK
-        || hdr.getNewMode() == FileMode.GITLINK) {
+    if (hdr instanceof CombinedFileHeader || hdr.getHunks().isEmpty()) {
       edits = Collections.emptyList();
     } else {
       edits = Collections.unmodifiableList(editList);
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 aa2a8a8429..e7d86ab722 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
@@ -311,14 +311,6 @@ public class PatchListLoader implements Callable<PatchList> {
 
   private PatchListEntry newEntry(RevTree aTree, FileHeader fileHeader,
       long sizeDelta) {
-    final FileMode oldMode = fileHeader.getOldMode();
-    final FileMode newMode = fileHeader.getNewMode();
-
-    if (oldMode == FileMode.GITLINK || newMode == FileMode.GITLINK) {
-      return new PatchListEntry(fileHeader, Collections.<Edit> emptyList(),
-          sizeDelta);
-    }
-
     if (aTree == null // want combined diff
         || fileHeader.getPatchType() != PatchType.UNIFIED
         || fileHeader.getHunks().isEmpty()) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
index d0c3d09795..c26aea81d7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
@@ -179,9 +179,7 @@ class PatchScriptBuilder {
     }
 
     boolean hugeFile = false;
-    if (a.mode == FileMode.GITLINK || b.mode == FileMode.GITLINK) {
-      // Do nothing
-    } else if (a.src == b.src && a.size() <= context
+    if (a.src == b.src && a.size() <= context
         && content.getEdits().isEmpty()) {
       // Odd special case; the files are identical (100% rename or copy)
       // and the user has asked for context that is larger than the file.
@@ -468,6 +466,10 @@ class PatchScriptBuilder {
           } else if (mode.getObjectType() == Constants.OBJ_BLOB) {
             srcContent = Text.asByteArray(db.open(id, Constants.OBJ_BLOB));
 
+          } else if (mode.getObjectType() == Constants.OBJ_COMMIT) {
+            String strContent = "Subproject commit " + ObjectId.toString(id);
+            srcContent = strContent.getBytes();
+
           } else {
             srcContent = Text.NO_BYTES;
           }