On a subtree, a PathSuffixFilter must return -1 ("indeterminate"), not 0 ("include"), otherwise negation goes wrong: an indeterminate result (-1) is passed on, but a decision (0/1) is inverted. As a result a negated PathSuffixFilter would skip all folders. Bug: 574253 Change-Id: I27fe785c0d772392a5b5efe0a7b1c9cafcb6e566 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>changes/26/182126/2
@@ -1,5 +1,5 @@ | |||
/* | |||
* Copyright (C) 2010, 2020 Google Inc. and others | |||
* Copyright (C) 2010, 2021 Google Inc. and others | |||
* | |||
* This program and the accompanying materials are made available under the | |||
* terms of the Eclipse Distribution License v. 1.0 which is available at | |||
@@ -18,6 +18,8 @@ import static org.junit.Assert.assertTrue; | |||
import java.io.BufferedOutputStream; | |||
import java.io.ByteArrayOutputStream; | |||
import java.io.File; | |||
import java.util.ArrayList; | |||
import java.util.List; | |||
import org.eclipse.jgit.api.Git; | |||
import org.eclipse.jgit.api.Status; | |||
@@ -34,8 +36,12 @@ import org.eclipse.jgit.patch.FileHeader; | |||
import org.eclipse.jgit.patch.HunkHeader; | |||
import org.eclipse.jgit.revwalk.RevCommit; | |||
import org.eclipse.jgit.storage.file.FileBasedConfig; | |||
import org.eclipse.jgit.treewalk.CanonicalTreeParser; | |||
import org.eclipse.jgit.treewalk.FileTreeIterator; | |||
import org.eclipse.jgit.treewalk.filter.OrTreeFilter; | |||
import org.eclipse.jgit.treewalk.filter.PathFilter; | |||
import org.eclipse.jgit.treewalk.filter.PathSuffixFilter; | |||
import org.eclipse.jgit.treewalk.filter.TreeFilter; | |||
import org.eclipse.jgit.util.FileUtils; | |||
import org.eclipse.jgit.util.RawParseUtils; | |||
import org.eclipse.jgit.util.io.DisabledOutputStream; | |||
@@ -491,6 +497,68 @@ public class DiffFormatterTest extends RepositoryTestCase { | |||
} | |||
} | |||
@Test | |||
public void testFilter() throws Exception { | |||
RevCommit parent; | |||
RevCommit head; | |||
try (Git git = new Git(db)) { | |||
writeTrashFile("foo.txt", "foo\n"); | |||
writeTrashFile("src/some.txt", "some\n"); | |||
writeTrashFile("src/image.png", "image\n"); | |||
writeTrashFile("src/test.pdf", "test\n"); | |||
writeTrashFile("src/xyz.txt", "xyz\n"); | |||
git.add().addFilepattern(".").call(); | |||
parent = git.commit().setMessage("initial").call(); | |||
writeTrashFile("foo.txt", "FOO\n"); | |||
writeTrashFile("src/some.txt", "SOME\n"); | |||
writeTrashFile("src/image.png", "IMAGE\n"); | |||
writeTrashFile("src/test.pdf", "TEST\n"); | |||
writeTrashFile("src/xyz.txt", "XYZ\n"); | |||
git.add().addFilepattern(".").call(); | |||
head = git.commit().setMessage("second").call(); | |||
} | |||
try (ByteArrayOutputStream os = new ByteArrayOutputStream(); | |||
DiffFormatter dfmt = new DiffFormatter(os)) { | |||
dfmt.setRepository(db); | |||
List<TreeFilter> skip = new ArrayList<>(); | |||
skip.add(PathSuffixFilter.create(".png")); | |||
skip.add(PathSuffixFilter.create(".pdf")); | |||
dfmt.setPathFilter(OrTreeFilter.create(skip).negate()); | |||
dfmt.format( | |||
new CanonicalTreeParser(null, db.newObjectReader(), | |||
parent.getTree()), | |||
new CanonicalTreeParser(null, db.newObjectReader(), | |||
head.getTree())); | |||
dfmt.flush(); | |||
String actual = os.toString("UTF-8"); | |||
String expected = "diff --git a/foo.txt b/foo.txt\n" | |||
+ "index 257cc56..b7d6715 100644\n" | |||
+ "--- a/foo.txt\n" | |||
+ "+++ b/foo.txt\n" | |||
+ "@@ -1 +1 @@\n" | |||
+ "-foo\n" | |||
+ "+FOO\n" | |||
+ "diff --git a/src/some.txt b/src/some.txt\n" | |||
+ "index 363ef61..76cea5f 100644\n" | |||
+ "--- a/src/some.txt\n" | |||
+ "+++ b/src/some.txt\n" | |||
+ "@@ -1 +1 @@\n" | |||
+ "-some\n" | |||
+ "+SOME\n" | |||
+ "diff --git a/src/xyz.txt b/src/xyz.txt\n" | |||
+ "index cd470e6..d4e3ab0 100644\n" | |||
+ "--- a/src/xyz.txt\n" | |||
+ "+++ b/src/xyz.txt\n" | |||
+ "@@ -1 +1 @@\n" | |||
+ "-xyz\n" | |||
+ "+XYZ\n"; | |||
assertEquals(expected, actual); | |||
} | |||
} | |||
@Test | |||
public void testTrackedFileInIgnoredFolderChanged() | |||
throws Exception { |
@@ -1,5 +1,5 @@ | |||
/* | |||
* Copyright (C) 2009, 2013 Google Inc. and others | |||
* Copyright (C) 2009, 2021 Google Inc. and others | |||
* | |||
* This program and the accompanying materials are made available under the | |||
* terms of the Eclipse Distribution License v. 1.0 which is available at | |||
@@ -41,10 +41,11 @@ public class PathSuffixFilterTest extends RepositoryTestCase { | |||
@Test | |||
public void testRecursiveFiltering() throws IOException { | |||
ObjectId treeId = createTree("a.sth", "a.txt", "sub/b.sth", "sub/b.txt"); | |||
ObjectId treeId = createTree("a.sth", "a.txt", "sub/b.sth", "sub/b.txt", | |||
"t.sth", "t.txt"); | |||
List<String> paths = getMatchingPaths(".txt", treeId, true); | |||
List<String> expected = Arrays.asList("a.txt", "sub/b.txt"); | |||
List<String> expected = Arrays.asList("a.txt", "sub/b.txt", "t.txt"); | |||
assertEquals(expected, paths); | |||
} | |||
@@ -59,6 +60,17 @@ public class PathSuffixFilterTest extends RepositoryTestCase { | |||
assertEquals(Arrays.asList("abc", "c"), getMatchingPaths("c", treeId)); | |||
} | |||
@Test | |||
public void testNegated() throws IOException { | |||
ObjectId treeId = createTree("a.sth", "a.txt", "sub/b.sth", | |||
"sub/b.txt", "t.sth", "t.txt"); | |||
List<String> paths = getMatchingPaths(".txt", treeId, true, true); | |||
List<String> expected = Arrays.asList("a.sth", "sub/b.sth", "t.sth"); | |||
assertEquals(expected, paths); | |||
} | |||
private ObjectId createTree(String... paths) throws IOException { | |||
final ObjectInserter odi = db.newObjectInserter(); | |||
final DirCache dc = db.readDirCache(); | |||
@@ -80,8 +92,18 @@ public class PathSuffixFilterTest extends RepositoryTestCase { | |||
private List<String> getMatchingPaths(String suffixFilter, | |||
final ObjectId treeId, boolean recursiveWalk) throws IOException { | |||
return getMatchingPaths(suffixFilter, treeId, recursiveWalk, false); | |||
} | |||
private List<String> getMatchingPaths(String suffixFilter, | |||
final ObjectId treeId, boolean recursiveWalk, boolean negated) | |||
throws IOException { | |||
try (TreeWalk tw = new TreeWalk(db)) { | |||
tw.setFilter(PathSuffixFilter.create(suffixFilter)); | |||
TreeFilter filter = PathSuffixFilter.create(suffixFilter); | |||
if (negated) { | |||
filter = filter.negate(); | |||
} | |||
tw.setFilter(filter); | |||
tw.setRecursive(recursiveWalk); | |||
tw.addTree(treeId); | |||
@@ -1,5 +1,5 @@ | |||
/* | |||
* Copyright (C) 2009, Google Inc. and others | |||
* Copyright (C) 2009, 2021 Google Inc. and others | |||
* | |||
* This program and the accompanying materials are made available under the | |||
* terms of the Eclipse Distribution License v. 1.0 which is available at | |||
@@ -73,6 +73,15 @@ public class PathSuffixFilter extends TreeFilter { | |||
} | |||
@Override | |||
public int matchFilter(TreeWalk walker) throws MissingObjectException, | |||
IncorrectObjectTypeException, IOException { | |||
if (walker.isSubtree()) { | |||
return -1; | |||
} | |||
return super.matchFilter(walker); | |||
} | |||
/** {@inheritDoc} */ | |||
@Override | |||
public boolean shouldBeRecursive() { |