{"thread":{"id":"28599","subject":"[PATCH v2 0/2] reroll bc/attr-ignore-case (top two commits)","startedAt":"2011-10-06T04:00:12Z","lastAt":"2011-10-06T04:00:14Z","messageCount":3,"participants":["Brandon Casey"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"176999","messageId":"1317873614-3057-1-git-send-email-drafnel@gmail.com","threadId":"28599","inReplyTo":null,"subject":"[PATCH v2 0/2] reroll bc/attr-ignore-case (top two commits)","fromName":"Brandon Casey","fromEmail":"drafnel@gmail.com","sentAt":"2011-10-06T04:00:12Z","receivedAt":"2011-10-06T04:00:12Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"This is the re-roll of this series that I said I would do.\n\nThis just swaps the order of the top two patches around and *hopefully*\nimproves the commit messages.\n\nMy original \"attr.c: respect core.ignorecase..\" patch was broken and I\nintended to mark it as WIP.  Your patch fixes it, but it's nicer if your\npatch comes first, that way the test suite (and git) is never broken.\n\nThis is built on top of aae0befee293ebdf9aa6391a074eb0e7c10cdf75 in\nbc/attr-ignore-case, which hopefully makes it easy to replace the top two\ncommits in that branch with these two commits.\n\nI know this is already in next, and you seem poised to merge to master,\nso feel free to drop these if you want.\n\nBrandon Casey (1):\n  attr.c: respect core.ignorecase when matching attribute patterns\n\nJunio C Hamano (1):\n  attr: read core.attributesfile from git_default_core_config\n\n attr.c                |   20 ++++------------\n builtin/check-attr.c  |    2 +\n cache.h               |    1 +\n config.c              |    3 ++\n environment.c         |    1 +\n t/t0003-attributes.sh |   60 ++++++++++++++++++++++++++++++++++++++++++++++++-\n 6 files changed, 71 insertions(+), 16 deletions(-)\n\n-- \n1.7.7.1.ge3b6f\n"},{"id":"177000","messageId":"1317873614-3057-2-git-send-email-drafnel@gmail.com","threadId":"28599","inReplyTo":"1317873614-3057-1-git-send-email-drafnel@gmail.com","subject":"[PATCH v2 1/2] attr: read core.attributesfile from git_default_core_config","fromName":"Brandon Casey","fromEmail":"drafnel@gmail.com","sentAt":"2011-10-06T04:00:13Z","receivedAt":"2011-10-06T04:00:13Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Junio C Hamano <gitster@pobox.com>\n\nThis code calls git_config from a helper function to parse the config entry\nit is interested in.  Calling git_config in this way may cause a problem if\nthe helper function can be called after a previous call to git_config by\nanother function since the second call to git_config may reset some\nvariable to the value in the config file which was previously overridden.\n\nThe above is not a problem in this case since the function passed to\ngit_config only parses one config entry and the variable it sets is not\nassigned outside of the parsing function.  But a programmer who desires\nall of the standard config options to be parsed may be tempted to modify\ngit_attr_config() so that it falls back to git_default_config() and then it\n_would_ be vulnerable to the above described behavior.\n\nSo, move the call to git_config up into the top-level cmd_* function and\nmove the responsibility for parsing core.attributesfile into the main\nconfig file parser.\n\nWhich is only the logical thing to do ;-)\n\nSigned-off-by: Brandon Casey <drafnel@gmail.com>\n---\n attr.c               |   15 ++-------------\n builtin/check-attr.c |    2 ++\n cache.h              |    1 +\n config.c             |    3 +++\n environment.c        |    1 +\n 5 files changed, 9 insertions(+), 13 deletions(-)\n\ndiff --git a/attr.c b/attr.c\nindex 0793859..124337d 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -20,8 +20,6 @@ static const char git_attr__unknown[] = \"(builtin)unknown\";\n #define ATTR__UNSET NULL\n #define ATTR__UNKNOWN git_attr__unknown\n \n-static const char *attributes_file;\n-\n /* This is a randomly chosen prime. */\n #define HASHSIZE 257\n \n@@ -494,14 +492,6 @@ static int git_attr_system(void)\n \treturn !git_env_bool(\"GIT_ATTR_NOSYSTEM\", 0);\n }\n \n-static int git_attr_config(const char *var, const char *value, void *dummy)\n-{\n-\tif (!strcmp(var, \"core.attributesfile\"))\n-\t\treturn git_config_pathname(&attributes_file, var, value);\n-\n-\treturn 0;\n-}\n-\n static void bootstrap_attr_stack(void)\n {\n \tif (!attr_stack) {\n@@ -521,9 +511,8 @@ static void bootstrap_attr_stack(void)\n \t\t\t}\n \t\t}\n \n-\t\tgit_config(git_attr_config, NULL);\n-\t\tif (attributes_file) {\n-\t\t\telem = read_attr_from_file(attributes_file, 1);\n+\t\tif (git_attributes_file) {\n+\t\t\telem = read_attr_from_file(git_attributes_file, 1);\n \t\t\tif (elem) {\n \t\t\t\telem->origin = NULL;\n \t\t\t\telem->prev = attr_stack;\ndiff --git a/builtin/check-attr.c b/builtin/check-attr.c\nindex 708988a..abb1165 100644\n--- a/builtin/check-attr.c\n+++ b/builtin/check-attr.c\n@@ -92,6 +92,8 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)\n \tstruct git_attr_check *check;\n \tint cnt, i, doubledash, filei;\n \n+\tgit_config(git_default_config, NULL);\n+\n \targc = parse_options(argc, argv, prefix, check_attr_options,\n \t\t\t     check_attr_usage, PARSE_OPT_KEEP_DASHDASH);\n \ndiff --git a/cache.h b/cache.h\nindex 607c2ea..8d95fb2 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -589,6 +589,7 @@ extern int warn_ambiguous_refs;\n extern int shared_repository;\n extern const char *apply_default_whitespace;\n extern const char *apply_default_ignorewhitespace;\n+extern const char *git_attributes_file;\n extern int zlib_compression_level;\n extern int core_compression_level;\n extern int core_compression_seen;\ndiff --git a/config.c b/config.c\nindex 4183f80..d3bcaa0 100644\n--- a/config.c\n+++ b/config.c\n@@ -491,6 +491,9 @@ static int git_default_core_config(const char *var, const char *value)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"core.attributesfile\"))\n+\t\treturn git_config_pathname(&git_attributes_file, var, value);\n+\n \tif (!strcmp(var, \"core.bare\")) {\n \t\tis_bare_repository_cfg = git_config_bool(var, value);\n \t\treturn 0;\ndiff --git a/environment.c b/environment.c\nindex e96edcf..d60b73f 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -29,6 +29,7 @@ const char *git_log_output_encoding;\n int shared_repository = PERM_UMASK;\n const char *apply_default_whitespace;\n const char *apply_default_ignorewhitespace;\n+const char *git_attributes_file;\n int zlib_compression_level = Z_BEST_SPEED;\n int core_compression_level;\n int core_compression_seen;\n-- \n1.7.7.1.ge3b6f\n"},{"id":"177001","messageId":"1317873614-3057-3-git-send-email-drafnel@gmail.com","threadId":"28599","inReplyTo":"1317873614-3057-1-git-send-email-drafnel@gmail.com","subject":"[PATCH v2 2/2] attr.c: respect core.ignorecase when matching attribute patterns","fromName":"Brandon Casey","fromEmail":"drafnel@gmail.com","sentAt":"2011-10-06T04:00:14Z","receivedAt":"2011-10-06T04:00:14Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"When core.ignorecase is true, the file globs configured in the\n.gitattributes file should be matched case-insensitively against the paths\nin the working directory.  Let's do so.\n\nPlus, add some tests.\n\nThe last set of tests is performed only on a case-insensitive filesystem.\nThose tests make sure that git handles the case where the .gitignore file\nresides in a subdirectory and the user supplies a path that does not match\nthe case in the filesystem.  In that case^H^H^H^Hsituation, part of the\npath supplied by the user is effectively interpreted case-insensitively,\nand part of it is dependent on the setting of core.ignorecase.  git should\nonly match the portion of the path below the directory holding the\n.gitignore file according to the setting of core.ignorecase.\n\nThis is also partly future-proofing.  Currently, git builds the attr stack\nbased on the path supplied by the user, so we don't have to do anything\nspecial (like use strcmp_icase) to handle the parts of that path that don't\nmatch the filesystem with respect to case.  If git instead built the attr\nstack by scanning the repository, then the paths in the origin field would\nnot necessarily match the paths supplied by the user.  If someone makes a\nchange like that in the future, these tests will notice.\n\nSigned-off-by: Brandon Casey <drafnel@gmail.com>\n---\n attr.c                |    5 ++-\n t/t0003-attributes.sh |   60 ++++++++++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 62 insertions(+), 3 deletions(-)\n\ndiff --git a/attr.c b/attr.c\nindex 124337d..76b079f 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -11,6 +11,7 @@\n #include \"cache.h\"\n #include \"exec_cmd.h\"\n #include \"attr.h\"\n+#include \"dir.h\"\n \n const char git_attr__true[] = \"(builtin)true\";\n const char git_attr__false[] = \"\\0(builtin)false\";\n@@ -631,7 +632,7 @@ static int path_matches(const char *pathname, int pathlen,\n \t\t/* match basename */\n \t\tconst char *basename = strrchr(pathname, '/');\n \t\tbasename = basename ? basename + 1 : pathname;\n-\t\treturn (fnmatch(pattern, basename, 0) == 0);\n+\t\treturn (fnmatch_icase(pattern, basename, 0) == 0);\n \t}\n \t/*\n \t * match with FNM_PATHNAME; the pattern has base implicitly\n@@ -645,7 +646,7 @@ static int path_matches(const char *pathname, int pathlen,\n \t\treturn 0;\n \tif (baselen != 0)\n \t\tbaselen++;\n-\treturn fnmatch(pattern, pathname + baselen, FNM_PATHNAME) == 0;\n+\treturn fnmatch_icase(pattern, pathname + baselen, FNM_PATHNAME) == 0;\n }\n \n static int macroexpand_one(int attr_nr, int rem);\ndiff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh\nindex ae2f1da..47a70c4 100755\n--- a/t/t0003-attributes.sh\n+++ b/t/t0003-attributes.sh\n@@ -9,7 +9,7 @@ attr_check () {\n \tpath=\"$1\"\n \texpect=\"$2\"\n \n-\tgit check-attr test -- \"$path\" >actual 2>err &&\n+\tgit $3 check-attr test -- \"$path\" >actual 2>err &&\n \techo \"$path: test: $2\" >expect &&\n \ttest_cmp expect actual &&\n \ttest_line_count = 0 err\n@@ -27,6 +27,7 @@ test_expect_success 'setup' '\n \t\techo \"onoff test -test\"\n \t\techo \"offon -test test\"\n \t\techo \"no notest\"\n+\t\techo \"A/e/F test=A/e/F\"\n \t) >.gitattributes &&\n \t(\n \t\techo \"g test=a/g\" &&\n@@ -93,6 +94,63 @@ test_expect_success 'attribute test' '\n \n '\n \n+test_expect_success 'attribute matching is case sensitive when core.ignorecase=0' '\n+\n+\ttest_must_fail attr_check F f \"-c core.ignorecase=0\" &&\n+\ttest_must_fail attr_check a/F f \"-c core.ignorecase=0\" &&\n+\ttest_must_fail attr_check a/c/F f \"-c core.ignorecase=0\" &&\n+\ttest_must_fail attr_check a/G a/g \"-c core.ignorecase=0\" &&\n+\ttest_must_fail attr_check a/B/g a/b/g \"-c core.ignorecase=0\" &&\n+\ttest_must_fail attr_check a/b/G a/b/g \"-c core.ignorecase=0\" &&\n+\ttest_must_fail attr_check a/b/H a/b/h \"-c core.ignorecase=0\" &&\n+\ttest_must_fail attr_check a/b/D/g \"a/b/d/*\" \"-c core.ignorecase=0\" &&\n+\ttest_must_fail attr_check oNoFf unset \"-c core.ignorecase=0\" &&\n+\ttest_must_fail attr_check oFfOn set \"-c core.ignorecase=0\" &&\n+\tattr_check NO unspecified \"-c core.ignorecase=0\" &&\n+\ttest_must_fail attr_check a/b/D/NO \"a/b/d/*\" \"-c core.ignorecase=0\" &&\n+\tattr_check a/b/d/YES a/b/d/* \"-c core.ignorecase=0\" &&\n+\ttest_must_fail attr_check a/E/f \"A/e/F\" \"-c core.ignorecase=0\"\n+\n+'\n+\n+test_expect_success 'attribute matching is case insensitive when core.ignorecase=1' '\n+\n+\tattr_check F f \"-c core.ignorecase=1\" &&\n+\tattr_check a/F f \"-c core.ignorecase=1\" &&\n+\tattr_check a/c/F f \"-c core.ignorecase=1\" &&\n+\tattr_check a/G a/g \"-c core.ignorecase=1\" &&\n+\tattr_check a/B/g a/b/g \"-c core.ignorecase=1\" &&\n+\tattr_check a/b/G a/b/g \"-c core.ignorecase=1\" &&\n+\tattr_check a/b/H a/b/h \"-c core.ignorecase=1\" &&\n+\tattr_check a/b/D/g \"a/b/d/*\" \"-c core.ignorecase=1\" &&\n+\tattr_check oNoFf unset \"-c core.ignorecase=1\" &&\n+\tattr_check oFfOn set \"-c core.ignorecase=1\" &&\n+\tattr_check NO unspecified \"-c core.ignorecase=1\" &&\n+\tattr_check a/b/D/NO \"a/b/d/*\" \"-c core.ignorecase=1\" &&\n+\tattr_check a/b/d/YES unspecified \"-c core.ignorecase=1\" &&\n+\tattr_check a/E/f \"A/e/F\" \"-c core.ignorecase=1\"\n+\n+'\n+\n+test_expect_success 'check whether FS is case-insensitive' '\n+\tmkdir junk &&\n+\techo good >junk/CamelCase &&\n+\techo bad >junk/camelcase &&\n+\tif test \"$(cat junk/CamelCase)\" != good\n+\tthen\n+\t\ttest_set_prereq CASE_INSENSITIVE_FS\n+\tfi\n+'\n+\n+test_expect_success CASE_INSENSITIVE_FS 'additional case insensitivity tests' '\n+\ttest_must_fail attr_check a/B/D/g \"a/b/d/*\" \"-c core.ignorecase=0\" &&\n+\ttest_must_fail attr_check A/B/D/NO \"a/b/d/*\" \"-c core.ignorecase=0\" &&\n+\tattr_check A/b/h a/b/h \"-c core.ignorecase=0\" &&\n+\tattr_check A/b/h a/b/h \"-c core.ignorecase=1\" &&\n+\tattr_check a/B/D/g \"a/b/d/*\" \"-c core.ignorecase=1\" &&\n+\tattr_check A/B/D/NO \"a/b/d/*\" \"-c core.ignorecase=1\"\n+'\n+\n test_expect_success 'unnormalized paths' '\n \n \tattr_check ./f f &&\n-- \n1.7.7.1.ge3b6f\n"}]}