{"thread":{"id":"32410","subject":"[PATCH] add core.pathspecGlob config option","startedAt":"2012-12-19T20:34:49Z","lastAt":"2012-12-20T05:26:36Z","messageCount":18,"participants":["Jeff King","Junio C Hamano","Nguyen Thai Ngoc Duy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"205210","messageId":"20121219203449.GA10001@sigill.intra.peff.net","threadId":"32410","inReplyTo":null,"subject":"[PATCH] add core.pathspecGlob config option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-19T20:34:49Z","receivedAt":"2012-12-19T20:34:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Git takes pathspec arguments in many places to limit the\nscope of an operation. These pathspecs are treated not as\nliteral paths, but as glob patterns that can be fed to\nfnmatch. When a user is giving a specific pattern, this is a\nnice feature.\n\nHowever, when programatically providing pathspecs, it can be\na nuisance. For example, to find the latest revision which\nmodified \"$foo\", one can use \"git rev-list -- $foo\". But if\n\"$foo\" contains glob characters (e.g., \"f*\"), it will\nerroneously match more entries than desired. The caller\nneeds to quote the characters in $foo, and even then, the\nresults may not be exactly the same as with a literal\npathspec. For instance, the depth checks in\nmatch_pathspec_depth do not kick in if we match via fnmatch.\n\nThis patch introduces a config option to turn all pathspecs\ninto literal strings. This makes it easy to turn off the\nglobbing behavior for a whole environment (e.g., if you are\nserving repos via a web interface that is only going to\nuse literal programmatic pathspecs), or for a particular run\n(by using \"git -c\" to set the option for this run).\n\nIt cannot turn off globbing for particular pathspecs. That\ncould eventually be done with a \":(noglob)\" magic pathspec\nprefix. However, that level of granularity is not often\nneeded, and doing \":(noglob)\" right would mean converting\nthe whole codebase to use \"struct pathspec\", as the usual\n\"const char **pathspec\" cannot represent extra per-item\nflags.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nPart of me thinks this is just gross, because \":(noglob)\" is the right\nsolution. But after spending a few hours trying it this morning, there\nis a ton of refactoring required to make it work correctly everywhere\n(although we could die() if we see it in places that are not using\ninit_pathspec, so at least we could say \"not supported yet\" and not just\nsilently ignore it).\n\nStill, this is so easy to do, and the lack of granularity does not hurt\nmy use cases at all (which, if you have not guessed, are improving\ncorner cases in scripted calls on GitHub). And I do not think it is just\nme, or just GitHub. Any server-side porcelain would be better off with\nsuch an option (for example, I think gitweb has the same issues; it is\njust that they are pretty rare, because most people do not put glob\nmetacharacters into their filenames).\n\nSo I think this is a nice, simple approach for sites that want it, and\nnoglob magic can come later (and will not be any harder to implement as\na result of this patch).\n\n cache.h                    |  1 +\n config.c                   |  5 +++++\n dir.c                      | 12 +++++-----\n environment.c              |  1 +\n t/t6130-pathspec-noglob.sh | 56 ++++++++++++++++++++++++++++++++++++++++++++++\n 5 files changed, 70 insertions(+), 5 deletions(-)\n create mode 100755 t/t6130-pathspec-noglob.sh\n\ndiff --git a/cache.h b/cache.h\nindex 18fdd18..0725a33 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -555,6 +555,7 @@ extern int precomposed_unicode;\n extern int core_preload_index;\n extern int core_apply_sparse_checkout;\n extern int precomposed_unicode;\n+extern int core_pathspec_glob;\n \n enum branch_track {\n \tBRANCH_TRACK_UNSPECIFIED = -1,\ndiff --git a/config.c b/config.c\nindex fb3f868..8a96ba7 100644\n--- a/config.c\n+++ b/config.c\n@@ -760,6 +760,11 @@ static int git_default_core_config(const char *var, const char *value)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"core.pathspecglob\")) {\n+\t\tcore_pathspec_glob = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \t/* Add other config variables here and to Documentation/config.txt. */\n \treturn 0;\n }\ndiff --git a/dir.c b/dir.c\nindex 5a83aa7..6e81d4f 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -126,7 +126,7 @@ static int match_one(const char *match, const char *name, int namelen)\n \t\tfor (;;) {\n \t\t\tunsigned char c1 = tolower(*match);\n \t\t\tunsigned char c2 = tolower(*name);\n-\t\t\tif (c1 == '\\0' || is_glob_special(c1))\n+\t\t\tif (c1 == '\\0' || (core_pathspec_glob && is_glob_special(c1)))\n \t\t\t\tbreak;\n \t\t\tif (c1 != c2)\n \t\t\t\treturn 0;\n@@ -138,7 +138,7 @@ static int match_one(const char *match, const char *name, int namelen)\n \t\tfor (;;) {\n \t\t\tunsigned char c1 = *match;\n \t\t\tunsigned char c2 = *name;\n-\t\t\tif (c1 == '\\0' || is_glob_special(c1))\n+\t\t\tif (c1 == '\\0' || (core_pathspec_glob && is_glob_special(c1)))\n \t\t\t\tbreak;\n \t\t\tif (c1 != c2)\n \t\t\t\treturn 0;\n@@ -148,14 +148,16 @@ static int match_one(const char *match, const char *name, int namelen)\n \t\t}\n \t}\n \n-\n \t/*\n \t * If we don't match the matchstring exactly,\n \t * we need to match by fnmatch\n \t */\n \tmatchlen = strlen(match);\n-\tif (strncmp_icase(match, name, matchlen))\n+\tif (strncmp_icase(match, name, matchlen)) {\n+\t\tif (!core_pathspec_glob)\n+\t\t\treturn 0;\n \t\treturn !fnmatch_icase(match, name, 0) ? MATCHED_FNMATCH : 0;\n+\t}\n \n \tif (namelen == matchlen)\n \t\treturn MATCHED_EXACTLY;\n@@ -1429,7 +1431,7 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)\n \n \t\titem->match = path;\n \t\titem->len = strlen(path);\n-\t\titem->use_wildcard = !no_wildcard(path);\n+\t\titem->use_wildcard = core_pathspec_glob && !no_wildcard(path);\n \t\tif (item->use_wildcard)\n \t\t\tpathspec->has_wildcard = 1;\n \t}\ndiff --git a/environment.c b/environment.c\nindex 85edd7f..d0d30a0 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -59,6 +59,7 @@ int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */\n int core_apply_sparse_checkout;\n int merge_log_config = -1;\n int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */\n+int core_pathspec_glob = 1;\n struct startup_info *startup_info;\n unsigned long pack_size_limit_cfg;\n \ndiff --git a/t/t6130-pathspec-noglob.sh b/t/t6130-pathspec-noglob.sh\nnew file mode 100755\nindex 0000000..bb962ac\n--- /dev/null\n+++ b/t/t6130-pathspec-noglob.sh\n@@ -0,0 +1,56 @@\n+#!/bin/sh\n+\n+test_description='test globbing (and noglob) of pathspec limiting'\n+. ./test-lib.sh\n+\n+test_expect_success 'create commits with glob characters' '\n+\ttest_commit unrelated bar &&\n+\ttest_commit vanilla foo &&\n+\ttest_commit star \"f*\" &&\n+\ttest_commit bracket \"f[o][o]\"\n+'\n+\n+test_expect_success 'vanilla pathspec matches literally' '\n+\techo vanilla >expect &&\n+\tgit log --format=%s -- foo >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'star pathspec globs' '\n+\tcat >expect <<-\\EOF &&\n+\tbracket\n+\tstar\n+\tvanilla\n+\tEOF\n+\tgit log --format=%s -- \"f*\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'bracket pathspec globs and matches literal brackets' '\n+\tcat >expect <<-\\EOF &&\n+\tbracket\n+\tvanilla\n+\tEOF\n+\tgit log --format=%s -- \"f[o][o]\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'no-glob config matches literally (vanilla)' '\n+\techo vanilla >expect &&\n+\tgit -c core.pathspecglob=false log --format=%s -- foo >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'no-glob config matches literally (star)' '\n+\techo star >expect &&\n+\tgit -c core.pathspecglob=false log --format=%s -- \"f*\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'no-glob config matches literally (bracket)' '\n+\techo bracket >expect &&\n+\tgit -c core.pathspecglob=false log --format=%s -- \"f[o][o]\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n-- \n1.8.1.rc2.24.gf3bad77\n"},{"id":"205212","messageId":"7vk3sd930z.fsf@alter.siamese.dyndns.org","threadId":"32410","inReplyTo":"20121219203449.GA10001@sigill.intra.peff.net","subject":"Re: [PATCH] add core.pathspecGlob config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-19T20:54:04Z","receivedAt":"2012-12-19T20:54:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> ... doing \":(noglob)\" right would mean converting\n> the whole codebase to use \"struct pathspec\", as the usual\n> \"const char **pathspec\" cannot represent extra per-item\n> flags.\n\nAs that is the longer-term direction we would want to go, I'd rather\nnot to take the approach in this patch for introducing user-facing\nsupport of literal pathspecs.\n\nHaving said that, even when we have the ':(noglob)' magic pathspec\nsupport, it would make sense to introduce a command line option to\nmake it easier for scripted Porcelains that call plumbing commands\nto pass literal pathspecs (i.e. they know exactly what paths they\nwant to operate, not what patterns the paths they want to operate\nwould match).  I do not think configuration variable makes much\nsense (unless you are thinking \"git -c core.literalpathspec=true\"\nas that command line option).\n\nThanks\n"},{"id":"205213","messageId":"20121219210919.GA11894@sigill.intra.peff.net","threadId":"32410","inReplyTo":"7vk3sd930z.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] add core.pathspecGlob config option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-19T21:09:19Z","receivedAt":"2012-12-19T21:09:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 19, 2012 at 12:54:04PM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > ... doing \":(noglob)\" right would mean converting\n> > the whole codebase to use \"struct pathspec\", as the usual\n> > \"const char **pathspec\" cannot represent extra per-item\n> > flags.\n> \n> As that is the longer-term direction we would want to go, I'd rather\n> not to take the approach in this patch for introducing user-facing\n> support of literal pathspecs.\n> \n> Having said that, even when we have the ':(noglob)' magic pathspec\n> support, it would make sense to introduce a command line option to\n> make it easier for scripted Porcelains that call plumbing commands\n> to pass literal pathspecs (i.e. they know exactly what paths they\n> want to operate, not what patterns the paths they want to operate\n> would match).\n\nRight, that is my use case. Even once \":(noglob)\" exists, it will still\nbe much simpler for me to use a single option, since I would never have\nmixed patterns and literal paths (and I suspect most use cases that\nwould care about the distinction would be in the same boat). And that is\nwhat led me to submit this at all, as it is not just \"well, it is a hack\nuntil :(noglob) works\", but \"this is not as fine a granularity as\n:(noglob), but it better matches what callers want\".\n\n> I do not think configuration variable makes much sense (unless you are\n> thinking \"git -c core.literalpathspec=true\" as that command line\n> option).\n\nThe configuration variable is much more convenient for me, because\notherwise I have to instrument every call to git with a \"--no-glob\"\noption. Because I have a very restricted environment, and happen to know\nthat globs will _never_ be useful on my repos (or, say, on commands run\nby a particular user who has this in their ~/.gitconfig), it makes sense\nto just turn off the feature with one switch.\n\nAnd my thinking was that for people who are not in such a restricted\nenvironment, \"git -c core.pathspecglob=false\" could serve as that\ncommand-line option, as you mentioned.\n\nIt's perhaps a better match to make it an environment variable. Then it\nis tied to a particular flow of execution, rather than having it be a\nproperty of a system, user, or repo (which is what config does). So for\nmy restricted environment, it would be sufficient for me to set the\nenvironment variable for the user who runs the scripts. And people who\nwant a command-line option can set it via the shell, or we can provide\n\"git --no-pathspec-glob\" to set it.\n\n-Peff\n"},{"id":"205214","messageId":"7vfw3191bi.fsf@alter.siamese.dyndns.org","threadId":"32410","inReplyTo":"20121219203449.GA10001@sigill.intra.peff.net","subject":"Re: [PATCH] add core.pathspecGlob config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-19T21:30:57Z","receivedAt":"2012-12-19T21:30:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> diff --git a/dir.c b/dir.c\n> index 5a83aa7..6e81d4f 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -126,7 +126,7 @@ static int match_one(const char *match, const char *name, int namelen)\n>  \t\tfor (;;) {\n>  \t\t\tunsigned char c1 = tolower(*match);\n>  \t\t\tunsigned char c2 = tolower(*name);\n> -\t\t\tif (c1 == '\\0' || is_glob_special(c1))\n> +\t\t\tif (c1 == '\\0' || (core_pathspec_glob && is_glob_special(c1)))\n>  \t\t\t\tbreak;\n>  \t\t\tif (c1 != c2)\n>  \t\t\t\treturn 0;\n\nI think you can also do the same to the common_prefix(); we check\nfor common leading directory prefix but punt upon seeing a directory\ncomponent that has a glob character, and under the \"literal\" mode,\nit is not a special character.\n"},{"id":"205216","messageId":"20121219214500.GA12705@sigill.intra.peff.net","threadId":"32410","inReplyTo":"7vfw3191bi.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] add core.pathspecGlob config option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-19T21:45:01Z","receivedAt":"2012-12-19T21:45:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 19, 2012 at 01:30:57PM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > diff --git a/dir.c b/dir.c\n> > index 5a83aa7..6e81d4f 100644\n> > --- a/dir.c\n> > +++ b/dir.c\n> > @@ -126,7 +126,7 @@ static int match_one(const char *match, const char *name, int namelen)\n> >  \t\tfor (;;) {\n> >  \t\t\tunsigned char c1 = tolower(*match);\n> >  \t\t\tunsigned char c2 = tolower(*name);\n> > -\t\t\tif (c1 == '\\0' || is_glob_special(c1))\n> > +\t\t\tif (c1 == '\\0' || (core_pathspec_glob && is_glob_special(c1)))\n> >  \t\t\t\tbreak;\n> >  \t\t\tif (c1 != c2)\n> >  \t\t\t\treturn 0;\n> \n> I think you can also do the same to the common_prefix(); we check\n> for common leading directory prefix but punt upon seeing a directory\n> component that has a glob character, and under the \"literal\" mode,\n> it is not a special character.\n\nYeah, I think you're right. Will add to my re-roll.\n\n-Peff\n"},{"id":"205218","messageId":"20121219215008.GA17908@sigill.intra.peff.net","threadId":"32410","inReplyTo":"20121219210919.GA11894@sigill.intra.peff.net","subject":"[PATCH] add GIT_PATHSPEC_GLOB environment variable","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-19T21:50:09Z","receivedAt":"2012-12-19T21:50:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 19, 2012 at 04:09:19PM -0500, Jeff King wrote:\n\n> It's perhaps a better match to make it an environment variable. Then it\n> is tied to a particular flow of execution, rather than having it be a\n> property of a system, user, or repo (which is what config does). So for\n> my restricted environment, it would be sufficient for me to set the\n> environment variable for the user who runs the scripts. And people who\n> want a command-line option can set it via the shell, or we can provide\n> \"git --no-pathspec-glob\" to set it.\n\nHere it is as an environment variable. I think this probably makes more\nsense in the general case (it's a little more work for my use case, but\nI think the intended use is much more obvious).\n\nI included the common_prefix fix you mentioned (I do not think it\nproduced incorrect results as it was, but it did not take full advantage\nof an optimization).\n\n-- >8 --\nSubject: add GIT_PATHSPEC_GLOB environment variable\n\nGit takes pathspec arguments in many places to limit the\nscope of an operation. These pathspecs are treated not as\nliteral paths, but as glob patterns that can be fed to\nfnmatch. When a user is giving a specific pattern, this is a\nnice feature.\n\nHowever, when programatically providing pathspecs, it can be\na nuisance. For example, to find the latest revision which\nmodified \"$foo\", one can use \"git rev-list -- $foo\". But if\n\"$foo\" contains glob characters (e.g., \"f*\"), it will\nerroneously match more entries than desired. The caller\nneeds to quote the characters in $foo, and even then, the\nresults may not be exactly the same as with a literal\npathspec. For instance, the depth checks in\nmatch_pathspec_depth do not kick in if we match via fnmatch.\n\nThis patch introduces an environment variable to turn all\npathspecs into literal strings. This makes it easy to turn\noff the globbing behavior for a whole environment (e.g., if\nyou are serving repos via a web interface that is only going\nto use literal programmatic pathspecs), or for a particular\nrun.\n\nIt cannot turn off globbing for particular pathspecs. That\ncould eventually be done with a \":(noglob)\" magic pathspec\nprefix. However, that level of granularity is not often\nneeded, and doing \":(noglob)\" right would mean converting\nthe whole codebase to use \"struct pathspec\", as the usual\n\"const char **pathspec\" cannot represent extra per-item\nflags.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n Documentation/git.txt      | 15 +++++++++++\n cache.h                    |  3 +++\n dir.c                      | 24 +++++++++++++-----\n git.c                      |  8 ++++++\n t/t6130-pathspec-noglob.sh | 62 ++++++++++++++++++++++++++++++++++++++++++++++\n 5 files changed, 106 insertions(+), 6 deletions(-)\n create mode 100755 t/t6130-pathspec-noglob.sh\n\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 1d797f2..7008b4d 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -422,6 +422,10 @@ help ...`.\n \tDo not use replacement refs to replace git objects. See\n \tlinkgit:git-replace[1] for more information.\n \n+--no-pathspec-glob::\n+\tDo not treat pathspecs as glob patterns. See the section on\n+\tthe `GIT_PATHSPEC_GLOB` environment variable below.\n+\n \n GIT COMMANDS\n ------------\n@@ -790,6 +794,17 @@ for further details.\n \tas a file path and will try to write the trace messages\n \tinto it.\n \n+GIT_PATHSPEC_GLOB::\n+\tSetting this variable to `0` will turn off the globbing features\n+\tof git's pathspecs. For example, running `GIT_PATHSPEC_GLOB=0 git\n+\tlog -- '*.c'` will search for commits literally touching the\n+\tpath `*.c`, not any paths that the glob `*.c` matches. You might\n+\twant this if you are feeding literal paths to git (e.g., paths\n+\tpreviously given to you by `git ls-tree`, `--raw` diff output,\n+\tetc). Setting it to `1` enables pathspec globbing (which is also\n+\tthe default).\n+\n+\n Discussion[[Discussion]]\n ------------------------\n \ndiff --git a/cache.h b/cache.h\nindex 18fdd18..98af77c 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -362,6 +362,7 @@ static inline enum object_type object_type(unsigned int mode)\n #define GIT_NOTES_DISPLAY_REF_ENVIRONMENT \"GIT_NOTES_DISPLAY_REF\"\n #define GIT_NOTES_REWRITE_REF_ENVIRONMENT \"GIT_NOTES_REWRITE_REF\"\n #define GIT_NOTES_REWRITE_MODE_ENVIRONMENT \"GIT_NOTES_REWRITE_MODE\"\n+#define GIT_PATHSPEC_GLOB_ENVIRONMENT \"GIT_PATHSPEC_GLOB\"\n \n /*\n  * Repository-local GIT_* environment variables\n@@ -490,6 +491,8 @@ extern int ce_path_match(const struct cache_entry *ce, const struct pathspec *pa\n extern void free_pathspec(struct pathspec *);\n extern int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec);\n \n+extern int allow_pathspec_glob(void);\n+\n #define HASH_WRITE_OBJECT 1\n #define HASH_FORMAT_CHECK 2\n extern int index_fd(unsigned char *sha1, int fd, struct stat *st, enum object_type type, const char *path, unsigned flags);\ndiff --git a/dir.c b/dir.c\nindex 5a83aa7..a4d4321 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -38,6 +38,7 @@ static size_t common_prefix_len(const char **pathspec)\n {\n \tconst char *n, *first;\n \tsize_t max = 0;\n+\tint glob = allow_pathspec_glob();\n \n \tif (!pathspec)\n \t\treturn max;\n@@ -47,7 +48,7 @@ static size_t common_prefix_len(const char **pathspec)\n \t\tsize_t i, len = 0;\n \t\tfor (i = 0; first == n || i < max; i++) {\n \t\t\tchar c = n[i];\n-\t\t\tif (!c || c != first[i] || is_glob_special(c))\n+\t\t\tif (!c || c != first[i] || (glob && is_glob_special(c)))\n \t\t\t\tbreak;\n \t\t\tif (c == '/')\n \t\t\t\tlen = i + 1;\n@@ -117,6 +118,7 @@ static int match_one(const char *match, const char *name, int namelen)\n static int match_one(const char *match, const char *name, int namelen)\n {\n \tint matchlen;\n+\tint glob = allow_pathspec_glob();\n \n \t/* If the match was just the prefix, we matched */\n \tif (!*match)\n@@ -126,7 +128,7 @@ static int match_one(const char *match, const char *name, int namelen)\n \t\tfor (;;) {\n \t\t\tunsigned char c1 = tolower(*match);\n \t\t\tunsigned char c2 = tolower(*name);\n-\t\t\tif (c1 == '\\0' || is_glob_special(c1))\n+\t\t\tif (c1 == '\\0' || (glob && is_glob_special(c1)))\n \t\t\t\tbreak;\n \t\t\tif (c1 != c2)\n \t\t\t\treturn 0;\n@@ -138,7 +140,7 @@ static int match_one(const char *match, const char *name, int namelen)\n \t\tfor (;;) {\n \t\t\tunsigned char c1 = *match;\n \t\t\tunsigned char c2 = *name;\n-\t\t\tif (c1 == '\\0' || is_glob_special(c1))\n+\t\t\tif (c1 == '\\0' || (glob && is_glob_special(c1)))\n \t\t\t\tbreak;\n \t\t\tif (c1 != c2)\n \t\t\t\treturn 0;\n@@ -148,14 +150,16 @@ static int match_one(const char *match, const char *name, int namelen)\n \t\t}\n \t}\n \n-\n \t/*\n \t * If we don't match the matchstring exactly,\n \t * we need to match by fnmatch\n \t */\n \tmatchlen = strlen(match);\n-\tif (strncmp_icase(match, name, matchlen))\n+\tif (strncmp_icase(match, name, matchlen)) {\n+\t\tif (!glob)\n+\t\t\treturn 0;\n \t\treturn !fnmatch_icase(match, name, 0) ? MATCHED_FNMATCH : 0;\n+\t}\n \n \tif (namelen == matchlen)\n \t\treturn MATCHED_EXACTLY;\n@@ -1429,7 +1433,7 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)\n \n \t\titem->match = path;\n \t\titem->len = strlen(path);\n-\t\titem->use_wildcard = !no_wildcard(path);\n+\t\titem->use_wildcard = allow_pathspec_glob() && !no_wildcard(path);\n \t\tif (item->use_wildcard)\n \t\t\tpathspec->has_wildcard = 1;\n \t}\n@@ -1445,3 +1449,11 @@ void free_pathspec(struct pathspec *pathspec)\n \tfree(pathspec->items);\n \tpathspec->items = NULL;\n }\n+\n+int allow_pathspec_glob(void)\n+{\n+\tstatic int flag = -1;\n+\tif (flag < 0)\n+\t\tflag = git_env_bool(GIT_PATHSPEC_GLOB_ENVIRONMENT, 1);\n+\treturn flag;\n+}\ndiff --git a/git.c b/git.c\nindex d33f9b3..db0496f 100644\n--- a/git.c\n+++ b/git.c\n@@ -135,6 +135,14 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)\n \t\t\tgit_config_push_parameter((*argv)[1]);\n \t\t\t(*argv)++;\n \t\t\t(*argc)--;\n+\t\t} else if (!strcmp(cmd, \"--pathspec-glob\")) {\n+\t\t\tsetenv(GIT_PATHSPEC_GLOB_ENVIRONMENT, \"1\", 1);\n+\t\t\tif (envchanged)\n+\t\t\t\t*envchanged = 1;\n+\t\t} else if (!strcmp(cmd, \"--no-pathspec-glob\")) {\n+\t\t\tsetenv(GIT_PATHSPEC_GLOB_ENVIRONMENT, \"0\", 1);\n+\t\t\tif (envchanged)\n+\t\t\t\t*envchanged = 1;\n \t\t} else {\n \t\t\tfprintf(stderr, \"Unknown option: %s\\n\", cmd);\n \t\t\tusage(git_usage_string);\ndiff --git a/t/t6130-pathspec-noglob.sh b/t/t6130-pathspec-noglob.sh\nnew file mode 100755\nindex 0000000..835e730\n--- /dev/null\n+++ b/t/t6130-pathspec-noglob.sh\n@@ -0,0 +1,62 @@\n+#!/bin/sh\n+\n+test_description='test globbing (and noglob) of pathspec limiting'\n+. ./test-lib.sh\n+\n+test_expect_success 'create commits with glob characters' '\n+\ttest_commit unrelated bar &&\n+\ttest_commit vanilla foo &&\n+\ttest_commit star \"f*\" &&\n+\ttest_commit bracket \"f[o][o]\"\n+'\n+\n+test_expect_success 'vanilla pathspec matches literally' '\n+\techo vanilla >expect &&\n+\tgit log --format=%s -- foo >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'star pathspec globs' '\n+\tcat >expect <<-\\EOF &&\n+\tbracket\n+\tstar\n+\tvanilla\n+\tEOF\n+\tgit log --format=%s -- \"f*\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'bracket pathspec globs and matches literal brackets' '\n+\tcat >expect <<-\\EOF &&\n+\tbracket\n+\tvanilla\n+\tEOF\n+\tgit log --format=%s -- \"f[o][o]\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'no-glob option matches literally (vanilla)' '\n+\techo vanilla >expect &&\n+\tgit --no-pathspec-glob log --format=%s -- foo >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'no-glob option matches literally (star)' '\n+\techo star >expect &&\n+\tgit --no-pathspec-glob log --format=%s -- \"f*\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'no-glob option matches literally (bracket)' '\n+\techo bracket >expect &&\n+\tgit --no-pathspec-glob log --format=%s -- \"f[o][o]\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'no-glob environment variable works' '\n+\techo star >expect &&\n+\tGIT_PATHSPEC_GLOB=0 git log --format=%s -- \"f*\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n-- \n1.8.1.rc2.24.gf3bad77\n"},{"id":"205220","messageId":"7v7god8zz0.fsf@alter.siamese.dyndns.org","threadId":"32410","inReplyTo":"20121219215008.GA17908@sigill.intra.peff.net","subject":"Re: [PATCH] add GIT_PATHSPEC_GLOB environment variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-19T22:00:03Z","receivedAt":"2012-12-19T22:00:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I included the common_prefix fix you mentioned (I do not think it\n> produced incorrect results as it was, but it did not take full advantage\n> of an optimization).\n\nI do not think it would have affected the outcome; you would only\nhave worked with more cycles.\n\n> Subject: add GIT_PATHSPEC_GLOB environment variable\n\nSeems cleanly done from a quick look.\n\nGiven that the normal mode of operation is to use globbing, I\nsuspect that the names would have been more natural if the toggle\nwere GIT_PATHSPEC_LITERAL and the boolean function were\nlimit_pathspec_to_literal(), instead of \"allow_pathspec_glob()\",\nsounding as if using glob is done only upon request.\n\nBut that is a minor issue.\n\n> This patch introduces an environment variable to turn all\n> pathspecs into literal strings. This makes it easy to turn\n> off the globbing behavior for a whole environment (e.g., if\n> you are serving repos via a web interface that is only going\n> to use literal programmatic pathspecs), or for a particular\n> run.\n\nI am not sure if \"web interface\" is a particularly good example,\nthough.  Is it unusual to imagine a Web UI that takes pathspecs from\nthe user to limit its output (e.g. \"diff\" or \"ls-tree\") to those\npaths that match them?  In such a case, the user would expect their\npathspecs to work the same way as the Git installed on their\ndesktop, I would think.\n\nWill queue; thanks.\n"},{"id":"205222","messageId":"20121219221250.GA22823@sigill.intra.peff.net","threadId":"32410","inReplyTo":"7v7god8zz0.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] add GIT_PATHSPEC_GLOB environment variable","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-19T22:12:50Z","receivedAt":"2012-12-19T22:12:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 19, 2012 at 02:00:03PM -0800, Junio C Hamano wrote:\n\n> > Subject: add GIT_PATHSPEC_GLOB environment variable\n> \n> Seems cleanly done from a quick look.\n> \n> Given that the normal mode of operation is to use globbing, I\n> suspect that the names would have been more natural if the toggle\n> were GIT_PATHSPEC_LITERAL and the boolean function were\n> limit_pathspec_to_literal(), instead of \"allow_pathspec_glob()\",\n> sounding as if using glob is done only upon request.\n> \n> But that is a minor issue.\n\nYeah, I was trying to avoid the double-negation of \"noglob=false\" for\nthe default behavior. I guess calling it literal is another way of\naccomplishing that, and it keeps the default at \"false\". I don't have a\nstrong preference.\n\n> > This patch introduces an environment variable to turn all\n> > pathspecs into literal strings. This makes it easy to turn\n> > off the globbing behavior for a whole environment (e.g., if\n> > you are serving repos via a web interface that is only going\n> > to use literal programmatic pathspecs), or for a particular\n> > run.\n> \n> I am not sure if \"web interface\" is a particularly good example,\n> though.  Is it unusual to imagine a Web UI that takes pathspecs from\n> the user to limit its output (e.g. \"diff\" or \"ls-tree\") to those\n> paths that match them?  In such a case, the user would expect their\n> pathspecs to work the same way as the Git installed on their\n> desktop, I would think.\n\nYes. If you want to provide for user-provided globbing pathspecs, then\nyou'd have to annotate each invocation with whether you want globbing or\nnot. What I was trying to illustrate was more how \"gitweb\" will let you\nclick on the \"history\" link for \"foo\" in a tree listing, resulting in a\npage that is generated by calling \"git rev-list foo\". You would probably\nnot want pathspec globbing there.\n\n> Will queue; thanks.\n\nDo we want to change the variable name and invert the logic? Now is\nprobably the best time to do it.\n\n-Peff\n"},{"id":"205223","messageId":"7v38z18z6z.fsf@alter.siamese.dyndns.org","threadId":"32410","inReplyTo":"20121219221250.GA22823@sigill.intra.peff.net","subject":"Re: [PATCH] add GIT_PATHSPEC_GLOB environment variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-19T22:16:52Z","receivedAt":"2012-12-19T22:16:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>> Will queue; thanks.\n>\n> Do we want to change the variable name and invert the logic?\n\nThat would be my preference.\n\nI am deep into today's integration cycle, and this PATHSPEC_GLOB\nversion is sitting at the tip of 'pu', so today's pushout will\ncontain that version, though.\n"},{"id":"205224","messageId":"20121219222035.GA22998@sigill.intra.peff.net","threadId":"32410","inReplyTo":"7v38z18z6z.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] add GIT_PATHSPEC_GLOB environment variable","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-19T22:20:35Z","receivedAt":"2012-12-19T22:20:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 19, 2012 at 02:16:52PM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> >> Will queue; thanks.\n> >\n> > Do we want to change the variable name and invert the logic?\n> \n> That would be my preference.\n> \n> I am deep into today's integration cycle, and this PATHSPEC_GLOB\n> version is sitting at the tip of 'pu', so today's pushout will\n> contain that version, though.\n\nThat's fine. I'll send out a revised version, and you can pick it up\nlater.\n\n-Peff\n"},{"id":"205226","messageId":"20121219223730.GA23154@sigill.intra.peff.net","threadId":"32410","inReplyTo":"20121219222035.GA22998@sigill.intra.peff.net","subject":"Re: [PATCH] add GIT_PATHSPEC_GLOB environment variable","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-19T22:37:30Z","receivedAt":"2012-12-19T22:37:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 19, 2012 at 05:20:35PM -0500, Jeff King wrote:\n\n> > > Do we want to change the variable name and invert the logic?\n> > \n> > That would be my preference.\n> > [...]\n> That's fine. I'll send out a revised version, and you can pick it up\n> later.\n\nHere it is.\n\n-- >8 --\nSubject: [PATCH] add global --literal-pathspecs option\n\nGit takes pathspec arguments in many places to limit the\nscope of an operation. These pathspecs are treated not as\nliteral paths, but as glob patterns that can be fed to\nfnmatch. When a user is giving a specific pattern, this is a\nnice feature.\n\nHowever, when programatically providing pathspecs, it can be\na nuisance. For example, to find the latest revision which\nmodified \"$foo\", one can use \"git rev-list -- $foo\". But if\n\"$foo\" contains glob characters (e.g., \"f*\"), it will\nerroneously match more entries than desired. The caller\nneeds to quote the characters in $foo, and even then, the\nresults may not be exactly the same as with a literal\npathspec. For instance, the depth checks in\nmatch_pathspec_depth do not kick in if we match via fnmatch.\n\nThis patch introduces a global command-line option (i.e.,\none for \"git\" itself, not for specific commands) to turn\nthis behavior off. It also has a matching environment\nvariable, which can make it easier if you are a script or\nporcelain interface that is going to issue many such\ncommands.\n\nThis option cannot turn off globbing for particular\npathspecs. That could eventually be done with a \":(noglob)\"\nmagic pathspec prefix. However, that level of granularity is\nmore cumbersome to use for many cases, and doing \":(noglob)\"\nright would mean converting the whole codebase to use\n\"struct pathspec\", as the usual \"const char **pathspec\"\ncannot represent extra per-item flags.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n Documentation/git.txt      | 15 +++++++++++\n cache.h                    |  3 +++\n dir.c                      | 25 ++++++++++++++-----\n git.c                      |  8 ++++++\n t/t6130-pathspec-noglob.sh | 62 ++++++++++++++++++++++++++++++++++++++++++++++\n 5 files changed, 107 insertions(+), 6 deletions(-)\n create mode 100755 t/t6130-pathspec-noglob.sh\n\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 1d797f2..8d869db 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -422,6 +422,11 @@ help ...`.\n \tDo not use replacement refs to replace git objects. See\n \tlinkgit:git-replace[1] for more information.\n \n+--literal-pathspecs::\n+\tTreat pathspecs literally, rather than as glob patterns. This is\n+\tequivalent to setting the `GIT_LITERAL_PATHSPECS` environment\n+\tvariable to `1`.\n+\n \n GIT COMMANDS\n ------------\n@@ -790,6 +795,16 @@ for further details.\n \tas a file path and will try to write the trace messages\n \tinto it.\n \n+GIT_LITERAL_PATHSPECS::\n+\tSetting this variable to `1` will cause git to treat all\n+\tpathspecs literally, rather than as glob patterns. For example,\n+\trunning `GIT_LITERAL_PATHSPECS=1 git log -- '*.c'` will search\n+\tfor commits that touch the path `*.c`, not any paths that the\n+\tglob `*.c` matches. You might want this if you are feeding\n+\tliteral paths to git (e.g., paths previously given to you by\n+\t`git ls-tree`, `--raw` diff output, etc).\n+\n+\n Discussion[[Discussion]]\n ------------------------\n \ndiff --git a/cache.h b/cache.h\nindex 18fdd18..95608d9 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -362,6 +362,7 @@ static inline enum object_type object_type(unsigned int mode)\n #define GIT_NOTES_DISPLAY_REF_ENVIRONMENT \"GIT_NOTES_DISPLAY_REF\"\n #define GIT_NOTES_REWRITE_REF_ENVIRONMENT \"GIT_NOTES_REWRITE_REF\"\n #define GIT_NOTES_REWRITE_MODE_ENVIRONMENT \"GIT_NOTES_REWRITE_MODE\"\n+#define GIT_LITERAL_PATHSPECS_ENVIRONMENT \"GIT_LITERAL_PATHSPECS\"\n \n /*\n  * Repository-local GIT_* environment variables\n@@ -490,6 +491,8 @@ extern int ce_path_match(const struct cache_entry *ce, const struct pathspec *pa\n extern void free_pathspec(struct pathspec *);\n extern int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec);\n \n+extern int limit_pathspec_to_literal(void);\n+\n #define HASH_WRITE_OBJECT 1\n #define HASH_FORMAT_CHECK 2\n extern int index_fd(unsigned char *sha1, int fd, struct stat *st, enum object_type type, const char *path, unsigned flags);\ndiff --git a/dir.c b/dir.c\nindex 5a83aa7..03ff36b 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -38,6 +38,7 @@ static size_t common_prefix_len(const char **pathspec)\n {\n \tconst char *n, *first;\n \tsize_t max = 0;\n+\tint literal = limit_pathspec_to_literal();\n \n \tif (!pathspec)\n \t\treturn max;\n@@ -47,7 +48,7 @@ static size_t common_prefix_len(const char **pathspec)\n \t\tsize_t i, len = 0;\n \t\tfor (i = 0; first == n || i < max; i++) {\n \t\t\tchar c = n[i];\n-\t\t\tif (!c || c != first[i] || is_glob_special(c))\n+\t\t\tif (!c || c != first[i] || (!literal && is_glob_special(c)))\n \t\t\t\tbreak;\n \t\t\tif (c == '/')\n \t\t\t\tlen = i + 1;\n@@ -117,6 +118,7 @@ static int match_one(const char *match, const char *name, int namelen)\n static int match_one(const char *match, const char *name, int namelen)\n {\n \tint matchlen;\n+\tint literal = limit_pathspec_to_literal();\n \n \t/* If the match was just the prefix, we matched */\n \tif (!*match)\n@@ -126,7 +128,7 @@ static int match_one(const char *match, const char *name, int namelen)\n \t\tfor (;;) {\n \t\t\tunsigned char c1 = tolower(*match);\n \t\t\tunsigned char c2 = tolower(*name);\n-\t\t\tif (c1 == '\\0' || is_glob_special(c1))\n+\t\t\tif (c1 == '\\0' || (!literal && is_glob_special(c1)))\n \t\t\t\tbreak;\n \t\t\tif (c1 != c2)\n \t\t\t\treturn 0;\n@@ -138,7 +140,7 @@ static int match_one(const char *match, const char *name, int namelen)\n \t\tfor (;;) {\n \t\t\tunsigned char c1 = *match;\n \t\t\tunsigned char c2 = *name;\n-\t\t\tif (c1 == '\\0' || is_glob_special(c1))\n+\t\t\tif (c1 == '\\0' || (!literal && is_glob_special(c1)))\n \t\t\t\tbreak;\n \t\t\tif (c1 != c2)\n \t\t\t\treturn 0;\n@@ -148,14 +150,16 @@ static int match_one(const char *match, const char *name, int namelen)\n \t\t}\n \t}\n \n-\n \t/*\n \t * If we don't match the matchstring exactly,\n \t * we need to match by fnmatch\n \t */\n \tmatchlen = strlen(match);\n-\tif (strncmp_icase(match, name, matchlen))\n+\tif (strncmp_icase(match, name, matchlen)) {\n+\t\tif (literal)\n+\t\t\treturn 0;\n \t\treturn !fnmatch_icase(match, name, 0) ? MATCHED_FNMATCH : 0;\n+\t}\n \n \tif (namelen == matchlen)\n \t\treturn MATCHED_EXACTLY;\n@@ -1429,7 +1433,8 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)\n \n \t\titem->match = path;\n \t\titem->len = strlen(path);\n-\t\titem->use_wildcard = !no_wildcard(path);\n+\t\titem->use_wildcard = !limit_pathspec_to_literal() &&\n+\t\t\t\t     !no_wildcard(path);\n \t\tif (item->use_wildcard)\n \t\t\tpathspec->has_wildcard = 1;\n \t}\n@@ -1445,3 +1450,11 @@ void free_pathspec(struct pathspec *pathspec)\n \tfree(pathspec->items);\n \tpathspec->items = NULL;\n }\n+\n+int limit_pathspec_to_literal(void)\n+{\n+\tstatic int flag = -1;\n+\tif (flag < 0)\n+\t\tflag = git_env_bool(GIT_LITERAL_PATHSPECS_ENVIRONMENT, 0);\n+\treturn flag;\n+}\ndiff --git a/git.c b/git.c\nindex d33f9b3..ed66c66 100644\n--- a/git.c\n+++ b/git.c\n@@ -135,6 +135,14 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)\n \t\t\tgit_config_push_parameter((*argv)[1]);\n \t\t\t(*argv)++;\n \t\t\t(*argc)--;\n+\t\t} else if (!strcmp(cmd, \"--literal-pathspecs\")) {\n+\t\t\tsetenv(GIT_LITERAL_PATHSPECS_ENVIRONMENT, \"1\", 1);\n+\t\t\tif (envchanged)\n+\t\t\t\t*envchanged = 1;\n+\t\t} else if (!strcmp(cmd, \"--no-literal-pathspecs\")) {\n+\t\t\tsetenv(GIT_LITERAL_PATHSPECS_ENVIRONMENT, \"0\", 1);\n+\t\t\tif (envchanged)\n+\t\t\t\t*envchanged = 1;\n \t\t} else {\n \t\t\tfprintf(stderr, \"Unknown option: %s\\n\", cmd);\n \t\t\tusage(git_usage_string);\ndiff --git a/t/t6130-pathspec-noglob.sh b/t/t6130-pathspec-noglob.sh\nnew file mode 100755\nindex 0000000..bb5e710\n--- /dev/null\n+++ b/t/t6130-pathspec-noglob.sh\n@@ -0,0 +1,62 @@\n+#!/bin/sh\n+\n+test_description='test globbing (and noglob) of pathspec limiting'\n+. ./test-lib.sh\n+\n+test_expect_success 'create commits with glob characters' '\n+\ttest_commit unrelated bar &&\n+\ttest_commit vanilla foo &&\n+\ttest_commit star \"f*\" &&\n+\ttest_commit bracket \"f[o][o]\"\n+'\n+\n+test_expect_success 'vanilla pathspec matches literally' '\n+\techo vanilla >expect &&\n+\tgit log --format=%s -- foo >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'star pathspec globs' '\n+\tcat >expect <<-\\EOF &&\n+\tbracket\n+\tstar\n+\tvanilla\n+\tEOF\n+\tgit log --format=%s -- \"f*\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'bracket pathspec globs and matches literal brackets' '\n+\tcat >expect <<-\\EOF &&\n+\tbracket\n+\tvanilla\n+\tEOF\n+\tgit log --format=%s -- \"f[o][o]\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'no-glob option matches literally (vanilla)' '\n+\techo vanilla >expect &&\n+\tgit --literal-pathspecs log --format=%s -- foo >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'no-glob option matches literally (star)' '\n+\techo star >expect &&\n+\tgit --literal-pathspecs log --format=%s -- \"f*\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'no-glob option matches literally (bracket)' '\n+\techo bracket >expect &&\n+\tgit --literal-pathspecs log --format=%s -- \"f[o][o]\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'no-glob environment variable works' '\n+\techo star >expect &&\n+\tGIT_LITERAL_PATHSPECS=1 git log --format=%s -- \"f*\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n-- \n1.8.1.rc2.24.gf3bad77\n"},{"id":"205233","messageId":"CACsJy8BB3=3ZHD5Ua9M-0+98JVigHBBuo07gBSgEwanvB0zBSA@mail.gmail.com","threadId":"32410","inReplyTo":"20121219203449.GA10001@sigill.intra.peff.net","subject":"Re: [PATCH] add core.pathspecGlob config option","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-12-20T01:28:57Z","receivedAt":"2012-12-20T01:28:57Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Dec 20, 2012 at 3:34 AM, Jeff King <peff@peff.net> wrote:\n> Part of me thinks this is just gross, because \":(noglob)\" is the right\n> solution. But after spending a few hours trying it this morning, there\n> is a ton of refactoring required to make it work correctly everywhere\n> (although we could die() if we see it in places that are not using\n> init_pathspec, so at least we could say \"not supported yet\" and not just\n> silently ignore it).\n\nYep, I'm still half way to converting everything to the new pathspec\ncode. I'm not there yet. And things like this probably better goes\nwith a config key or command line option, appending :(noglob) to every\npathspec item is not pleasant. :(icase) may go the same way.\n\n> So I think this is a nice, simple approach for sites that want it, and\n> noglob magic can come later (and will not be any harder to implement as\n> a result of this patch).\n\nAny chance to make use of nd/pathspec-wildcard? It changes the same\ncode path in match_one. If you base on top of nd/pathspec-wildcard,\nall you have to do is assign nowildcard_len to len (i.e. no wildcard\npart).\n-- \nDuy\n"},{"id":"205238","messageId":"20121220031327.GB9917@sigill.intra.peff.net","threadId":"32410","inReplyTo":"CACsJy8BB3=3ZHD5Ua9M-0+98JVigHBBuo07gBSgEwanvB0zBSA@mail.gmail.com","subject":"Re: [PATCH] add core.pathspecGlob config option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-20T03:13:27Z","receivedAt":"2012-12-20T03:13:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 20, 2012 at 08:28:57AM +0700, Nguyen Thai Ngoc Duy wrote:\n\n> > So I think this is a nice, simple approach for sites that want it, and\n> > noglob magic can come later (and will not be any harder to implement as\n> > a result of this patch).\n> \n> Any chance to make use of nd/pathspec-wildcard? It changes the same\n> code path in match_one. If you base on top of nd/pathspec-wildcard,\n> all you have to do is assign nowildcard_len to len (i.e. no wildcard\n> part).\n\nI'd rather keep it separate for now. One, just because they really are\nindependent topics, and two, because I am actually back-porting it for\nGitHub (we are fairly conservative about upgrading our backend git\nversions, as most of the interesting stuff happens on the client side; I\ncherry-pick critical patches with no regard to the release cycle).\n\nAnd the resolution is pretty trivial, too. It looks like this:\n\ndiff --cc dir.c\nindex 5c0e5f6,03ff36b..81cb439\n--- a/dir.c\n+++ b/dir.c\n@@@ -1456,14 -1433,10 +1460,18 @@@ int init_pathspec(struct pathspec *path\n  \n  \t\titem->match = path;\n  \t\titem->len = strlen(path);\n- \t\titem->nowildcard_len = simple_length(path);\n -\t\titem->use_wildcard = !limit_pathspec_to_literal() &&\n -\t\t\t\t     !no_wildcard(path);\n -\t\tif (item->use_wildcard)\n -\t\t\tpathspec->has_wildcard = 1;\n +\t\titem->flags = 0;\n- \t\tif (item->nowildcard_len < item->len) {\n- \t\t\tpathspec->has_wildcard = 1;\n- \t\t\tif (path[item->nowildcard_len] == '*' &&\n- \t\t\t    no_wildcard(path + item->nowildcard_len + 1))\n- \t\t\t\titem->flags |= PATHSPEC_ONESTAR;\n++\t\tif (limit_pathspec_to_literal())\n++\t\t\titem->nowildcard_len = item->len;\n++\t\telse {\n++\t\t\titem->nowildcard_len = simple_length(path);\n++\t\t\tif (item->nowildcard_len < item->len) {\n++\t\t\t\tpathspec->has_wildcard = 1;\n++\t\t\t\tif (path[item->nowildcard_len] == '*' &&\n++\t\t\t\t    no_wildcard(path + item->nowildcard_len + 1))\n++\t\t\t\t\titem->flags |= PATHSPEC_ONESTAR;\n++\t\t\t}\n +\t\t}\n  \t}\n  \n  \tqsort(pathspec->items, pathspec->nr,\n\nNot re-indenting the conditional would make the diff more readable, but\nI think the resulting code is simpler to read if all of the wildcard\nstuff is inside the \"else\" block.\n\n-Peff\n"},{"id":"205239","messageId":"7vehil7557.fsf@alter.siamese.dyndns.org","threadId":"32410","inReplyTo":"20121220031327.GB9917@sigill.intra.peff.net","subject":"Re: [PATCH] add core.pathspecGlob config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-20T03:51:16Z","receivedAt":"2012-12-20T03:51:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Dec 20, 2012 at 08:28:57AM +0700, Nguyen Thai Ngoc Duy wrote:\n>\n>> > So I think this is a nice, simple approach for sites that want it, and\n>> > noglob magic can come later (and will not be any harder to implement as\n>> > a result of this patch).\n>> \n>> Any chance to make use of nd/pathspec-wildcard? It changes the same\n>> code path in match_one. If you base on top of nd/pathspec-wildcard,\n>> all you have to do is assign nowildcard_len to len (i.e. no wildcard\n>> part).\n>\n> I'd rather keep it separate for now. One, just because they really are\n> independent topics, and two, because I am actually back-porting it for\n> GitHub (we are fairly conservative about upgrading our backend git\n> versions, as most of the interesting stuff happens on the client side; I\n> cherry-pick critical patches with no regard to the release cycle).\n>\n> And the resolution is pretty trivial, too. It looks like this:\n>\n> diff --cc dir.c\n> index 5c0e5f6,03ff36b..81cb439\n> --- a/dir.c\n> +++ b/dir.c\n> @@@ -1456,14 -1433,10 +1460,18 @@@ int init_pathspec(struct pathspec *path\n>   \n>   \t\titem->match = path;\n>   \t\titem->len = strlen(path);\n> - \t\titem->nowildcard_len = simple_length(path);\n>  -\t\titem->use_wildcard = !limit_pathspec_to_literal() &&\n>  -\t\t\t\t     !no_wildcard(path);\n>  -\t\tif (item->use_wildcard)\n>  -\t\t\tpathspec->has_wildcard = 1;\n>  +\t\titem->flags = 0;\n> - \t\tif (item->nowildcard_len < item->len) {\n> - \t\t\tpathspec->has_wildcard = 1;\n> - \t\t\tif (path[item->nowildcard_len] == '*' &&\n> - \t\t\t    no_wildcard(path + item->nowildcard_len + 1))\n> - \t\t\t\titem->flags |= PATHSPEC_ONESTAR;\n> ++\t\tif (limit_pathspec_to_literal())\n> ++\t\t\titem->nowildcard_len = item->len;\n> ++\t\telse {\n> ++\t\t\titem->nowildcard_len = simple_length(path);\n> ++\t\t\tif (item->nowildcard_len < item->len) {\n> ++\t\t\t\tpathspec->has_wildcard = 1;\n> ++\t\t\t\tif (path[item->nowildcard_len] == '*' &&\n> ++\t\t\t\t    no_wildcard(path + item->nowildcard_len + 1))\n> ++\t\t\t\t\titem->flags |= PATHSPEC_ONESTAR;\n> ++\t\t\t}\n>  +\t\t}\n>   \t}\n>   \n>   \tqsort(pathspec->items, pathspec->nr,\n\nHmph.  I thought that returning the length without any \"stop at glob\nspecial\" trick from simple_length() would be a simpler resolution.\n\nThat is what is queued at the tip of 'pu', anyway.\n"},{"id":"205240","messageId":"20121220035543.GA14965@sigill.intra.peff.net","threadId":"32410","inReplyTo":"7vehil7557.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] add core.pathspecGlob config option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-20T03:55:43Z","receivedAt":"2012-12-20T03:55:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 19, 2012 at 07:51:16PM -0800, Junio C Hamano wrote:\n\n> > ++\t\tif (limit_pathspec_to_literal())\n> > ++\t\t\titem->nowildcard_len = item->len;\n> > ++\t\telse {\n> > ++\t\t\titem->nowildcard_len = simple_length(path);\n> > ++\t\t\tif (item->nowildcard_len < item->len) {\n> > ++\t\t\t\tpathspec->has_wildcard = 1;\n> > ++\t\t\t\tif (path[item->nowildcard_len] == '*' &&\n> > ++\t\t\t\t    no_wildcard(path + item->nowildcard_len + 1))\n> > ++\t\t\t\t\titem->flags |= PATHSPEC_ONESTAR;\n> > ++\t\t\t}\n> >  +\t\t}\n> \n> Hmph.  I thought that returning the length without any \"stop at glob\n> special\" trick from simple_length() would be a simpler resolution.\n> \n> That is what is queued at the tip of 'pu', anyway.\n\nI don't think we can make a change in simple_length. It gets used not\nonly for pathspecs, but also for parsing exclude patterns, which I do\nnot think should be affected by this option.\n\n-Peff\n"},{"id":"205241","messageId":"20121220040602.GA15172@sigill.intra.peff.net","threadId":"32410","inReplyTo":"20121220035543.GA14965@sigill.intra.peff.net","subject":"Re: [PATCH] add core.pathspecGlob config option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-20T04:06:02Z","receivedAt":"2012-12-20T04:06:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 19, 2012 at 10:55:43PM -0500, Jeff King wrote:\n\n> On Wed, Dec 19, 2012 at 07:51:16PM -0800, Junio C Hamano wrote:\n> \n> > > ++\t\tif (limit_pathspec_to_literal())\n> > > ++\t\t\titem->nowildcard_len = item->len;\n> > > ++\t\telse {\n> > > ++\t\t\titem->nowildcard_len = simple_length(path);\n> > > ++\t\t\tif (item->nowildcard_len < item->len) {\n> > > ++\t\t\t\tpathspec->has_wildcard = 1;\n> > > ++\t\t\t\tif (path[item->nowildcard_len] == '*' &&\n> > > ++\t\t\t\t    no_wildcard(path + item->nowildcard_len + 1))\n> > > ++\t\t\t\t\titem->flags |= PATHSPEC_ONESTAR;\n> > > ++\t\t\t}\n> > >  +\t\t}\n> > \n> > Hmph.  I thought that returning the length without any \"stop at glob\n> > special\" trick from simple_length() would be a simpler resolution.\n> > \n> > That is what is queued at the tip of 'pu', anyway.\n> \n> I don't think we can make a change in simple_length. It gets used not\n> only for pathspecs, but also for parsing exclude patterns, which I do\n> not think should be affected by this option.\n\nOur test suite wouldn't catch such a misfeature, of course, because the\nfeature is not turned on by default. But I found it instructive to run\nall of the tests with GIT_LITERAL_PATHSPECS on. There are failures, of\ncourse, but by inspecting each failure you can see that it is an\nintended effect of the patch (i.e., each tries to use a wildcard\npathspec, which no longer works).\n\nWhen you suggested changing common_prefix, I ran such a test both with\nand without the change[1] and confirmed that it did not change the set\nof failure sites. I did not try it, but I suspect running such a test\nwith the tip of pu would reveal new failures in the .gitignore tests.\n\n-Peff\n\n[1] This is in addition to reading and reasoning about the code, of\n    course. I would not consider this a very robust form of testing,\n    though a test failure which cannot be easily explained would be a\n    good starting point for investigation.\n"},{"id":"205242","messageId":"20121220041141.GA23798@sigill.intra.peff.net","threadId":"32410","inReplyTo":"20121220040602.GA15172@sigill.intra.peff.net","subject":"Re: [PATCH] add core.pathspecGlob config option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-20T04:11:41Z","receivedAt":"2012-12-20T04:11:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 19, 2012 at 11:06:02PM -0500, Jeff King wrote:\n\n> > I don't think we can make a change in simple_length. It gets used not\n> > only for pathspecs, but also for parsing exclude patterns, which I do\n> > not think should be affected by this option.\n> \n> Our test suite wouldn't catch such a misfeature, of course, because the\n> feature is not turned on by default. But I found it instructive to run\n> all of the tests with GIT_LITERAL_PATHSPECS on. There are failures, of\n> course, but by inspecting each failure you can see that it is an\n> intended effect of the patch (i.e., each tries to use a wildcard\n> pathspec, which no longer works).\n> \n> When you suggested changing common_prefix, I ran such a test both with\n> and without the change[1] and confirmed that it did not change the set\n> of failure sites. I did not try it, but I suspect running such a test\n> with the tip of pu would reveal new failures in the .gitignore tests.\n\nI just tried it, and indeed, running the test suite with this patch:\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 256f1c6..1c43593 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -102,6 +102,9 @@ export EDITOR\n export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME\n export EDITOR\n \n+GIT_LITERAL_PATHSPECS=1\n+export GIT_LITERAL_PATHSPECS\n+\n # Add libc MALLOC and MALLOC_PERTURB test\n # only if we are not executing the test with valgrind\n if expr \" $GIT_TEST_OPTS \" : \".* --valgrind \" >/dev/null ||\n\n\nproduces many more failures on \"pu\" than it does on jk/pathspec-literal.\n\n-Peff\n"},{"id":"205246","messageId":"7va9t970qb.fsf@alter.siamese.dyndns.org","threadId":"32410","inReplyTo":"20121220035543.GA14965@sigill.intra.peff.net","subject":"Re: [PATCH] add core.pathspecGlob config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-20T05:26:36Z","receivedAt":"2012-12-20T05:26:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Wed, Dec 19, 2012 at 07:51:16PM -0800, Junio C Hamano wrote:\n>\n>> > ++\t\tif (limit_pathspec_to_literal())\n>> > ++\t\t\titem->nowildcard_len = item->len;\n>> > ++\t\telse {\n>> > ++\t\t\titem->nowildcard_len = simple_length(path);\n>> > ++\t\t\tif (item->nowildcard_len < item->len) {\n>> > ++\t\t\t\tpathspec->has_wildcard = 1;\n>> > ++\t\t\t\tif (path[item->nowildcard_len] == '*' &&\n>> > ++\t\t\t\t    no_wildcard(path + item->nowildcard_len + 1))\n>> > ++\t\t\t\t\titem->flags |= PATHSPEC_ONESTAR;\n>> > ++\t\t\t}\n>> >  +\t\t}\n>> \n>> Hmph.  I thought that returning the length without any \"stop at glob\n>> special\" trick from simple_length() would be a simpler resolution.\n>> \n>> That is what is queued at the tip of 'pu', anyway.\n>\n> I don't think we can make a change in simple_length. It gets used not\n> only for pathspecs, but also for parsing exclude patterns, which I do\n> not think should be affected by this option.\n\nOuch, yeah, you're right.\n"}]}