Re: [JGIT PATCH v2 19/24] Added the class AddRuleListFactory.

tisdagen den 13 maj 2008 13.24.56 skrev Florian Köberle:

Shawn O. Pearce wrote:

quoted

Florian Koeberle [off-list ref] wrote:

quoted

+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.

You mean a/\*/x* does not?

quoted

quoted

+	 * 
+	 * 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.

Correctness is very important. Obviously we will slip bugs trough, but
not intentionally in a part where legacy behaviour may play an important
role. With these options available I'd select number 2.

I think your observations are interesting, especially the one you make
here about a/*/c/test.txt not matching because I don't see from the git
add manual why it shouldn't. I does not look like a recent git bug either. 

btw, I should you can use the ö in your name in mail and too. It'll make
for some interesting testing of non-ascii handling that we need to take
on (another argument for not complicating handling of names too much).
I'm sure it will get pretty hairy regardless.

-- robin