Accept Change-Id even if footer contains not well-formed entries

Instead of only looking for a Change-Id in the last section if it
consists only of well-formed "key: value" lines replace the last
occurrence of a valid Change-Id line in the last section. Some tools
require footer lines e.g. without a colon.

Gerrit doesn't accept Change-Id lines in the footer if the Change-Id
line doesn't start at the beginning of the line.

Bug: 400818
Change-Id: Icce54872adc8c566994beea848448a2f7ca87085
Signed-off-by: Stefan Lay <stefan.lay@sap.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/ChangeIdUtilTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/ChangeIdUtilTest.java
index 54f3114c6d9970827be7db8540c7defc36b4e890..66649b10066a86f52187575a408cca1cf61966ec 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/ChangeIdUtilTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/ChangeIdUtilTest.java
@@ -642,29 +642,58 @@ public class ChangeIdUtilTest {
                assertEquals(3, ChangeIdUtil.indexOfChangeId("x\n" + "\n"
                                + "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n",
                                "\n"));
+               assertEquals(3, ChangeIdUtil.indexOfChangeId("x\n" + "\n"
+                               + "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n\n\n",
+                               "\n"));
+               assertEquals(3, ChangeIdUtil.indexOfChangeId("x\n" + "\n"
+                                                                               + "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n \n \n",
+                                                               "\n"));
+               assertEquals(3, ChangeIdUtil.indexOfChangeId("x\n" + "\n"
+                               + "Change-Id:  I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n",
+                               "\n"));
+
+               // leading whitespace is rejected by Gerrit
+               assertEquals(-1, ChangeIdUtil.indexOfChangeId("x\n" + "\n"
+                               + " Change-Id:  I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n",
+                               "\n"));
+               assertEquals(-1, ChangeIdUtil.indexOfChangeId("x\n" + "\n"
+                               + "\t Change-Id:  I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n",
+                               "\n"));
+
+               assertEquals(-1, ChangeIdUtil.indexOfChangeId("x\n" + "\n"
+                               + "Change-Id: \n", "\n"));
+               assertEquals(3, ChangeIdUtil.indexOfChangeId("x\n" + "\n"
+                               + "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701 \n",
+                               "\n"));
+               assertEquals(12, ChangeIdUtil.indexOfChangeId("x\n" + "\n"
+                               + "Bug 4711\n"
+                               + "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n",
+                               "\n"));
+               assertEquals(56, ChangeIdUtil.indexOfChangeId("x\n"
+                               + "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n"
+                               + "\n"
+                               + "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n",
+                               "\n"));
+               assertEquals(-1, ChangeIdUtil.indexOfChangeId("x\n"
+                               + "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n"
+                               + "\n" + "x\n", "\n"));
+               assertEquals(-1, ChangeIdUtil.indexOfChangeId("x\n\n"
+                               + "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n"
+                               + "\n" + "x\n", "\n"));
                assertEquals(5, ChangeIdUtil.indexOfChangeId("x\r\n" + "\r\n"
                                + "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\r\n",
                                "\r\n"));
                assertEquals(3, ChangeIdUtil.indexOfChangeId("x\r" + "\r"
                                + "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\r",
                                "\r"));
+               assertEquals(3, ChangeIdUtil.indexOfChangeId("x\r" + "\r"
+                               + "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\r",
+                               "\r"));
                assertEquals(8, ChangeIdUtil.indexOfChangeId("x\ny\n\nz\n" + "\n"
                                + "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n",
                                "\n"));
        }
 
-       @Test
-       public void testIndexOfFirstFooterLine() {
-               assertEquals(
-                               2,
-                               ChangeIdUtil.indexOfFirstFooterLine(new String[] { "a", "",
-                                               "Bug: 42", "Signed-Off-By: j.developer@a.com" }));
-               assertEquals(
-                               3,
-                               ChangeIdUtil.indexOfFirstFooterLine(new String[] { "a",
-                                               "Bug: 42", "", "Signed-Off-By: j.developer@a.com" }));
-       }
-
        private void hookDoesNotModify(final String in) throws Exception {
                assertEquals(in, call(in));
        }

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/ChangeIdUtil.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/ChangeIdUtil.java
index 41bb4cc7eb9aff29e188bde5d917ddb3ef4b4214..7487f0d2b902c0065ed4e885513db80a0e3182fb 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/ChangeIdUtil.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/ChangeIdUtil.java
@@ -125,9 +125,14 @@ public class ChangeIdUtil {
        private static final Pattern footerPattern = Pattern
                        .compile("(^[a-zA-Z0-9-]+:(?!//).*$)"); //$NON-NLS-1$
 
+       private static final Pattern changeIdPattern = Pattern
+                       .compile("(^" + CHANGE_ID + " *I[a-f0-9]{40}$)"); //$NON-NLS-1$ //$NON-NLS-2$
+
        private static final Pattern includeInFooterPattern = Pattern
                        .compile("^[ \\[].*$"); //$NON-NLS-1$
 
+       private static final Pattern trailingWhitespace = Pattern.compile("\\s+$");
+
        /**
         * Find the right place to insert a Change-Id and return it.
         * <p>
@@ -209,8 +214,11 @@ public class ChangeIdUtil {
        }
 
        /**
-        * Find the index in the String {@code} message} where the Change-Id entry
-        * begins
+        * Return the index in the String {@code message} where the Change-Id entry
+        * in the footer begins. If there are more than one entries matching the
+        * pattern, return the index of the last one in the last section. Because of
+        * Bug: 400818 we release the constraint here that a footer must contain
+        * only lines matching {@code footerPattern}.
         *
         * @param message
         * @param delimiter
@@ -221,14 +229,32 @@ public class ChangeIdUtil {
         */
        public static int indexOfChangeId(String message, String delimiter) {
                String[] lines = message.split(delimiter);
-               int footerFirstLine = indexOfFirstFooterLine(lines);
-               if (footerFirstLine == lines.length)
-                       return -1;
+               int indexOfChangeIdLine = 0;
+               boolean inFooter = false;
+               for (int i = lines.length - 1; i >= 0; --i) {
+                       if (!inFooter && isEmptyLine(lines[i]))
+                               continue;
+                       inFooter = true;
+                       if (changeIdPattern.matcher(trimRight(lines[i])).matches()) {
+                               indexOfChangeIdLine = i;
+                               break;
+                       } else if (isEmptyLine(lines[i]) || i == 0)
+                               return -1;
+               }
+               int indexOfChangeIdLineinString = 0;
+               for (int i = 0; i < indexOfChangeIdLine; ++i)
+                       indexOfChangeIdLineinString += lines[i].length()
+                                       + delimiter.length();
+               return indexOfChangeIdLineinString
+                               + lines[indexOfChangeIdLine].indexOf(CHANGE_ID);
+       }
+
+       private static boolean isEmptyLine(String line) {
+               return line.trim().length() == 0;
+       }
 
-               int indexOfFooter = 0;
-               for (int i = 0; i < footerFirstLine; ++i)
-                       indexOfFooter += lines[i].length() + delimiter.length();
-               return message.indexOf(CHANGE_ID, indexOfFooter);
+       private static String trimRight(String s) {
+               return trailingWhitespace.matcher(s).replaceAll(""); //$NON-NLS-1$
        }
 
        /**