From 3b41fcbd96610dc0fddded2aebc4e3b5c702b4fb Mon Sep 17 00:00:00 2001 From: Stefan Lay Date: Fri, 15 Feb 2013 12:34:44 +0100 Subject: [PATCH] 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 Signed-off-by: Matthias Sohn --- .../eclipse/jgit/util/ChangeIdUtilTest.java | 53 ++++++++++++++----- .../org/eclipse/jgit/util/ChangeIdUtil.java | 44 +++++++++++---- 2 files changed, 76 insertions(+), 21 deletions(-) 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 54f3114c6..66649b100 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 @@ public void testIndexOfChangeId() { 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 41bb4cc7e..7487f0d2b 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 @@ public static ObjectId computeChangeId(final ObjectId treeId, 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. *

@@ -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 - * 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 String insertId(String message, ObjectId changeId, */ 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$ } /**