From c053900c5bc89d2b55c563e04bc1edab53cdc143 Mon Sep 17 00:00:00 2001 From: Andrey Loskutov Date: Sun, 14 Dec 2014 18:07:59 +0100 Subject: [PATCH] PathMatcher should respect "assumeDirectory" flag The path matcher should not fail if the rule ends with trailing slash, target pattern does not ends with the slash and the "assumeDirectory" flag is set. E.g. */bin/ should also match a/bin if this pattern is threated as directory by WorkingTreeIterator (FileMode.TREE). The old code/tests have never tested directory rules with patterns *without* trailing slashes but with the "assumeDirectory" flag set. Unfortunately this is exactly what WorkingTreeIterator does... The tests are changed to test *both* cases now (with trailing slash and without) if the target pattern has trailing slash (represents directory). Bug: 454672 Change-Id: I621c1644d9e94df3eb9f6f09c6de0fe51f0950a4 Also-by: Markus Duft Signed-off-by: Andrey Loskutov --- .../ignore/IgnoreMatcherParametrizedTest.java | 115 +++++++++++++++--- .../jgit/ignore/internal/PathMatcher.java | 3 +- 2 files changed, 103 insertions(+), 15 deletions(-) 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 cbfe6f279..699542c57 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,16 +239,62 @@ 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() { /* @@ -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/src/org/eclipse/jgit/ignore/internal/PathMatcher.java b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/PathMatcher.java index dcecf303c..d3e5f6a05 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();