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>
This commit is contained in:
parent
9a5f4b46cc
commit
3b41fcbd96
|
@ -642,29 +642,58 @@ public void testIndexOfChangeId() {
|
||||||
assertEquals(3, ChangeIdUtil.indexOfChangeId("x\n" + "\n"
|
assertEquals(3, ChangeIdUtil.indexOfChangeId("x\n" + "\n"
|
||||||
+ "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n",
|
+ "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\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 \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"
|
assertEquals(5, ChangeIdUtil.indexOfChangeId("x\r\n" + "\r\n"
|
||||||
+ "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\r\n",
|
+ "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\r\n",
|
||||||
"\r\n"));
|
"\r\n"));
|
||||||
assertEquals(3, ChangeIdUtil.indexOfChangeId("x\r" + "\r"
|
assertEquals(3, ChangeIdUtil.indexOfChangeId("x\r" + "\r"
|
||||||
+ "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\r",
|
+ "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\r",
|
||||||
"\r"));
|
"\r"));
|
||||||
|
assertEquals(3, ChangeIdUtil.indexOfChangeId("x\r" + "\r"
|
||||||
|
+ "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\r",
|
||||||
|
"\r"));
|
||||||
assertEquals(8, ChangeIdUtil.indexOfChangeId("x\ny\n\nz\n" + "\n"
|
assertEquals(8, ChangeIdUtil.indexOfChangeId("x\ny\n\nz\n" + "\n"
|
||||||
+ "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n",
|
+ "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n",
|
||||||
"\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 {
|
private void hookDoesNotModify(final String in) throws Exception {
|
||||||
assertEquals(in, call(in));
|
assertEquals(in, call(in));
|
||||||
}
|
}
|
||||||
|
|
|
@ -125,9 +125,14 @@ public static ObjectId computeChangeId(final ObjectId treeId,
|
||||||
private static final Pattern footerPattern = Pattern
|
private static final Pattern footerPattern = Pattern
|
||||||
.compile("(^[a-zA-Z0-9-]+:(?!//).*$)"); //$NON-NLS-1$
|
.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
|
private static final Pattern includeInFooterPattern = Pattern
|
||||||
.compile("^[ \\[].*$"); //$NON-NLS-1$
|
.compile("^[ \\[].*$"); //$NON-NLS-1$
|
||||||
|
|
||||||
|
private static final Pattern trailingWhitespace = Pattern.compile("\\s+$");
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Find the right place to insert a Change-Id and return it.
|
* Find the right place to insert a Change-Id and return it.
|
||||||
* <p>
|
* <p>
|
||||||
|
@ -209,8 +214,11 @@ public static String insertId(String message, ObjectId changeId,
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Find the index in the String {@code} message} where the Change-Id entry
|
* Return the index in the String {@code message} where the Change-Id entry
|
||||||
* begins
|
* 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 message
|
||||||
* @param delimiter
|
* @param delimiter
|
||||||
|
@ -221,14 +229,32 @@ public static String insertId(String message, ObjectId changeId,
|
||||||
*/
|
*/
|
||||||
public static int indexOfChangeId(String message, String delimiter) {
|
public static int indexOfChangeId(String message, String delimiter) {
|
||||||
String[] lines = message.split(delimiter);
|
String[] lines = message.split(delimiter);
|
||||||
int footerFirstLine = indexOfFirstFooterLine(lines);
|
int indexOfChangeIdLine = 0;
|
||||||
if (footerFirstLine == lines.length)
|
boolean inFooter = false;
|
||||||
return -1;
|
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;
|
private static boolean isEmptyLine(String line) {
|
||||||
for (int i = 0; i < footerFirstLine; ++i)
|
return line.trim().length() == 0;
|
||||||
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$
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
Loading…
Reference in New Issue