Browse Source

Skip ignored directories in FileTreeIterator

Make FileTreeIterator not enter ignored directories by default. We
only need to enter ignored directories if we do some operation against
git, and there is at least one tracked file underneath an ignored
directory.

Walking ignored directories should be avoided as much as possible as
it is a potential performance bottleneck. Some projects have a lot of
files or very deep hierarchies in ignored directories; walking those
may be costly (especially so on Windows). See for instance also bug
500106.

Provide a FileTreeIterator.setWalkIgnoredDirectories() operation to
force the iterator to iterate also through otherwise ignored
directories. Useful for tests (IgnoreNodeTest, CGitIgnoreTest), or
to implement things like "git ls-files --ignored".

Add tests in DirCacheCheckoutTest, and amend IndexDiffTest to test a
little bit more.

Bug: 388582
Change-Id: I6ff584a42c55a07120a4369fd308409431bdb94a
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
stable-5.1
Thomas Wolf 7 years ago committed by Matthias Sohn
parent
commit
d7deda98d0
  1. 4
      org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/CGitIgnoreTest.java
  2. 4
      org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreNodeTest.java
  3. 113
      org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/DirCacheCheckoutTest.java
  4. 5
      org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/IndexDiffTest.java
  5. 30
      org.eclipse.jgit/src/org/eclipse/jgit/treewalk/FileTreeIterator.java
  6. 44
      org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java

4
org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/CGitIgnoreTest.java

@ -141,7 +141,9 @@ public class CGitIgnoreTest extends RepositoryTestCase {
// Do a tree walk that does descend into ignored directories and return // Do a tree walk that does descend into ignored directories and return
// a list of all ignored files // a list of all ignored files
try (TreeWalk walk = new TreeWalk(db)) { try (TreeWalk walk = new TreeWalk(db)) {
walk.addTree(new FileTreeIterator(db)); FileTreeIterator iter = new FileTreeIterator(db);
iter.setWalkIgnoredDirectories(true);
walk.addTree(iter);
walk.setRecursive(true); walk.setRecursive(true);
while (walk.next()) { while (walk.next()) {
if (walk.getTree(WorkingTreeIterator.class).isEntryIgnored()) { if (walk.getTree(WorkingTreeIterator.class).isEntryIgnored()) {

4
org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreNodeTest.java

@ -732,7 +732,9 @@ public class IgnoreNodeTest extends RepositoryTestCase {
private void beginWalk() { private void beginWalk() {
walk = new TreeWalk(db); walk = new TreeWalk(db);
walk.addTree(new FileTreeIterator(db)); FileTreeIterator iter = new FileTreeIterator(db);
iter.setWalkIgnoredDirectories(true);
walk.addTree(iter);
} }
private void endWalk() throws IOException { private void endWalk() throws IOException {

113
org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/DirCacheCheckoutTest.java

@ -78,8 +78,10 @@ import org.eclipse.jgit.junit.RepositoryTestCase;
import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.junit.TestRepository.BranchBuilder; import org.eclipse.jgit.junit.TestRepository.BranchBuilder;
import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.treewalk.AbstractTreeIterator;
import org.eclipse.jgit.treewalk.FileTreeIterator; import org.eclipse.jgit.treewalk.FileTreeIterator;
import org.eclipse.jgit.treewalk.TreeWalk; import org.eclipse.jgit.treewalk.TreeWalk;
import org.eclipse.jgit.treewalk.WorkingTreeIterator;
import org.eclipse.jgit.util.FS; import org.eclipse.jgit.util.FS;
import org.eclipse.jgit.util.FileUtils; import org.eclipse.jgit.util.FileUtils;
import org.junit.Assume; import org.junit.Assume;
@ -1904,6 +1906,117 @@ public class DirCacheCheckoutTest extends RepositoryTestCase {
assertUpdated(longFileName); assertUpdated(longFileName);
} }
@Test
public void testIgnoredDirectory() throws Exception {
writeTrashFile(".gitignore", "src/ignored");
writeTrashFile("src/ignored/sub/foo.txt", "1");
try (Git git = new Git(db)) {
git.add().addFilepattern(".").call();
RevCommit commit = git.commit().setMessage("adding .gitignore")
.call();
writeTrashFile("foo.txt", "2");
writeTrashFile("zzz.txt", "3");
git.add().addFilepattern("foo.txt").call();
git.commit().setMessage("add file").call();
assertEquals("Should not have entered ignored directory", 1,
resetHardAndCount(commit));
}
}
@Test
public void testIgnoredDirectoryWithTrackedContent() throws Exception {
writeTrashFile("src/ignored/sub/foo.txt", "1");
try (Git git = new Git(db)) {
git.add().addFilepattern(".").call();
git.commit().setMessage("adding foo.txt").call();
writeTrashFile(".gitignore", "src/ignored");
writeTrashFile("src/ignored/sub/foo.txt", "2");
writeTrashFile("src/ignored/other/bar.txt", "3");
git.add().addFilepattern(".").call();
RevCommit commit = git.commit().setMessage("adding .gitignore")
.call();
writeTrashFile("foo.txt", "2");
writeTrashFile("zzz.txt", "3");
git.add().addFilepattern("foo.txt").call();
git.commit().setMessage("add file").call();
File file = writeTrashFile("src/ignored/sub/foo.txt", "3");
assertEquals("Should have entered ignored directory", 3,
resetHardAndCount(commit));
checkFile(file, "2");
}
}
@Test
public void testResetWithChangeInGitignore() throws Exception {
writeTrashFile(".gitignore", "src/ignored");
writeTrashFile("src/ignored/sub/foo.txt", "1");
try (Git git = new Git(db)) {
git.add().addFilepattern(".").call();
RevCommit initial = git.commit().setMessage("initial").call();
writeTrashFile("src/newignored/foo.txt", "2");
writeTrashFile("src/.gitignore", "newignored");
git.add().addFilepattern(".").call();
RevCommit commit = git.commit().setMessage("newignored").call();
assertEquals("Should not have entered src/newignored directory", 1,
resetHardAndCount(initial));
assertEquals("Should have entered src/newignored directory", 2,
resetHardAndCount(commit));
deleteTrashFile("src/.gitignore");
git.rm().addFilepattern("src/.gitignore").call();
RevCommit top = git.commit().setMessage("Unignore newignore")
.call();
assertEquals("Should have entered src/newignored directory", 2,
resetHardAndCount(initial));
assertEquals("Should have entered src/newignored directory", 2,
resetHardAndCount(commit));
assertEquals("Should not have entered src/newignored directory", 1,
resetHardAndCount(top));
}
}
private static class TestFileTreeIterator extends FileTreeIterator {
// For assertions only
private final int[] count;
public TestFileTreeIterator(Repository repo, int[] count) {
super(repo);
this.count = count;
}
protected TestFileTreeIterator(final WorkingTreeIterator p,
final File root, FS fs, FileModeStrategy fileModeStrategy,
int[] count) {
super(p, root, fs, fileModeStrategy);
this.count = count;
}
@Override
protected AbstractTreeIterator enterSubtree() {
count[0] += 1;
return new TestFileTreeIterator(this,
((FileEntry) current()).getFile(), fs, fileModeStrategy,
count);
}
}
private int resetHardAndCount(RevCommit commit) throws Exception {
int[] callCount = { 0 };
DirCache cache = db.lockDirCache();
FileTreeIterator workingTreeIterator = new TestFileTreeIterator(db,
callCount);
try {
DirCacheCheckout checkout = new DirCacheCheckout(db, null, cache,
commit.getTree().getId(), workingTreeIterator);
checkout.setFailOnConflict(false);
checkout.checkout();
} finally {
cache.unlock();
}
return callCount[0];
}
public void assertWorkDir(Map<String, String> i) public void assertWorkDir(Map<String, String> i)
throws CorruptObjectException, throws CorruptObjectException,
IOException { IOException {

5
org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/IndexDiffTest.java

@ -470,7 +470,8 @@ public class IndexDiffTest extends RepositoryTestCase {
} }
/** /**
* Test that ignored folders aren't listed as untracked * Test that ignored folders aren't listed as untracked, but are listed as
* ignored.
* *
* @throws Exception * @throws Exception
*/ */
@ -499,6 +500,8 @@ public class IndexDiffTest extends RepositoryTestCase {
diff.diff(); diff.diff();
assertEquals(new HashSet<>(Arrays.asList("src")), assertEquals(new HashSet<>(Arrays.asList("src")),
diff.getUntrackedFolders()); diff.getUntrackedFolders());
assertEquals(new HashSet<>(Arrays.asList("sr", "target")),
diff.getIgnoredNotInIndex());
git.add().addFilepattern("src").call(); git.add().addFilepattern("src").call();
writeTrashFile("sr/com/X1.java", ""); writeTrashFile("sr/com/X1.java", "");

30
org.eclipse.jgit/src/org/eclipse/jgit/treewalk/FileTreeIterator.java

@ -52,6 +52,7 @@ import java.io.FileInputStream;
import java.io.IOException; import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
import org.eclipse.jgit.dircache.DirCacheIterator;
import org.eclipse.jgit.errors.IncorrectObjectTypeException; import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.FileMode; import org.eclipse.jgit.lib.FileMode;
@ -67,6 +68,7 @@ import org.eclipse.jgit.util.FS;
* {@link org.eclipse.jgit.treewalk.TreeWalk}. * {@link org.eclipse.jgit.treewalk.TreeWalk}.
*/ */
public class FileTreeIterator extends WorkingTreeIterator { public class FileTreeIterator extends WorkingTreeIterator {
/** /**
* the starting directory of this Iterator. All entries are located directly * the starting directory of this Iterator. All entries are located directly
* in this directory. * in this directory.
@ -207,7 +209,33 @@ public class FileTreeIterator extends WorkingTreeIterator {
@Override @Override
public AbstractTreeIterator createSubtreeIterator(ObjectReader reader) public AbstractTreeIterator createSubtreeIterator(ObjectReader reader)
throws IncorrectObjectTypeException, IOException { throws IncorrectObjectTypeException, IOException {
return new FileTreeIterator(this, ((FileEntry) current()).getFile(), fs, fileModeStrategy); if (!walksIgnoredDirectories() && isEntryIgnored()) {
DirCacheIterator iterator = getDirCacheIterator();
if (iterator == null) {
return new EmptyTreeIterator(this);
}
// Only enter if we have an associated DirCacheIterator that is
// at the same entry (which indicates there is some already
// tracked file underneath this directory). Otherwise the
// directory is indeed ignored and can be skipped entirely.
}
return enterSubtree();
}
/**
* Create a new iterator for the current entry's subtree.
* <p>
* The parent reference of the iterator must be <code>this</code>, otherwise
* the caller would not be able to exit out of the subtree iterator
* correctly and return to continue walking <code>this</code>.
*
* @return a new iterator that walks over the current subtree.
* @since 5.0
*/
protected AbstractTreeIterator enterSubtree() {
return new FileTreeIterator(this, ((FileEntry) current()).getFile(), fs,
fileModeStrategy);
} }
private Entry[] entries() { private Entry[] entries() {

44
org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java

@ -256,6 +256,45 @@ public abstract class WorkingTreeIterator extends AbstractTreeIterator {
state.dirCacheTree = treeId; state.dirCacheTree = treeId;
} }
/**
* Retrieves the {@link DirCacheIterator} at the current entry if
* {@link #setDirCacheIterator(TreeWalk, int)} was called.
*
* @return the DirCacheIterator, or {@code null} if not set or not at the
* current entry
* @since 5.0
*/
protected DirCacheIterator getDirCacheIterator() {
if (state.dirCacheTree >= 0 && state.walk != null) {
return state.walk.getTree(state.dirCacheTree,
DirCacheIterator.class);
}
return null;
}
/**
* Defines whether this {@link WorkingTreeIterator} walks ignored
* directories.
*
* @param includeIgnored
* {@code false} to skip ignored directories, if possible;
* {@code true} to always include them in the walk
* @since 5.0
*/
public void setWalkIgnoredDirectories(boolean includeIgnored) {
state.walkIgnored = includeIgnored;
}
/**
* Tells whether this {@link WorkingTreeIterator} walks ignored directories.
*
* @return {@code true} if it does, {@code false} otherwise
* @since 5.0
*/
public boolean walksIgnoredDirectories() {
return state.walkIgnored;
}
/** {@inheritDoc} */ /** {@inheritDoc} */
@Override @Override
public boolean hasId() { public boolean hasId() {
@ -1365,7 +1404,10 @@ public abstract class WorkingTreeIterator extends AbstractTreeIterator {
TreeWalk walk; TreeWalk walk;
/** Position of the matching {@link DirCacheIterator}. */ /** Position of the matching {@link DirCacheIterator}. */
int dirCacheTree; int dirCacheTree = -1;
/** Whether the iterator shall walk ignored directories. */
boolean walkIgnored = false;
final Map<String, Boolean> directoryToIgnored = new HashMap<>(); final Map<String, Boolean> directoryToIgnored = new HashMap<>();

Loading…
Cancel
Save