motiejus
/
jgit
1
Fork 0
Code Releases Activity

Fix processing of gitignore negations 

Processing of negated rules, like !bin/ was not working correctly: they
were interpreted too broad, resulting in unexpected untracked files
which should actually be ignored

Bug: 409664
Change-Id: I0a422fd6607941461bf2175c9105a0311612efa0
Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>
This commit is contained in:
Marc Strapetz 2018-02-23 13:34:23 +01:00
parent 152d5e2a14
commit 78420b7d0a
4 changed files with 440 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

64
org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/CGitIgnoreTest.java
View File

@ -321,4 +321,68 @@ public void testEscapedBothBracketsInGroup() throws Exception {
writeTrashFile(".gitignore", "[\\[\\]]\n");
assertSameAsCGit();
}
@Test
public void testSimpleRootGitIgnoreGlobalNegation1() throws Exception {
// see IgnoreNodeTest.testSimpleRootGitIgnoreGlobalNegation1
createFiles("x1", "a/x2", "x3/y");
writeTrashFile(".gitignore", "*\n!x*");
assertSameAsCGit();
}
@Test
public void testRepeatedNegationInDifferentFiles5() throws Exception {
// see IgnoreNodeTest.testRepeatedNegationInDifferentFiles5
createFiles("a/b/e/nothere.o");
writeTrashFile(".gitignore", "e");
writeTrashFile("a/.gitignore", "e");
writeTrashFile("a/b/.gitignore", "!e");
assertSameAsCGit();
}
@Test
public void testRepeatedNegationInDifferentFilesWithWildmatcher1()
throws Exception {
createFiles("e", "x/e/f", "a/e/x1", "a/e/x2", "a/e/y", "a/e/sub/y");
writeTrashFile(".gitignore", "a/e/**");
writeTrashFile("a/.gitignore", "!e/x*");
assertSameAsCGit();
}
@Test
public void testRepeatedNegationInDifferentFilesWithWildmatcher2()
throws Exception {
createFiles("e", "dir/f", "dir/g/h", "a/dir/i", "a/dir/j/k",
"a/b/dir/l", "a/b/dir/m/n", "a/b/dir/m/o/p", "a/q/dir/r",
"a/q/dir/dir/s", "c/d/dir/x", "c/d/dir/y");
writeTrashFile(".gitignore", "**/dir/*");
writeTrashFile("a/.gitignore", "!dir/*");
writeTrashFile("a/b/.gitignore", "!**/dir/*");
writeTrashFile("c/.gitignore", "!d/dir/x");
assertSameAsCGit();
}
@Test
public void testNegationForSubDirectoryWithinIgnoredDirectoryHasNoEffect1()
throws Exception {
createFiles("e", "a/f", "a/b/g", "a/b/h/i");
writeTrashFile(".gitignore", "a/b");
writeTrashFile("a/.gitignore", "!b/*");
assertSameAsCGit();
}
/*
* See https://bugs.eclipse.org/bugs/show_bug.cgi?id=407475
*/
@Test
public void testNegationAllExceptJavaInSrcAndExceptChildDirInSrc()
throws Exception {
// see
// IgnoreNodeTest.testNegationAllExceptJavaInSrcAndExceptChildDirInSrc
createFiles("nothere.o", "src/keep.java", "src/nothere.o",
"src/a/keep.java", "src/a/keep.o");
writeTrashFile(".gitignore", "/*\n!/src/");
writeTrashFile("src/.gitignore", "*\n!*.java\n!*/");
assertSameAsCGit();
}
}

233
org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreNodeTest.java
View File

@ -80,6 +80,141 @@ public class IgnoreNodeTest extends RepositoryTestCase {
private TreeWalk walk;
@Test
public void testSimpleRootGitIgnoreGlobalIgnore() throws IOException {
writeIgnoreFile(".gitignore", "x");
writeTrashFile("a/x/file", "");
writeTrashFile("b/x", "");
writeTrashFile("x/file", "");
beginWalk();
assertEntry(F, tracked, ".gitignore");
assertEntry(D, tracked, "a");
assertEntry(D, ignored, "a/x");
assertEntry(F, ignored, "a/x/file");
assertEntry(D, tracked, "b");
assertEntry(F, ignored, "b/x");
assertEntry(D, ignored, "x");
assertEntry(F, ignored, "x/file");
endWalk();
}
@Test
public void testSimpleRootGitIgnoreGlobalDirIgnore() throws IOException {
writeIgnoreFile(".gitignore", "x/");
writeTrashFile("a/x/file", "");
writeTrashFile("x/file", "");
beginWalk();
assertEntry(F, tracked, ".gitignore");
assertEntry(D, tracked, "a");
assertEntry(D, ignored, "a/x");
assertEntry(F, ignored, "a/x/file");
assertEntry(D, ignored, "x");
assertEntry(F, ignored, "x/file");
endWalk();
}
@Test
public void testSimpleRootGitIgnoreWildMatcher() throws IOException {
writeIgnoreFile(".gitignore", "**");
writeTrashFile("a/x", "");
writeTrashFile("y", "");
beginWalk();
assertEntry(F, ignored, ".gitignore");
assertEntry(D, ignored, "a");
assertEntry(F, ignored, "a/x");
assertEntry(F, ignored, "y");
endWalk();
}
@Test
public void testSimpleRootGitIgnoreWildMatcherDirOnly() throws IOException {
writeIgnoreFile(".gitignore", "**/");
writeTrashFile("a/x", "");
writeTrashFile("y", "");
beginWalk();
assertEntry(F, tracked, ".gitignore");
assertEntry(D, ignored, "a");
assertEntry(F, ignored, "a/x");
assertEntry(F, tracked, "y");
endWalk();
}
@Test
public void testSimpleRootGitIgnoreGlobalNegation1() throws IOException {
writeIgnoreFile(".gitignore", "*", "!x*");
writeTrashFile("x1", "");
writeTrashFile("a/x2", "");
writeTrashFile("x3/y", "");
beginWalk();
assertEntry(F, ignored, ".gitignore");
assertEntry(D, ignored, "a");
assertEntry(F, ignored, "a/x2");
assertEntry(F, tracked, "x1");
assertEntry(D, tracked, "x3");
assertEntry(F, ignored, "x3/y");
endWalk();
}
@Test
public void testSimpleRootGitIgnoreGlobalNegation2() throws IOException {
writeIgnoreFile(".gitignore", "*", "!x*", "!/a");
writeTrashFile("x1", "");
writeTrashFile("a/x2", "");
writeTrashFile("x3/y", "");
beginWalk();
assertEntry(F, ignored, ".gitignore");
assertEntry(D, tracked, "a");
assertEntry(F, tracked, "a/x2");
assertEntry(F, tracked, "x1");
assertEntry(D, tracked, "x3");
assertEntry(F, ignored, "x3/y");
endWalk();
}
@Test
public void testSimpleRootGitIgnoreGlobalNegation3() throws IOException {
writeIgnoreFile(".gitignore", "*", "!x*", "!x*/**");
writeTrashFile("x1", "");
writeTrashFile("a/x2", "");
writeTrashFile("x3/y", "");
beginWalk();
assertEntry(F, ignored, ".gitignore");
assertEntry(D, ignored, "a");
assertEntry(F, ignored, "a/x2");
assertEntry(F, tracked, "x1");
assertEntry(D, tracked, "x3");
assertEntry(F, tracked, "x3/y");
endWalk();
}
@Test
public void testSimpleRootGitIgnoreGlobalNegation4() throws IOException {
writeIgnoreFile(".gitignore", "*", "!**/");
writeTrashFile("x1", "");
writeTrashFile("a/x2", "");
writeTrashFile("x3/y", "");
beginWalk();
assertEntry(F, ignored, ".gitignore");
assertEntry(D, tracked, "a");
assertEntry(F, ignored, "a/x2");
assertEntry(F, ignored, "x1");
assertEntry(D, tracked, "x3");
assertEntry(F, ignored, "x3/y");
endWalk();
}
@Test
public void testRules() throws IOException {
writeIgnoreFile(".git/info/exclude", "*~", "/out");
@ -210,7 +345,7 @@ public void testNegationAllExceptJavaInSrcAndExceptChildDirInSrc()
assertEntry(F, ignored, "src/.gitignore");
assertEntry(D, tracked, "src/a");
assertEntry(F, tracked, "src/a/keep.java");
assertEntry(F, tracked, "src/a/keep.o");
assertEntry(F, ignored, "src/a/keep.o");
assertEntry(F, tracked, "src/keep.java");
assertEntry(F, ignored, "src/nothere.o");
endWalk();
@ -315,6 +450,102 @@ public void testRepeatedNegationInDifferentFiles4() throws IOException {
endWalk();
}
@Test
public void testRepeatedNegationInDifferentFiles5() throws IOException {
writeIgnoreFile(".gitignore", "e");
writeIgnoreFile("a/.gitignore", "e");
writeIgnoreFile("a/b/.gitignore", "!e");
writeTrashFile("a/b/e/nothere.o", "");
beginWalk();
assertEntry(F, tracked, ".gitignore");
assertEntry(D, tracked, "a");
assertEntry(F, tracked, "a/.gitignore");
assertEntry(D, tracked, "a/b");
assertEntry(F, tracked, "a/b/.gitignore");
assertEntry(D, tracked, "a/b/e");
assertEntry(F, tracked, "a/b/e/nothere.o");
endWalk();
}
@Test
public void testIneffectiveNegationDifferentLevels1() throws IOException {
writeIgnoreFile(".gitignore", "a/b/e/", "!a/b/e/*");
writeTrashFile("a/b/e/nothere.o", "");
beginWalk();
assertEntry(F, tracked, ".gitignore");
assertEntry(D, tracked, "a");
assertEntry(D, tracked, "a/b");
assertEntry(D, ignored, "a/b/e");
assertEntry(F, ignored, "a/b/e/nothere.o");
endWalk();
}
@Test
public void testIneffectiveNegationDifferentLevels2() throws IOException {
writeIgnoreFile(".gitignore", "a/b/e/");
writeIgnoreFile("a/.gitignore", "!b/e/*");
writeTrashFile("a/b/e/nothere.o", "");
beginWalk();
assertEntry(F, tracked, ".gitignore");
assertEntry(D, tracked, "a");
assertEntry(F, tracked, "a/.gitignore");
assertEntry(D, tracked, "a/b");
assertEntry(D, ignored, "a/b/e");
assertEntry(F, ignored, "a/b/e/nothere.o");
endWalk();
}
@Test
public void testIneffectiveNegationDifferentLevels3() throws IOException {
writeIgnoreFile(".gitignore", "a/b/e/");
writeIgnoreFile("a/b/.gitignore", "!e/*");
writeTrashFile("a/b/e/nothere.o", "");
beginWalk();
assertEntry(F, tracked, ".gitignore");
assertEntry(D, tracked, "a");
assertEntry(D, tracked, "a/b");
assertEntry(F, tracked, "a/b/.gitignore");
assertEntry(D, ignored, "a/b/e");
assertEntry(F, ignored, "a/b/e/nothere.o");
endWalk();
}
@Test
public void testIneffectiveNegationDifferentLevels4() throws IOException {
writeIgnoreFile(".gitignore", "a/b/e/");
writeIgnoreFile("a/b/e/.gitignore", "!*");
writeTrashFile("a/b/e/nothere.o", "");
beginWalk();
assertEntry(F, tracked, ".gitignore");
assertEntry(D, tracked, "a");
assertEntry(D, tracked, "a/b");
assertEntry(D, ignored, "a/b/e");
assertEntry(F, ignored, "a/b/e/.gitignore");
assertEntry(F, ignored, "a/b/e/nothere.o");
endWalk();
}
@Test
public void testIneffectiveNegationDifferentLevels5() throws IOException {
writeIgnoreFile("a/.gitignore", "b/e/");
writeIgnoreFile("a/b/.gitignore", "!e/*");
writeTrashFile("a/b/e/nothere.o", "");
beginWalk();
assertEntry(D, tracked, "a");
assertEntry(F, tracked, "a/.gitignore");
assertEntry(D, tracked, "a/b");
assertEntry(F, tracked, "a/b/.gitignore");
assertEntry(D, ignored, "a/b/e");
assertEntry(F, ignored, "a/b/e/nothere.o");
endWalk();
}
@Test
public void testEmptyIgnoreNode() {
// Rules are never empty: WorkingTreeIterator optimizes empty files away

67
org.eclipse.jgit/src/org/eclipse/jgit/ignore/IgnoreNode.java
View File

@ -145,7 +145,12 @@ public List<FastIgnoreRule> getRules() {
* @return status of the path.
*/
public MatchResult isIgnored(String entryPath, boolean isDirectory) {
return isIgnored(entryPath, isDirectory, false);
final Boolean result = checkIgnored(entryPath, isDirectory);
if (result == null) {
return MatchResult.CHECK_PARENT;
}
return result ? MatchResult.IGNORED : MatchResult.NOT_IGNORED;
}
/**
@ -159,45 +164,47 @@ public MatchResult isIgnored(String entryPath, boolean isDirectory) {
* true if the target item is a directory.
* @param negateFirstMatch
* true if the first match should be negated
* @deprecated negateFirstMatch is not honored anymore
* @return status of the path.
* @since 3.6
*/
@Deprecated
public MatchResult isIgnored(String entryPath, boolean isDirectory,
boolean negateFirstMatch) {
if (rules.isEmpty())
if (negateFirstMatch)
return MatchResult.CHECK_PARENT_NEGATE_FIRST_MATCH;
else
return MatchResult.CHECK_PARENT;
final Boolean result = checkIgnored(entryPath, isDirectory);
if (result == null) {
return negateFirstMatch
? MatchResult.CHECK_PARENT_NEGATE_FIRST_MATCH
: MatchResult.CHECK_PARENT;
}
// Parse rules in the reverse order that they were read
return result ? MatchResult.IGNORED : MatchResult.NOT_IGNORED;
}
/**
* Determine if an entry path matches an ignore rule.
*
* @param entryPath
* the path to test. The path must be relative to this ignore
* node's own repository path, and in repository path format
* (uses '/' and not '\').
* @param isDirectory
* true if the target item is a directory.
* @return Boolean.TRUE, if the entry is ignored; Boolean.FALSE, if the
* entry is forced to be not ignored (negated match); or null, if
* undetermined
* @since 4.11
*/
public Boolean checkIgnored(String entryPath, boolean isDirectory) {
// Parse rules in the reverse order that they were read because later
// rules have higher priority
for (int i = rules.size() - 1; i > -1; i--) {
FastIgnoreRule rule = rules.get(i);
if (rule.isMatch(entryPath, isDirectory)) {
if (rule.getResult()) {
// rule matches: path could be ignored
if (negateFirstMatch)
// ignore current match, reset "negate" flag, continue
negateFirstMatch = false;
else
// valid match, just return
return MatchResult.IGNORED;
} else {
// found negated rule
if (negateFirstMatch)
// not possible to re-include excluded ignore rule
return MatchResult.NOT_IGNORED;
else
// set the flag and continue
negateFirstMatch = true;
}
if (rule.isMatch(entryPath, isDirectory, true)) {
return Boolean.valueOf(rule.getResult());
}
}
if (negateFirstMatch)
// negated rule found but there is no previous rule in *this* file
return MatchResult.CHECK_PARENT_NEGATE_FIRST_MATCH;
// *this* file has no matching rules
return MatchResult.CHECK_PARENT;
return null;
}
/** {@inheritDoc} */

141
org.eclipse.jgit/src/org/eclipse/jgit/treewalk/WorkingTreeIterator.java
View File

@ -60,6 +60,8 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.Map;
import org.eclipse.jgit.api.errors.FilterFailedException;
import org.eclipse.jgit.attributes.AttributesNode;
@ -661,54 +663,60 @@ public boolean isEntryIgnored() throws IOException {
* a relevant ignore rule file exists but cannot be read.
*/
protected boolean isEntryIgnored(final int pLen) throws IOException {
return isEntryIgnored(pLen, mode, false);
return isEntryIgnored(pLen, mode);
}
/**
* Determine if the entry path is ignored by an ignore rule. Consider
* possible rule negation from child iterator.
* Determine if the entry path is ignored by an ignore rule.
*
* @param pLen
* the length of the path in the path buffer.
* @param fileMode
* the original iterator file mode
* @param negatePrevious
* true if the previous matching iterator rule was negation
* @return true if the entry is ignored by an ignore rule.
* @throws IOException
* a relevant ignore rule file exists but cannot be read.
*/
private boolean isEntryIgnored(final int pLen, int fileMode,
boolean negatePrevious)
private boolean isEntryIgnored(final int pLen, int fileMode)
throws IOException {
IgnoreNode rules = getIgnoreNode();
if (rules != null) {
// The ignore code wants path to start with a '/' if possible.
// If we have the '/' in our path buffer because we are inside
// a subdirectory include it in the range we convert to string.
//
int pOff = pathOffset;
if (0 < pOff)
pOff--;
String p = TreeWalk.pathOf(path, pOff, pLen);
switch (rules.isIgnored(p, FileMode.TREE.equals(fileMode),
negatePrevious)) {
case IGNORED:
return true;
case NOT_IGNORED:
return false;
case CHECK_PARENT:
negatePrevious = false;
break;
case CHECK_PARENT_NEGATE_FIRST_MATCH:
negatePrevious = true;
break;
}
// The ignore code wants path to start with a '/' if possible.
// If we have the '/' in our path buffer because we are inside
// a sub-directory include it in the range we convert to string.
//
final int pOff = 0 < pathOffset ? pathOffset - 1 : pathOffset;
String pathRel = TreeWalk.pathOf(this.path, pOff, pLen);
String parentRel = getParentPath(pathRel);
// CGit is processing .gitignore files by starting at the root of the
// repository and then recursing into subdirectories. With this
// approach, top-level ignored directories will be processed first which
// allows to skip entire subtrees and further .gitignore-file processing
// within these subtrees.
//
// We will follow the same approach by marking directories as "ignored"
// here. This allows to have a simplified FastIgnore.checkIgnore()
// implementation (both in terms of code and computational complexity):
//
// Without the "ignored" flag, we would have to apply the ignore-check
// to a path and all of its parents always(!), to determine whether a
// path is ignored directly or by one of its parent directories; with
// the "ignored" flag, we know at this point that the parent directory
// is definitely not ignored, thus the path can only become ignored if
// there is a rule matching the path itself.
if (isDirectoryIgnored(parentRel)) {
return true;
}
if (parent instanceof WorkingTreeIterator)
return ((WorkingTreeIterator) parent).isEntryIgnored(pLen, fileMode,
negatePrevious);
return false;
IgnoreNode rules = getIgnoreNode();
final Boolean ignored = rules != null
? rules.checkIgnored(pathRel, FileMode.TREE.equals(fileMode))
: null;
if (ignored != null) {
return ignored.booleanValue();
}
return parent instanceof WorkingTreeIterator
&& ((WorkingTreeIterator) parent).isEntryIgnored(pLen,
fileMode);
}
private IgnoreNode getIgnoreNode() throws IOException {
@ -1372,6 +1380,8 @@ private static final class IteratorState {
/** Position of the matching {@link DirCacheIterator}. */
int dirCacheTree;
final Map<String, Boolean> directoryToIgnored = new HashMap<>();
IteratorState(WorkingTreeOptions options) {
this.options = options;
this.nameEncoder = Constants.CHARSET.newEncoder();
@ -1448,4 +1458,67 @@ private EolStreamType getEolStreamType(OperationType opType)
}
return eolStreamTypeHolder.get();
}
private boolean isDirectoryIgnored(String pathRel) throws IOException {
final int pOff = 0 < pathOffset ? pathOffset - 1 : pathOffset;
final String base = TreeWalk.pathOf(this.path, 0, pOff);
final String pathAbs = concatPath(base, pathRel);
return isDirectoryIgnored(pathRel, pathAbs);
}
private boolean isDirectoryIgnored(String pathRel, String pathAbs)
throws IOException {
assert pathRel.length() == 0 || (pathRel.charAt(0) != '/'
&& pathRel.charAt(pathRel.length() - 1) != '/');
assert pathAbs.length() == 0 || (pathAbs.charAt(0) != '/'
&& pathAbs.charAt(pathAbs.length() - 1) != '/');
assert pathAbs.endsWith(pathRel);
Boolean ignored = state.directoryToIgnored.get(pathAbs);
if (ignored != null) {
return ignored;
}
final String parentRel = getParentPath(pathRel);
if (parentRel != null && isDirectoryIgnored(parentRel)) {
state.directoryToIgnored.put(pathAbs, Boolean.TRUE);
return true;
}
final IgnoreNode ignoreNode = getIgnoreNode();
for (String path = pathRel; ignoreNode != null
&& !"".equals(path); path = getParentPath(path)) {
ignored = ignoreNode.checkIgnored(path, true);
if (ignored != null) {
state.directoryToIgnored.put(pathAbs, ignored);
return ignored;
}
}
if (!(this.parent instanceof WorkingTreeIterator)) {
state.directoryToIgnored.put(pathAbs, Boolean.FALSE);
return false;
}
final WorkingTreeIterator wtParent = (WorkingTreeIterator) this.parent;
final String parentRelPath = concatPath(
TreeWalk.pathOf(this.path, wtParent.pathOffset, pathOffset - 1),
pathRel);
assert concatPath(TreeWalk.pathOf(wtParent.path, 0,
Math.max(0, wtParent.pathOffset - 1)), parentRelPath)
.equals(pathAbs);
return wtParent.isDirectoryIgnored(parentRelPath, pathAbs);
}
private static String getParentPath(String path) {
final int slashIndex = path.lastIndexOf('/', path.length() - 2);
if (slashIndex > 0) {
return path.substring(path.charAt(0) == '/' ? 1 : 0, slashIndex);
}
return path.length() > 0 ? "" : null;
}
private static String concatPath(String p1, String p2) {
return p1 + (p1.length() > 0 && p2.length() > 0 ? "/" : "") + p2;
}
}