From: Florian Köberle <florianskarten@web.de>
Date: Tue, 13 May 2008 11:24:56 GMT
Subject: Re: [JGIT PATCH v2 19/24] Added the class AddRuleListFactory.
Message-ID: <48297A88.5040508@web.de>
In-Reply-To: <20080513012909.GG29038@spearce.org>

Shawn O. Pearce wrote:
> Florian Koeberle <florianskarten@web.de> wrote:
>> +class AddRuleListFactory {
>> +	/*
>> +	 * The add command of git 1.5.2.5 behaves a little bit stange: "git add
>> +	 * a/\*z" adds the file "a/b/xyz" but "git add a/x\*" does not.
>> +	 * 
>> +	 * The first is parsed as pattern "*z" for whole directory tree "a". The
>> +	 * second is parsed as an path.
>> +	 * 
>> +	 */
> 
> Its not strange.  C Git expands each file path to its _full_ path
> and stores that into a buffer, then runs fnmatch() for each pattern
> on the buffer.  If fnmatch() succeeds the path is added to the index.
> 
> In the case above we are running a match of "a/\*.z" against
> "a/b/xyz" and that passes.  Or we run "a/x\*" on "a/b/xyz" and it
> fails as the sequence of characters "a/x" does not appear in the
> string "a/b".
> 
> You are running into this odd corner case because you are not
> treating the pattern passed as something that matches against the
> entire path.  This is one reason why TreeFilter's use the entire
> path when they process an entry for inclusion or exclusion, and why
> TreeWalk has each AbstractTreeIterator append the current entry name
> onto the end of the current path buffer, so we can always examine
> the full path from the root of the repository/working directory.
> 
> Trying to avoid the full path in classes like ComplexFilePattern
> is why you are running into this corner case here, and must now do
> extra contortions to somewhat match the behavior of C Git.
> 
> At this point I think most of the rules package is overcomplicated
> and overoptimized, and yet doesn't actually quite match the behavior
> of C Git.
> 

Still I think that the behavior of git-add is strange:
For example, if you want to add the following file:
a/b/c/test.txt
Then I can do this with "a/\*.txt" or "a/b\*.txt" but not with 
"a/\*/c/test.txt"

I know that I handle "a/b\*.txt" wrong, and I don't know a nice way to 
implement it in the current rules framework.

I see three options:
1.) Let the jgit add command work in another way then git-add does.
2.) Don't use the rules framework to determine if a file is selected by 
the add command.
3.) Completely drop the patches
[4.) Add some evil hacks to make it working] <- I don't like that version

Please tell me which way to go, so that I don't waste even more time on 
patches which will never make it in.

Best regards,
Florian