[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
[
List Home]
|
Re: [jgit-dev] IgnoreNode.isIgnored() (Was: [Announce] JGit / EGit Release 4.11.0.201803080745-r)
|
On 12.03.2018 10:12, Thomas Wolf wrote:
On Mar 10, 2018, at 18:18, Matthias Sohn <matthias.sohn@xxxxxxxxx> wrote:
On Sat, Mar 10, 2018 at 4:04 PM, Rüdiger Herrmann <ruediger.herrmann@xxxxxx> wrote:
Dear JGit devs,
the following test used to work with JGit 4.10 and earlier. After updating to version 4.11, the test fails:
@Test
public void testIsIgnored() throws IOException {
IgnoreNode ignoreNode = new IgnoreNode();
ignoreNode.parse( new ByteArrayInputStream( "bin/".getBytes( StandardCharsets.UTF_8 ) ) );
MatchResult matchResult = ignoreNode.isIgnored( "bin/Foo.class", false );
assertEquals( MatchResult.IGNORED, matchResult );
}
Is this a regression or am I doing something wrong?
bisecting between 4.10 and 4.11 yields the following commit causing this test to fail:
https://git.eclipse.org/r/#/c/117186/
commit 78420b7d0a65d591d00f32675efb0a42cda6c84a
Author: Marc Strapetz <marc.strapetz@xxxxxxxxxxx>
Date: Fri Feb 23 13:34:23 2018 +0100
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@xxxxxxxxxxx>
IMO the test is flawed. The git ignore pattern “bin/“ definitely doesn’t match the file “bin/Foo.class”.
Exclusion processing in git (and in JGit) is a recursive directory traversal: git and JGit will thus first
visit the directory “bin”, which _will_ match, and then will prune that whole subtree.
Correct exclusion information is thus available only during a TreeWalk (same for gitattributes). Simply
creating an IgnoreNode and then throwing an arbitrary path at it may give different results, as in this
case. Note that with a file pattern like “bin/*” or “bin/**” the test might work. But certainly not with a
directory pattern.
Yes. Before release 4.11, FastIgnoreRule's 'non-pathMatch' mode was used
which was causing troubles: while it may have been convenient to have
"bin/" ignoring the entire directory tree already in low-level code,
this was exactly the problem for exclude-patterns and hence I was not
able to maintain this behavior. Also, I couldn't find a counterpart in
Git code.
Now, all non-test code except of DescribeCommand is using only
pathMatch=true. For DescribeCommand, this behavior doesn't seem to be
relevant, though, at least it should be easy to change/rewrite. Hence,
for release 5, it makes sense to simplify the API, including PathMatcher
itself and get rid of pathMatch-parameter completely.
Maybe that whole IgnoreNode.isIgnored() method should be removed — it isn’t called in JGit or EGit.
This could be part of the cleanup.
-Marc
