Fix the handling of .git/info/exclude and core.excludesFile

The RootIgnoreNode in a WorkingTreeIterator must _not_ add the rules
from .git/info/exclude or from the file designated by git config
core.excludesFile to the list of rules read from the root .gitignore.
These really must be separate nodes in a hierarchy, otherwise the
precedence rules from [1] are violated and the outcome is not the
same as in C git.

[1] https://git-scm.com/docs/gitignore

Bug: 580381
Change-Id: I57802ba7bbbe4f183504c882b6c77a78cc3a9b99
Signed-off-by: Thomas Wolf <twolf@apache.org>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/CGitIgnoreTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/CGitIgnoreTest.java
index ae3f05111f4a9a950cd8e84a144ed30d5c5b259d..083e6cd00530f17e65a2a9e4cf88e4a9836794a1 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/CGitIgnoreTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/CGitIgnoreTest.java
@@ -13,6 +13,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 
 import java.io.BufferedInputStream;
 import java.io.BufferedReader;
@@ -20,6 +21,7 @@ import java.io.ByteArrayInputStream;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStreamReader;
+import java.nio.file.Files;
 import java.util.LinkedHashSet;
 import java.util.Set;
 
@@ -354,4 +356,84 @@ public class CGitIgnoreTest extends RepositoryTestCase {
                writeTrashFile("src/.gitignore", "*\n!*.java\n!*/");
                assertSameAsCGit();
        }
+
+       @Test
+       public void testMultipleEntriesIgnored() throws Exception {
+               createFiles("dir/a");
+               writeTrashFile(".gitignore", "!dir/a\ndir/a");
+               assertSameAsCGit();
+       }
+
+       @Test
+       public void testMultipleEntriesNotIgnored() throws Exception {
+               createFiles("dir/a");
+               writeTrashFile(".gitignore", "dir/a\n!dir/a");
+               assertSameAsCGit("dir/a");
+       }
+
+       @Test
+       public void testInfoExcludes() throws Exception {
+               createFiles("dir/a", "dir/b");
+               File gitDir = db.getDirectory();
+               File info = new File(gitDir, "info");
+               assertTrue(info.mkdirs());
+               File infoExclude = new File(info, "exclude");
+               Files.writeString(infoExclude.toPath(), "dir/a");
+               assertSameAsCGit("dir/b");
+       }
+
+       @Test
+       public void testInfoExcludesPrecedence() throws Exception {
+               createFiles("dir/a", "dir/b");
+               writeTrashFile(".gitignore", "!dir/a");
+               File gitDir = db.getDirectory();
+               File info = new File(gitDir, "info");
+               assertTrue(info.mkdirs());
+               File infoExclude = new File(info, "exclude");
+               Files.writeString(infoExclude.toPath(), "dir/a");
+               assertSameAsCGit("dir/a", "dir/b");
+       }
+
+       @Test
+       public void testCoreExcludes() throws Exception {
+               createFiles("dir/a", "dir/b");
+               writeTrashFile(".fake_git_ignore", "dir/a");
+               assertSameAsCGit("dir/b");
+       }
+
+       @Test
+       public void testInfoCoreExcludes() throws Exception {
+               createFiles("dir/a", "dir/b");
+               File gitDir = db.getDirectory();
+               File info = new File(gitDir, "info");
+               assertTrue(info.mkdirs());
+               File infoExclude = new File(info, "exclude");
+               Files.writeString(infoExclude.toPath(), "!a");
+               writeTrashFile(".fake_git_ignore", "dir/a");
+               assertSameAsCGit("dir/b");
+       }
+
+       @Test
+       public void testInfoCoreExcludesPrecedence() throws Exception {
+               createFiles("dir/a", "dir/b");
+               File gitDir = db.getDirectory();
+               File info = new File(gitDir, "info");
+               assertTrue(info.mkdirs());
+               File infoExclude = new File(info, "exclude");
+               Files.writeString(infoExclude.toPath(), "!dir/a");
+               writeTrashFile(".fake_git_ignore", "dir/a");
+               assertSameAsCGit("dir/a", "dir/b");
+       }
+
+       @Test
+       public void testInfoCoreExcludesPrecedence2() throws Exception {
+               createFiles("dir/a", "dir/b");
+               File gitDir = db.getDirectory();
+               File info = new File(gitDir, "info");
+               assertTrue(info.mkdirs());
+               File infoExclude = new File(info, "exclude");
+               Files.writeString(infoExclude.toPath(), "dir/a");
+               writeTrashFile(".fake_git_ignore", "!dir/a");
+               assertSameAsCGit("dir/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 427eac5b535a3399bf76843e51058d7c7c5a5e51..b108b0a95919d2ba0dc11d266bbd04056233eae4 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java
@@ -1274,11 +1274,15 @@ public abstract class WorkingTreeIterator extends AbstractTreeIterator {
                }
 
                IgnoreNode load() throws IOException {
-                       IgnoreNode r = new IgnoreNode();
+                       return load(null);
+               }
+
+               IgnoreNode load(IgnoreNode parent) throws IOException {
+                       IgnoreNodeWithParent r = new IgnoreNodeWithParent(parent);
                        try (InputStream in = entry.openInputStream()) {
                                r.parse(name, in);
                        }
-                       return r.getRules().isEmpty() ? null : r;
+                       return r.getRules().isEmpty() && parent == null ? null : r;
                }
        }
 
@@ -1292,29 +1296,41 @@ public abstract class WorkingTreeIterator extends AbstractTreeIterator {
                }
 
                @Override
-               IgnoreNode load() throws IOException {
-                       IgnoreNode r;
-                       if (entry != null) {
-                               r = super.load();
-                               if (r == null)
-                                       r = new IgnoreNode();
-                       } else {
-                               r = new IgnoreNode();
-                       }
-
+               IgnoreNode load(IgnoreNode parent) throws IOException {
+                       IgnoreNode coreExclude = new IgnoreNodeWithParent(parent);
                        FS fs = repository.getFS();
                        Path path = repository.getConfig().getPath(
                                        ConfigConstants.CONFIG_CORE_SECTION, null,
                                        ConfigConstants.CONFIG_KEY_EXCLUDESFILE, fs, null, null);
                        if (path != null) {
-                               loadRulesFromFile(r, path.toFile());
+                               loadRulesFromFile(coreExclude, path.toFile());
+                       }
+                       if (coreExclude.getRules().isEmpty()) {
+                               coreExclude = parent;
                        }
 
+                       IgnoreNode infoExclude = new IgnoreNodeWithParent(
+                                       coreExclude);
                        File exclude = fs.resolve(repository.getDirectory(),
                                        Constants.INFO_EXCLUDE);
-                       loadRulesFromFile(r, exclude);
+                       loadRulesFromFile(infoExclude, exclude);
+                       if (infoExclude.getRules().isEmpty()) {
+                               infoExclude = null;
+                       }
 
-                       return r.getRules().isEmpty() ? null : r;
+                       IgnoreNode parentNode = infoExclude != null ? infoExclude
+                                       : coreExclude;
+
+                       IgnoreNode r;
+                       if (entry != null) {
+                               r = super.load(parentNode);
+                               if (r == null) {
+                                       return null;
+                               }
+                       } else {
+                               return parentNode;
+                       }
+                       return r.getRules().isEmpty() ? parentNode : r;
                }
 
                private static void loadRulesFromFile(IgnoreNode r, File exclude)
@@ -1327,6 +1343,24 @@ public abstract class WorkingTreeIterator extends AbstractTreeIterator {
                }
        }
 
+       private static class IgnoreNodeWithParent extends IgnoreNode {
+
+               private final IgnoreNode parent;
+
+               IgnoreNodeWithParent(IgnoreNode parent) {
+                       this.parent = parent;
+               }
+
+               @Override
+               public Boolean checkIgnored(String path, boolean isDirectory) {
+                       Boolean result = super.checkIgnored(path, isDirectory);
+                       if (result == null && parent != null) {
+                               return parent.checkIgnored(path, isDirectory);
+                       }
+                       return result;
+               }
+       }
+
        /** Magic type indicating we know rules exist, but they aren't loaded. */
        private static class PerDirectoryAttributesNode extends AttributesNode {
                final Entry entry;