Browse Source

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>
stable-3.6
Andrey Loskutov 10 years ago committed by Matthias Sohn
parent
commit
c053900c5b
  1. 115
      org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreMatcherParametrizedTest.java
  2. 3
      org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/PathMatcher.java

115
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.assertFalse;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeFalse;
import static org.junit.Assume.assumeTrue;
import java.util.Arrays; import java.util.Arrays;
import org.eclipse.jgit.junit.Assert;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.junit.runners.Parameterized; import org.junit.runners.Parameterized;
@ -236,16 +239,62 @@ public class IgnoreMatcherParametrizedTest {
} }
@Test @Test
public void testTrailingSlash() { public void testDirModeAndNoRegex() {
String pattern = "/src/"; String pattern = "/src/";
assertMatched(pattern, "/src/"); assertMatched(pattern, "/src/");
assertMatched(pattern, "/src/new"); assertMatched(pattern, "/src/new");
assertMatched(pattern, "/src/new/a.c"); assertMatched(pattern, "/src/new/a.c");
assertMatched(pattern, "/src/a.c"); assertMatched(pattern, "/src/a.c");
// no match as a "file" pattern, because rule is for directories only
assertNotMatched(pattern, "/src"); assertNotMatched(pattern, "/src");
assertNotMatched(pattern, "/srcA/"); 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 @Test
public void testNameOnlyMatches() { public void testNameOnlyMatches() {
/* /*
@ -321,11 +370,16 @@ public class IgnoreMatcherParametrizedTest {
* Pattern as it would appear in a .gitignore file * Pattern as it would appear in a .gitignore file
* @param target * @param target
* Target file path relative to repository's GIT_DIR * 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); boolean value = match(pattern, target);
assertTrue("Expected a match for: " + pattern + " with: " + target, if (assume.length == 0 || !assume[0].booleanValue())
value); 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 * Pattern as it would appear in a .gitignore file
* @param target * @param target
* Target file path relative to repository's GIT_DIR * 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); boolean value = match(pattern, target);
assertFalse("Expected no match for: " + pattern + " with: " + target, if (assume.length == 0 || !assume[0].booleanValue())
value); 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) { private boolean match(String pattern, String target) {
boolean isDirectory = target.endsWith("/"); 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()) { if (useOldRule.booleanValue()) {
IgnoreRule r = new IgnoreRule(pattern); IgnoreRule r = new IgnoreRule(pattern);
// If speed of this test is ever an issue, we can use a presetRule return r.isMatch(target, true);
// field
// to avoid recompiling a pattern each time.
return r.isMatch(target, isDirectory);
} }
FastIgnoreRule r = new FastIgnoreRule(pattern); FastIgnoreRule r = new FastIgnoreRule(pattern);
// If speed of this test is ever an issue, we can use a presetRule field return r.isMatch(target, true);
// to avoid recompiling a pattern each time.
return r.isMatch(target, isDirectory);
} }
} }

3
org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/PathMatcher.java

@ -217,7 +217,8 @@ public class PathMatcher extends AbstractMatcher {
matcher++; matcher++;
match = matches(matcher, path, left, endExcl, match = matches(matcher, path, left, endExcl,
assumeDirectory); assumeDirectory);
} else if (dirOnly) } else if (dirOnly && !assumeDirectory)
// Directory expectations not met
return false; return false;
} }
return match && matcher + 1 == matchers.size(); return match && matcher + 1 == matchers.size();

Loading…
Cancel
Save