Fix diff header on renamed or copied files

When git-core renames or copies a file and the mode differs the
header shows the mode change first, then the rename or copy data:

  diff --git a/COPYING b/LICENSE
  old mode 100644
  new mode 100755
  similarity index 92%
  rename from COPYING
  rename to LICENSE
  index d645695..54863be
  --- a/COPYING
  +++ b/LICENSE
  @@ -56,20 +56,6 @@

JGit relies on this ordering inside of FileHeader. Parsing "new file
mode NNN" after "copy from/to" or "rename from/to" resets the change
type to be ADD, losing the COPIED or RENAMED status and old path.

This fixes a 4 year old bug in Gerrit Code Review that prevents
opening a file for review if the file was copied from another file,
modified in this change, and the mode was updated (e.g. execute
bit was added).

Change-Id: If4c9ecd61ef0ca8e3e1ea857301f7b5c948efb96
[ms: added test case]
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffFormatterTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffFormatterTest.java
index 00e5f06747fb2a9ce2c3880eeb6474f0e214d1e5..145d899028a0b1072807c11a3060793a721fd702 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffFormatterTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffFormatterTest.java
@@ -285,6 +285,29 @@ public class DiffFormatterTest extends RepositoryTestCase {
                assertEquals(expected, fh.getScriptText());
        }
 
+       @Test
+       public void testCreateFileHeaderForRenameModeChange()
+                       throws Exception {
+               DiffEntry a = DiffEntry.delete(PATH_A, ObjectId.zeroId());
+               DiffEntry b = DiffEntry.add(PATH_B, ObjectId.zeroId());
+               b.oldMode = FileMode.REGULAR_FILE;
+               b.newMode = FileMode.EXECUTABLE_FILE;
+               DiffEntry m = DiffEntry.pair(ChangeType.RENAME, a, b, 100);
+               m.oldId = null;
+               m.newId = null;
+
+               FileHeader fh = df.toFileHeader(m);
+               //@formatter:off
+               String expected = DIFF + "a/src/a b/src/b\n" +
+                               "old mode 100644\n" +
+                               "new mode 100755\n" +
+                               "similarity index 100%\n" +
+                               "rename from src/a\n" +
+                               "rename to src/b\n";
+               //@formatter:on
+               assertEquals(expected, fh.getScriptText());
+       }
+
        @Test
        public void testDiff() throws Exception {
                write(new File(db.getDirectory().getParent(), "test.txt"), "test");

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java b/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java
index 4cd93191272b39008a0c3af926d805dace300950..8b1c023c52507ed79a8578521446ce145b931aed 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java
@@ -1044,6 +1044,17 @@ public class DiffFormatter {
 
                formatGitDiffFirstHeaderLine(o, type, oldp, newp);
 
+               if ((type == MODIFY || type == COPY || type == RENAME)
+                               && !oldMode.equals(newMode)) {
+                       o.write(encodeASCII("old mode ")); //$NON-NLS-1$
+                       oldMode.copyTo(o);
+                       o.write('\n');
+
+                       o.write(encodeASCII("new mode ")); //$NON-NLS-1$
+                       newMode.copyTo(o);
+                       o.write('\n');
+               }
+
                switch (type) {
                case ADD:
                        o.write(encodeASCII("new file mode ")); //$NON-NLS-1$
@@ -1077,12 +1088,6 @@ public class DiffFormatter {
 
                        o.write(encode("copy to " + quotePath(newp))); //$NON-NLS-1$
                        o.write('\n');
-
-                       if (!oldMode.equals(newMode)) {
-                               o.write(encodeASCII("new file mode ")); //$NON-NLS-1$
-                               newMode.copyTo(o);
-                               o.write('\n');
-                       }
                        break;
 
                case MODIFY:
@@ -1094,16 +1099,6 @@ public class DiffFormatter {
                        break;
                }
 
-               if ((type == MODIFY || type == RENAME) && !oldMode.equals(newMode)) {
-                       o.write(encodeASCII("old mode ")); //$NON-NLS-1$
-                       oldMode.copyTo(o);
-                       o.write('\n');
-
-                       o.write(encodeASCII("new mode ")); //$NON-NLS-1$
-                       newMode.copyTo(o);
-                       o.write('\n');
-               }
-
                if (ent.getOldId() != null && !ent.getOldId().equals(ent.getNewId())) {
                        formatIndexLine(o, ent);
                }