diff options
6 files changed, 203 insertions, 57 deletions
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/SimilarityIndexTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/SimilarityIndexTest.java index 95423609a9..4724677bb8 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/SimilarityIndexTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/SimilarityIndexTest.java @@ -83,7 +83,7 @@ public class SimilarityIndexTest { + "B\n" // + "B\n").getBytes("UTF-8"); SimilarityIndex si = new SimilarityIndex(); - si.hash(new ByteArrayInputStream(in), in.length); + si.hash(new ByteArrayInputStream(in), in.length, false); assertEquals(2, si.size()); } @@ -104,6 +104,48 @@ public class SimilarityIndexTest { } @Test + public void testCommonScore_SameFiles_CR_canonicalization() + throws TableFullException { + String text = "" // + + "A\r\n" // + + "B\r\n" // + + "D\r\n" // + + "B\r\n"; + SimilarityIndex src = hash(text); + SimilarityIndex dst = hash(text.replace("\r", "")); + assertEquals(8, src.common(dst)); + assertEquals(8, dst.common(src)); + + assertEquals(100, src.score(dst, 100)); + assertEquals(100, dst.score(src, 100)); + } + + @Test + public void testCommonScoreLargeObject_SameFiles_CR_canonicalization() + throws TableFullException, IOException { + String text = "" // + + "A\r\n" // + + "B\r\n" // + + "D\r\n" // + + "B\r\n"; + SimilarityIndex src = new SimilarityIndex(); + byte[] bytes1 = text.getBytes("UTF-8"); + src.hash(new ByteArrayInputStream(bytes1), bytes1.length, true); + src.sort(); + + SimilarityIndex dst = new SimilarityIndex(); + byte[] bytes2 = text.replace("\r", "").getBytes("UTF-8"); + dst.hash(new ByteArrayInputStream(bytes2), bytes2.length, true); + dst.sort(); + + assertEquals(8, src.common(dst)); + assertEquals(8, dst.common(src)); + + assertEquals(100, src.score(dst, 100)); + assertEquals(100, dst.score(src, 100)); + } + + @Test public void testCommonScore_EmptyFiles() throws TableFullException { SimilarityIndex src = hash(""); SimilarityIndex dst = hash(""); @@ -132,24 +174,8 @@ public class SimilarityIndexTest { } private static SimilarityIndex hash(String text) throws TableFullException { - SimilarityIndex src = new SimilarityIndex() { - @Override - void hash(byte[] raw, int ptr, final int end) - throws TableFullException { - while (ptr < end) { - int hash = raw[ptr] & 0xff; - int start = ptr; - do { - int c = raw[ptr++] & 0xff; - if (c == '\n') - break; - } while (ptr < end && ptr - start < 64); - add(hash, ptr - start); - } - } - }; + SimilarityIndex src = new SimilarityIndex(); byte[] raw = Constants.encode(text); - src.setFileSize(raw.length); src.hash(raw, 0, raw.length); src.sort(); return src; diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreMatcherParametrizedTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreMatcherParametrizedTest.java index cbfe6f2790..699542c57f 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreMatcherParametrizedTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreMatcherParametrizedTest.java @@ -44,9 +44,12 @@ package org.eclipse.jgit.ignore; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeFalse; +import static org.junit.Assume.assumeTrue; import java.util.Arrays; +import org.eclipse.jgit.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -236,17 +239,63 @@ public class IgnoreMatcherParametrizedTest { } @Test - public void testTrailingSlash() { + public void testDirModeAndNoRegex() { String pattern = "/src/"; assertMatched(pattern, "/src/"); assertMatched(pattern, "/src/new"); assertMatched(pattern, "/src/new/a.c"); assertMatched(pattern, "/src/a.c"); + // no match as a "file" pattern, because rule is for directories only assertNotMatched(pattern, "/src"); assertNotMatched(pattern, "/srcA/"); } @Test + public void testDirModeAndRegex1() { + // IgnoreRule was buggy for some cases below, therefore using "Assume" + Boolean assume = useOldRule; + + String pattern = "a/*/src/"; + assertMatched(pattern, "a/b/src/"); + assertMatched(pattern, "a/b/src/new"); + assertMatched(pattern, "a/b/src/new/a.c"); + assertMatched(pattern, "a/b/src/a.c"); + // no match as a "file" pattern, because rule is for directories only + assertNotMatched(pattern, "a/b/src", assume); + assertNotMatched(pattern, "a/b/srcA/"); + } + + @Test + public void testDirModeAndRegex2() { + // IgnoreRule was buggy for some cases below, therefore using "Assume" + Boolean assume = useOldRule; + + String pattern = "a/[a-b]/src/"; + assertMatched(pattern, "a/b/src/"); + assertMatched(pattern, "a/b/src/new"); + assertMatched(pattern, "a/b/src/new/a.c"); + assertMatched(pattern, "a/b/src/a.c"); + // no match as a "file" pattern, because rule is for directories only + assertNotMatched(pattern, "a/b/src", assume); + assertNotMatched(pattern, "a/b/srcA/"); + } + + @Test + public void testDirModeAndRegex3() { + // IgnoreRule was buggy for some cases below, therefore using "Assume" + Boolean assume = useOldRule; + + String pattern = "**/src/"; + assertMatched(pattern, "a/b/src/", assume); + assertMatched(pattern, "a/b/src/new", assume); + assertMatched(pattern, "a/b/src/new/a.c", assume); + assertMatched(pattern, "a/b/src/a.c", assume); + // no match as a "file" pattern, because rule is for directories only + assertNotMatched(pattern, "a/b/src", assume); + assertNotMatched(pattern, "a/b/srcA/", assume); + } + + @Test public void testNameOnlyMatches() { /* * Name-only matches do not contain any path separators @@ -321,11 +370,16 @@ public class IgnoreMatcherParametrizedTest { * Pattern as it would appear in a .gitignore file * @param target * Target file path relative to repository's GIT_DIR + * @param assume */ - public void assertMatched(String pattern, String target) { + public void assertMatched(String pattern, String target, Boolean... assume) { boolean value = match(pattern, target); - assertTrue("Expected a match for: " + pattern + " with: " + target, - value); + if (assume.length == 0 || !assume[0].booleanValue()) + assertTrue("Expected a match for: " + pattern + " with: " + target, + value); + else + assumeTrue("Expected a match for: " + pattern + " with: " + target, + value); } /** @@ -336,11 +390,17 @@ public class IgnoreMatcherParametrizedTest { * Pattern as it would appear in a .gitignore file * @param target * Target file path relative to repository's GIT_DIR + * @param assume */ - public void assertNotMatched(String pattern, String target) { + public void assertNotMatched(String pattern, String target, + Boolean... assume) { boolean value = match(pattern, target); - assertFalse("Expected no match for: " + pattern + " with: " + target, - value); + if (assume.length == 0 || !assume[0].booleanValue()) + assertFalse("Expected no match for: " + pattern + " with: " + + target, value); + else + assumeFalse("Expected no match for: " + pattern + " with: " + + target, value); } /** @@ -355,16 +415,43 @@ public class IgnoreMatcherParametrizedTest { */ private boolean match(String pattern, String target) { boolean isDirectory = target.endsWith("/"); + boolean match; + if (useOldRule.booleanValue()) { + IgnoreRule r = new IgnoreRule(pattern); + match = r.isMatch(target, isDirectory); + } else { + FastIgnoreRule r = new FastIgnoreRule(pattern); + match = r.isMatch(target, isDirectory); + } + + if (isDirectory) { + boolean noTrailingSlash = matchAsDir(pattern, + target.substring(0, target.length() - 1)); + if (match != noTrailingSlash) { + String message = "Difference in result for directory pattern: " + + pattern + " with: " + target + + " if target is given without trailing slash"; + Assert.assertEquals(message, match, noTrailingSlash); + } + } + return match; + } + + /** + * + * @param target + * must not ends with a slash! + * @param pattern + * same as {@link #match(String, String)} + * @return same as {@link #match(String, String)} + */ + private boolean matchAsDir(String pattern, String target) { + assertFalse(target.endsWith("/")); if (useOldRule.booleanValue()) { IgnoreRule r = new IgnoreRule(pattern); - // If speed of this test is ever an issue, we can use a presetRule - // field - // to avoid recompiling a pattern each time. - return r.isMatch(target, isDirectory); + return r.isMatch(target, true); } FastIgnoreRule r = new FastIgnoreRule(pattern); - // If speed of this test is ever an issue, we can use a presetRule field - // to avoid recompiling a pattern each time. - return r.isMatch(target, isDirectory); + return r.isMatch(target, true); } } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/T0001_PersonIdentTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/T0001_PersonIdentTest.java index 1b7276bf77..315c495606 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/T0001_PersonIdentTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/T0001_PersonIdentTest.java @@ -44,6 +44,7 @@ package org.eclipse.jgit.lib; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import java.util.Date; import java.util.TimeZone; @@ -83,4 +84,15 @@ public class T0001_PersonIdentTest { public void nullForEmailShouldThrowIllegalArgumentException() { new PersonIdent("A U Thor", null); } + + @Test + public void testToExternalStringTrimsNameAndEmail() throws Exception { + PersonIdent personIdent = new PersonIdent(" A U Thor ", + " author@example.com "); + + String externalString = personIdent.toExternalString(); + + assertTrue(externalString.startsWith("A U Thor <author@example.com>")); + } + } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/diff/SimilarityIndex.java b/org.eclipse.jgit/src/org/eclipse/jgit/diff/SimilarityIndex.java index 17ccb9726f..f376b8e36e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/diff/SimilarityIndex.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/diff/SimilarityIndex.java @@ -79,8 +79,11 @@ class SimilarityIndex { /** Maximum value of the count field, also mask to extract the count. */ private static final long MAX_COUNT = (1L << KEY_SHIFT) - 1; - /** Total size of the file we hashed into the structure. */ - private long fileSize; + /** + * Total amount of bytes hashed into the structure, including \n. This is + * usually the size of the file minus number of CRLF encounters. + */ + private long hashedCnt; /** Number of non-zero entries in {@link #idHash}. */ private int idSize; @@ -108,48 +111,59 @@ class SimilarityIndex { idGrowAt = growAt(idHashBits); } - long getFileSize() { - return fileSize; - } - - void setFileSize(long size) { - fileSize = size; - } - void hash(ObjectLoader obj) throws MissingObjectException, IOException, TableFullException { if (obj.isLarge()) { - ObjectStream in = obj.openStream(); - try { - setFileSize(in.getSize()); - hash(in, fileSize); - } finally { - in.close(); - } + hashLargeObject(obj); } else { byte[] raw = obj.getCachedBytes(); - setFileSize(raw.length); hash(raw, 0, raw.length); } } + private void hashLargeObject(ObjectLoader obj) throws IOException, + TableFullException { + ObjectStream in1 = obj.openStream(); + boolean text; + try { + text = !RawText.isBinary(in1); + } finally { + in1.close(); + } + + ObjectStream in2 = obj.openStream(); + try { + hash(in2, in2.getSize(), text); + } finally { + in2.close(); + } + } + void hash(byte[] raw, int ptr, final int end) throws TableFullException { + final boolean text = !RawText.isBinary(raw); + hashedCnt = 0; while (ptr < end) { int hash = 5381; + int blockHashedCnt = 0; int start = ptr; // Hash one line, or one block, whichever occurs first. do { int c = raw[ptr++] & 0xff; + // Ignore CR in CRLF sequence if text + if (text && c == '\r' && ptr < end && raw[ptr] == '\n') + continue; + blockHashedCnt++; if (c == '\n') break; hash = (hash << 5) + hash + c; } while (ptr < end && ptr - start < 64); - add(hash, ptr - start); + hashedCnt += blockHashedCnt; + add(hash, blockHashedCnt); } } - void hash(InputStream in, long remaining) throws IOException, + void hash(InputStream in, long remaining, boolean text) throws IOException, TableFullException { byte[] buf = new byte[4096]; int ptr = 0; @@ -157,6 +171,7 @@ class SimilarityIndex { while (0 < remaining) { int hash = 5381; + int blockHashedCnt = 0; // Hash one line, or one block, whichever occurs first. int n = 0; @@ -170,11 +185,16 @@ class SimilarityIndex { n++; int c = buf[ptr++] & 0xff; + // Ignore CR in CRLF sequence if text + if (text && c == '\r' && ptr < cnt && buf[ptr] == '\n') + continue; + blockHashedCnt++; if (c == '\n') break; hash = (hash << 5) + hash + c; } while (n < 64 && n < remaining); - add(hash, n); + hashedCnt += blockHashedCnt; + add(hash, blockHashedCnt); remaining -= n; } } @@ -193,7 +213,7 @@ class SimilarityIndex { } int score(SimilarityIndex dst, int maxScore) { - long max = Math.max(fileSize, dst.fileSize); + long max = Math.max(hashedCnt, dst.hashedCnt); if (max == 0) return maxScore; return (int) ((common(dst) * maxScore) / max); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/PathMatcher.java b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/PathMatcher.java index dcecf303c4..d3e5f6a053 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/PathMatcher.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/PathMatcher.java @@ -217,7 +217,8 @@ public class PathMatcher extends AbstractMatcher { matcher++; match = matches(matcher, path, left, endExcl, assumeDirectory); - } else if (dirOnly) + } else if (dirOnly && !assumeDirectory) + // Directory expectations not met return false; } return match && matcher + 1 == matchers.size(); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/PersonIdent.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/PersonIdent.java index 69f7fd4404..8f7e3eff7c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/PersonIdent.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/PersonIdent.java @@ -254,9 +254,9 @@ public class PersonIdent implements Serializable { */ public String toExternalString() { final StringBuilder r = new StringBuilder(); - r.append(getName()); + r.append(getName().trim()); r.append(" <"); //$NON-NLS-1$ - r.append(getEmailAddress()); + r.append(getEmailAddress().trim()); r.append("> "); //$NON-NLS-1$ r.append(when / 1000); r.append(' '); |