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(-) 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html