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 <markus.duft@salomon.at>
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>

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 cbfe6f2790402b5cda9441e466644b4b8a8e6f01..699542c57f8dece4e5bb334f7cbd7e7e681ae07a 100644 (file)
--- 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 dcecf303c4394255271169488ba47989b7474c41..d3e5f6a05369e31fa67d49c4a8f25e2b2c029aba 100644 (file)
--- 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();