[PATCH 4/4] attr.c: respect core.ignorecase when matching attribute patterns
When core.ignorecase is true, the file globs configured in the .gitattributes file should be matched case-insensitively against the paths in the working directory.
---
Two points:
1) I think these two changes of fnmatch to fnmatch_icase should be all that is necessary. There are a number of uses of strncmp where the "origin" path of an attribute entry is compared to the prefix of a file path, but since the attribute stack is built from the same path string that it is being compared against, we shouldn't have to do strncmp_icase everywhere. The case of the two strings should necessarily match.
This needs some testing by someone on a case-insensitive filesystem.
Also, notice some of the new tests are marked with a CASE_INSENSITIVE_FS pre-requisite. I tested on a USB thumb drive, but it would be nice if someone tested on a platform that is natively case-insensitive. Maybe CASE_INSENSITIVE_FS should be moved to test-lib.sh, and t0005(others?) should be updated to use it?
2)
The bad news, this breaks t8005. The breakage is caused by
git_attr_config calling git_default_config, and stomping on the
git_log_output_encoding set by setup_revisions() when it parsed the
--encoding command line option.
What happens is cmd_blame() calls git_config() which parses the config files and sets up the global config variables like git_log_output_encoding, then later blame calls setup_revisions() which parses the command line option --encoding and overrides the value in git_log_output_encoding, then, even later, userdiff looks up an attribute on a path and calls git_check_attr() which calls git_config _again_, which resets git_log_output_encoding to the value in the config file (stomping on the value set by --encoding on the git blame command line).
Since fnmatch_icase depends on the ignore_case global variable being set correctly, the obvious thing for me to do was to allow git_default_config to call git_default_core_config and parse the core.ignorecase config option. So I modified git_attr_config so it fell back to git_default_config. But that can have the undesired side effect described above.
It's easy to work around this issue. I could just parse core.ignorecase in git_attr_config() and set ignore_case myself like:
if (!strcmp(var, "core.ignorecase")) {
ignore_case = git_config_bool(var, value);
return 0;
}The big question is whether it should be safe to call git_config() multiple times? Right now, it is not. We also don't protect against git_config() being called multiple times either.
I suspect that setup_revisions() is not the only place where a command line option overrides a global config variable and it would be a big can of worms to try to fix them all.
Thoughts?
---
attr.c | 7 +++--
t/t0003-attributes.sh | 60 ++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 63 insertions(+), 4 deletions(-)
Show changes to 2 files +63 −4
attr.c, t/t0003-attributes.sh
diff --git a/attr.c b/attr.c
index 3359b39..b8ed7cf 100644
--- a/attr.c
+++ b/attr.c
@@ -11,6 +11,7 @@
#include "cache.h"
#include "exec_cmd.h"
#include "attr.h"
+#include "dir.h"
const char git_attr__true[] = "(builtin)true";
const char git_attr__false[] = "\0(builtin)false";
@@ -499,7 +500,7 @@ static int git_attr_config(const char *var, const char *value, void *dummy)
if (!strcmp(var, "core.attributesfile"))
return git_config_pathname(&attributes_file, var, value);
- return 0;
+ return git_default_config(var, value, dummy);
}
static void bootstrap_attr_stack(void)
@@ -643,7 +644,7 @@ static int path_matches(const char *pathname, int pathlen,
/* match basename */
const char *basename = strrchr(pathname, '/');
basename = basename ? basename + 1 : pathname;
- return (fnmatch(pattern, basename, 0) == 0);
+ return (fnmatch_icase(pattern, basename, 0) == 0);
}
/*
* match with FNM_PATHNAME; the pattern has base implicitly
@@ -657,7 +658,7 @@ static int path_matches(const char *pathname, int pathlen,
return 0;
if (baselen != 0)
baselen++;
- return fnmatch(pattern, pathname + baselen, FNM_PATHNAME) == 0;
+ return fnmatch_icase(pattern, pathname + baselen, FNM_PATHNAME) == 0;
}
static int macroexpand_one(int attr_nr, int rem);
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index ae2f1da..47a70c4 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -9,7 +9,7 @@ attr_check () {
path="$1"
expect="$2"
- git check-attr test -- "$path" >actual 2>err &&
+ git $3 check-attr test -- "$path" >actual 2>err &&
echo "$path: test: $2" >expect &&
test_cmp expect actual &&
test_line_count = 0 err
@@ -27,6 +27,7 @@ test_expect_success 'setup' '
echo "onoff test -test"
echo "offon -test test"
echo "no notest"
+ echo "A/e/F test=A/e/F"
) >.gitattributes &&
(
echo "g test=a/g" &&
@@ -93,6 +94,63 @@ test_expect_success 'attribute test' '
'
+test_expect_success 'attribute matching is case sensitive when core.ignorecase=0' '
+
+ test_must_fail attr_check F f "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/F f "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/c/F f "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/G a/g "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/B/g a/b/g "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/b/G a/b/g "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/b/H a/b/h "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/b/D/g "a/b/d/*" "-c core.ignorecase=0" &&
+ test_must_fail attr_check oNoFf unset "-c core.ignorecase=0" &&
+ test_must_fail attr_check oFfOn set "-c core.ignorecase=0" &&
+ attr_check NO unspecified "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/b/D/NO "a/b/d/*" "-c core.ignorecase=0" &&
+ attr_check a/b/d/YES a/b/d/* "-c core.ignorecase=0" &&
+ test_must_fail attr_check a/E/f "A/e/F" "-c core.ignorecase=0"
+
+'
+
+test_expect_success 'attribute matching is case insensitive when core.ignorecase=1' '
+
+ attr_check F f "-c core.ignorecase=1" &&
+ attr_check a/F f "-c core.ignorecase=1" &&
+ attr_check a/c/F f "-c core.ignorecase=1" &&
+ attr_check a/G a/g "-c core.ignorecase=1" &&
+ attr_check a/B/g a/b/g "-c core.ignorecase=1" &&
+ attr_check a/b/G a/b/g "-c core.ignorecase=1" &&
+ attr_check a/b/H a/b/h "-c core.ignorecase=1" &&
+ attr_check a/b/D/g "a/b/d/*" "-c core.ignorecase=1" &&
+ attr_check oNoFf unset "-c core.ignorecase=1" &&
+ attr_check oFfOn set "-c core.ignorecase=1" &&
+ attr_check NO unspecified "-c core.ignorecase=1" &&
+ attr_check a/b/D/NO "a/b/d/*" "-c core.ignorecase=1" &&
+ attr_check a/b/d/YES unspecified "-c core.ignorecase=1" &&
+ attr_check a/E/f "A/e/F" "-c core.ignorecase=1"
+
+'
+
+test_expect_success 'check whether FS is case-insensitive' '
+ mkdir junk &&
+ echo good >junk/CamelCase &&
+ echo bad >junk/camelcase &&
+ if test "$(cat junk/CamelCase)" != good
+ then
+ test_set_prereq CASE_INSENSITIVE_FS
+ fi
+'
+
+test_expect_success CASE_INSENSITIVE_FS 'additional case insensitivity tests' '
+ test_must_fail attr_check a/B/D/g "a/b/d/*" "-c core.ignorecase=0" &&
+ test_must_fail attr_check A/B/D/NO "a/b/d/*" "-c core.ignorecase=0" &&
+ attr_check A/b/h a/b/h "-c core.ignorecase=0" &&
+ attr_check A/b/h a/b/h "-c core.ignorecase=1" &&
+ attr_check a/B/D/g "a/b/d/*" "-c core.ignorecase=1" &&
+ attr_check A/B/D/NO "a/b/d/*" "-c core.ignorecase=1"
+'
+
test_expect_success 'unnormalized paths' '
attr_check ./f f &&
--
1.7.6
Brandon Casey
