Re: [PATCH 13/14] t4135-*.sh: Skip the "backslash" tests on cygwin

From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Date: Dec 21, 2010, 19:31 UTC
Message-ID: <4D1100A3.2010309@ramsay1.demon.co.uk>
In-Reply-To: <201012172254.31242.j6t@kdbg.org>

Johannes Sixt wrote:

Show 11 quoted lines

> Yes, that's clear, and this is the reason that we must skip this test on 
> MinGW.
> 
> But you said that when the single quotes are removed, the test passes (and you 
> are right). Then git-add sees this pathspec/pattern:
> 
>     fo[ou]bar
> 
> and it matches 'foobar' when it interprets that as a pattern, but 'fo[ou]bar' 
> when it interprets that as straight file name. Even on Linux, the latter 
> happens, and *that* is suspicious. What am I missing?

Ah, sorry, I mis-understood your point. :(

So, I decided to have a quick look and ... yeah, something is not quite right!

Hmm, I *think* I have a fix, see patch attached below. This patch provoked a failure of the unicode tests in t0050-filesystem.sh for me on Linux. However, these tests are borked when run by the dash shell, but work fine when run by bash. (see separate e-mail) So, all (non svn) tests pass for me when I run the tests thus:

    $ SHELL_PATH=/bin/bash make NO_SVN_TESTS=1 test

Of course, this is no guarantee that I haven't messed up all git commands that use match_one() to process pathspecs, but is at least promising.

The problem boils down to the call to strncmp_icase() suppressing the call to fnmatch() when the pattern contains glob chars, but the (remaining) string is equal to the name; thus returning an exact match (MATCHED_EXACTLY) rather than calling fnmatch (and returning either no-match or MATCHED_FNMATCH).

Note that the test itself is not correct; the argument to git-ls-files should be quoted the same as the git-add before it ... (well you could pass fo\\[ou\\]bar instead!).

[BTW, I started looking at the history of this function and I think this problem has been there for a long time!]

Hmm, I think this is all being rewritten, at the moment (in branch nd/struct-pathspec) isn't it?

Anyway, let me know what you think...

ATB, Ramsay Jones

--- 8< ---
From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Date: Sun, 19 Dec 2010 19:47:39 +0000
Subject: [PATCH] dir.c: Fix handling of filespecs containing glob-ing chars

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
 dir.c          |    2 +-
 t/t3700-add.sh |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/dir.c b/dir.c
index 852e60f..4e8c7bf 100644
--- a/dir.c
+++ b/dir.c
@@ -139,7 +139,7 @@ static int match_one(const char *match, const char *name, int namelen)
 	 * we need to match by fnmatch
 	 */
 	matchlen = strlen(match);
-	if (strncmp_icase(match, name, matchlen))
+	if (is_glob_special(*match) || strncmp_icase(match, name, matchlen))
 		return !fnmatch_icase(match, name, 0) ? MATCHED_FNMATCH : 0;
 
 	if (namelen == matchlen)
diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index ec71083..9140164 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -239,7 +239,7 @@ test_expect_success BSLASHPSPEC "git add 'fo\\[ou\\]bar' ignores foobar" '
 	git reset --hard &&
 	touch fo\[ou\]bar foobar &&
 	git add '\''fo\[ou\]bar'\'' &&
-	git ls-files fo\[ou\]bar | fgrep fo\[ou\]bar &&
+	git ls-files '\''fo\[ou\]bar'\'' | fgrep fo\[ou\]bar &&
 	! ( git ls-files foobar | grep foobar )
 '

-- 
1.7.3