From a1ce9063fbcafbc306f40f6d384d48726de09da4 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Fri, 15 Jul 2022 20:39:19 +0200 Subject: [PATCH] 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 --- .../eclipse/jgit/ignore/CGitIgnoreTest.java | 82 +++++++++++++++++++ .../jgit/treewalk/WorkingTreeIterator.java | 64 +++++++++++---- 2 files changed, 131 insertions(+), 15 deletions(-) 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 ae3f05111..083e6cd00 100644 --- 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 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.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 void testNegationAllExceptJavaInSrcAndExceptChildDirInSrc() 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 427eac5b5..b108b0a95 100644 --- 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 @@ private static class PerDirectoryIgnoreNode extends IgnoreNode { } 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 @@ private static class RootIgnoreNode extends PerDirectoryIgnoreNode { } @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 @@ private static void loadRulesFromFile(IgnoreNode r, File exclude) } } + 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;