DiffFormatter: correctly deal with tracked files in ignored folders

In JGit 5.0, the FileTreeIterator was changed to skip ignored folders
by default. To catch tracked files inside ignored folders, the tree
walk needs to have a DirCacheIterator, and the FileTreeIterator has
to know about that DirCacheIterator via setDirCacheIterator(). (Or
the optimization has to be switched off explicitly via
setWalkIgnoredDirectories(true).)

Skipping ignored directories is an important optimization in some
cases, for instance in node.js/npm projects, where we'd otherwise
traverse the whole huge and deep hierarchy of the typically ignored
node_modules folder.

While all uses of WorkingTreeIterator in JGit had been adapted,
DiffFormatter was forgotten. To make it work correctly (again) also
for such cases, make it set up a WorkingTreeeIterator automatically,
and make sure the WorkingTreeSource can find such files, too. Also
pass the repository to the TreeWalks used inside the DiffFormatter
to pick up the correct attributes, filters, and line-ending settings.

Bug: 565081
Change-Id: Ie88ac81166dc396ba28b83313964c1712b6ca199
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffFormatterTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffFormatterTest.java
index a929def2ffeafc63d1f204f0825652157f93772d..62824d3aecbe45e312d2dc74c9db05e3ae8ed03f 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffFormatterTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffFormatterTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010, 2013 Google Inc. and others
+ * Copyright (C) 2010, 2020 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
@@ -13,12 +13,14 @@ package org.eclipse.jgit.diff;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 
 import java.io.BufferedOutputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 
 import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.api.Status;
 import org.eclipse.jgit.diff.DiffEntry.ChangeType;
 import org.eclipse.jgit.dircache.DirCacheIterator;
 import org.eclipse.jgit.junit.RepositoryTestCase;
@@ -467,6 +469,58 @@ public class DiffFormatterTest extends RepositoryTestCase {
                }
        }
 
+       @Test
+       public void testTrackedFileInIgnoredFolderUnchanged()
+                       throws Exception {
+               commitFile("empty/empty/foo", "", "master");
+               commitFile(".gitignore", "empty/*", "master");
+               try (Git git = new Git(db)) {
+                       Status status = git.status().call();
+                       assertTrue(status.isClean());
+               }
+               try (ByteArrayOutputStream os = new ByteArrayOutputStream();
+                               DiffFormatter dfmt = new DiffFormatter(os)) {
+                       dfmt.setRepository(db);
+                       dfmt.format(new DirCacheIterator(db.readDirCache()),
+                                       new FileTreeIterator(db));
+                       dfmt.flush();
+
+                       String actual = os.toString("UTF-8");
+
+                       assertEquals("", actual);
+               }
+       }
+
+       @Test
+       public void testTrackedFileInIgnoredFolderChanged()
+                       throws Exception {
+               String expectedDiff = "diff --git a/empty/empty/foo b/empty/empty/foo\n"
+                               + "index e69de29..5ea2ed4 100644\n" //
+                               + "--- a/empty/empty/foo\n" //
+                               + "+++ b/empty/empty/foo\n" //
+                               + "@@ -0,0 +1 @@\n" //
+                               + "+changed\n";
+
+               commitFile("empty/empty/foo", "", "master");
+               commitFile(".gitignore", "empty/*", "master");
+               try (Git git = new Git(db)) {
+                       Status status = git.status().call();
+                       assertTrue(status.isClean());
+               }
+               try (ByteArrayOutputStream os = new ByteArrayOutputStream();
+                               DiffFormatter dfmt = new DiffFormatter(os)) {
+                       writeTrashFile("empty/empty/foo", "changed\n");
+                       dfmt.setRepository(db);
+                       dfmt.format(new DirCacheIterator(db.readDirCache()),
+                                       new FileTreeIterator(db));
+                       dfmt.flush();
+
+                       String actual = os.toString("UTF-8");
+
+                       assertEquals(expectedDiff, actual);
+               }
+       }
+
        @Test
        public void testDiffAutoCrlfSmallFile() throws Exception {
                String content = "01234\r\n01234\r\n01234\r\n";

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/diff/ContentSource.java b/org.eclipse.jgit/src/org/eclipse/jgit/diff/ContentSource.java
index 7ae005ada8513c0345ae1fc3c773131e6fada8d8..1a41df3d0a9c53648db3c0e1a8d64379046e80d6 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/diff/ContentSource.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/diff/ContentSource.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010, Google Inc. and others
+ * Copyright (C) 2010, 2020 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
@@ -123,7 +123,8 @@ public abstract class ContentSource {
                WorkingTreeIterator ptr;
 
                WorkingTreeSource(WorkingTreeIterator iterator) {
-                       this.tw = new TreeWalk((ObjectReader) null);
+                       this.tw = new TreeWalk(iterator.getRepository(),
+                                       (ObjectReader) null);
                        this.tw.setRecursive(true);
                        this.iterator = iterator;
                }
@@ -173,6 +174,15 @@ public abstract class ContentSource {
                private void seek(String path) throws IOException {
                        if (!path.equals(current)) {
                                iterator.reset();
+                               // Possibly this iterator had an associated DirCacheIterator,
+                               // but we have no access to it and thus don't know about it.
+                               // We have to reset this iterator here to work without
+                               // DirCacheIterator and to descend always into ignored
+                               // directories. Otherwise we might not find tracked files below
+                               // ignored folders. Since we're looking only for a single
+                               // specific path this is not a performance problem.
+                               iterator.setWalkIgnoredDirectories(true);
+                               iterator.setDirCacheIterator(null, -1);
                                tw.reset();
                                tw.addTree(iterator);
                                tw.setFilter(PathFilter.create(path));

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java b/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java
index 81367eaa08dd8725f1a7d32f6fbf633bcf7a8705..ec21954aa2e29f00378e73d6a7ae754a14790ab6 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2009, Google Inc.
- * Copyright (C) 2008-2009, Johannes E. Schindelin <johannes.schindelin@gmx.de> and others
+ * Copyright (C) 2008-2020, Johannes E. Schindelin <johannes.schindelin@gmx.de> 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
@@ -502,9 +502,18 @@ public class DiffFormatter implements AutoCloseable {
                        throws IOException {
                assertHaveReader();
 
-               TreeWalk walk = new TreeWalk(reader);
-               walk.addTree(a);
-               walk.addTree(b);
+               TreeWalk walk = new TreeWalk(repository, reader);
+               int aIndex = walk.addTree(a);
+               int bIndex = walk.addTree(b);
+               if (repository != null) {
+                       if (a instanceof WorkingTreeIterator
+                                       && b instanceof DirCacheIterator) {
+                               ((WorkingTreeIterator) a).setDirCacheIterator(walk, bIndex);
+                       } else if (b instanceof WorkingTreeIterator
+                                       && a instanceof DirCacheIterator) {
+                               ((WorkingTreeIterator) b).setDirCacheIterator(walk, aIndex);
+                       }
+               }
                walk.setRecursive(true);
 
                TreeFilter filter = getDiffTreeFilterFor(a, b);

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java
index 994af2607c2b7cfbf0a28dac3aeaec785bebb8ae..4c26dd0c4038f96407c4d039697337111d82458a 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java
@@ -2,7 +2,7 @@
  * Copyright (C) 2008, Shawn O. Pearce <spearce@spearce.org>
  * Copyright (C) 2010, Christian Halstrick <christian.halstrick@sap.com>
  * Copyright (C) 2010, Matthias Sohn <matthias.sohn@sap.com>
- * Copyright (C) 2012-2013, Robin Rosenberg and others
+ * Copyright (C) 2012-2020, Robin Rosenberg 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
@@ -522,6 +522,17 @@ public abstract class WorkingTreeIterator extends AbstractTreeIterator {
                return state.options;
        }
 
+       /**
+        * Retrieves the {@link Repository} this {@link WorkingTreeIterator}
+        * operates on.
+        *
+        * @return the {@link Repository}
+        * @since 5.9
+        */
+       public Repository getRepository() {
+               return repository;
+       }
+
        /** {@inheritDoc} */
        @Override
        public int idOffset() {