Merge "Accept Change-Id even if footer contains not well-formed entries" into stable-2.3
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 54f3114..66649b1 100644
--- 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 @@
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 41bb4cc..7487f0d 100644
--- 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 @@
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 @@
}
/**
- * 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 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);
+ }
- int indexOfFooter = 0;
- for (int i = 0; i < footerFirstLine; ++i)
- indexOfFooter += lines[i].length() + delimiter.length();
- return message.indexOf(CHANGE_ID, indexOfFooter);
+ private static boolean isEmptyLine(String line) {
+ return line.trim().length() == 0;
+ }
+
+ private static String trimRight(String s) {
+ return trailingWhitespace.matcher(s).replaceAll(""); //$NON-NLS-1$
}
/**
