{"thread":{"id":"11007","subject":"[RFC] Making ce_path_match() more useful by accepting globs","startedAt":"2007-11-25T18:03:12Z","lastAt":"2007-11-26T19:38:13Z","messageCount":5,"participants":["Junio C Hamano","Alex Riesen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"60838","messageId":"7vr6ienqxr.fsf@gitster.siamese.dyndns.org","threadId":"11007","inReplyTo":null,"subject":"[RFC] Making ce_path_match() more useful by accepting globs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-25T18:03:12Z","receivedAt":"2007-11-25T18:03:12Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Currently ce_path_match() only uses \"the leading directory\" match, and\ndoes not understand file globs. These do not work:\n\n git diff-files 't/*.sh'\n git diff-index HEAD 'xdiff/*.c'\n git update-index -g 'Documentation/howto/*.txt'\n\nThis teaches the ce_path_match(), the underlying function that are used\nfor checking if a given cache entry matches the given set of pathspecs,\nto use the match_pathspec() from git-ls-files, which knows about glob\npatterns.\n\nSigned-off-by: Junio C Hamano \n---\n\n * Having two different behaviours of pathspec matching has been\n bothering me for quite some time. The changes here look trivially\n correct and the result passes all the tests, but this is quite close\n to the core part of the system, and would benefit greatly from extra\n set of eyes.\n\n This patch does not touch the tree walker, and does not affect\n diff-tree nor ancestry pruning done in the revision traversal. That\n however is even closer to the core and is performance critical. It\n needs to be done carefully not to descend into trees that would never\n match needlessly. IOW, not today.\n\n dir.c | 5 +++--\n read-cache.c | 17 ++---------------\n 2 files changed, 5 insertions(+), 17 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 225fdfb..be640c9 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -98,20 +98,21 @@ int match_pathspec(const char **pathspec, const char *name, int namelen, int pre\n {\n \tint retval;\n \tconst char *match;\n+\tint want_seen = !!seen;\n \n \tname += prefix;\n \tnamelen -= prefix;\n \n \tfor (retval = 0; (match = *pathspec++) != NULL; seen++) {\n \t\tint how;\n-\t\tif (retval && *seen == MATCHED_EXACTLY)\n+\t\tif (retval && seen && want_seen && *seen == MATCHED_EXACTLY)\n \t\t\tcontinue;\n \t\tmatch += prefix;\n \t\thow = match_one(match, name, namelen);\n \t\tif (how) {\n \t\t\tif (retval < how)\n \t\t\t\tretval = how;\n-\t\t\tif (*seen < how)\n+\t\t\tif (want_seen && *seen < how)\n \t\t\t\t*seen = how;\n \t\t}\n \t}\ndiff --git a/read-cache.c b/read-cache.c\nindex 7db5588..767464e 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -472,7 +472,7 @@ int ce_same_name(struct cache_entry *a, struct cache_entry *b)\n \n int ce_path_match(const struct cache_entry *ce, const char **pathspec)\n {\n-\tconst char *match, *name;\n+\tconst char *name;\n \tint len;\n \n \tif (!pathspec)\n@@ -480,20 +480,7 @@ int ce_path_match(const struct cache_entry *ce, const char **pathspec)\n \n \tlen = ce_namelen(ce);\n \tname = ce->name;\n-\twhile ((match = *pathspec++) != NULL) {\n-\t\tint matchlen = strlen(match);\n-\t\tif (matchlen > len)\n-\t\t\tcontinue;\n-\t\tif (memcmp(name, match, matchlen))\n-\t\t\tcontinue;\n-\t\tif (matchlen && name[matchlen-1] == '/')\n-\t\t\treturn 1;\n-\t\tif (name[matchlen] == '/' || !name[matchlen])\n-\t\t\treturn 1;\n-\t\tif (!matchlen)\n-\t\t\treturn 1;\n-\t}\n-\treturn 0;\n+\treturn !!match_pathspec(pathspec, name, len, 0, NULL);\n }\n \n /*\n"},{"id":"60891","messageId":"20071126001114.GA10286@steel.home","threadId":"11007","inReplyTo":"7vr6ienqxr.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFC] Making ce_path_match() more useful by accepting globs","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-11-26T00:11:14Z","receivedAt":"2007-11-26T00:11:14Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Junio C Hamano, Sun, Nov 25, 2007 19:03:12 +0100:\n> Currently ce_path_match() only uses \"the leading directory\" match, and\n> does not understand file globs. These do not work:\n> \n> git diff-files 't/*.sh'\n> git diff-index HEAD 'xdiff/*.c'\n> git update-index -g 'Documentation/howto/*.txt'\n\nHow should my scripts handle files with \"*\" in names?\nMaybe before the proposed change we should make all plumbing accept\n\"--stdin\" (which does not expand globs)?\n\nP.S. you have something broken in your mailer:\n\n To: Linus@sceptre.sasl.smtp.pobox.com,\n\t \"Torvalds writes:\n\n> Junio C Hamano, Sun, Nov 25, 2007 19:03:12 +0100:\n>> Currently ce_path_match() only uses \"the leading directory\" match, and\n>> does not understand file globs. These do not work:\n>> \n>> git diff-files 't/*.sh'\n>> git diff-index HEAD 'xdiff/*.c'\n>> git update-index -g 'Documentation/howto/*.txt'\n>\n> How should my scripts handle files with \"*\" in names?\n\nWe DO NOT CARE.\n\nWhy?\n\nHow would you handle such files from the command line session without\ngit? \"ls such-*-a-file\" will also show such-silly-a-file as well.\n\nIOW, the user is shooting in the foot --- and at that point I am not all\nthat interested in helping him.\n\nHaving said that, I would think that quoting the meta from fnmatch(3)\nlike this:\n\n\tgit ls-files 'such-\\*-a-file'\n\nwould work fine, just like\n\n\tls such-\\*-a-file\n\nwould.\n\nIf \"ls such-*-a-file\" reports only one file,\n\n\tgit ls-files 'such-*-a-file'\n\nwould also report that file as well.\n\nSo in practice I do not see a problem.\n"},{"id":"60894","messageId":"7vhcj9j0hj.fsf@gitster.siamese.dyndns.org","threadId":"11007","inReplyTo":"7vlk8lj1b7.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFC] Making ce_path_match() more useful by accepting globs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-26T00:48:08Z","receivedAt":"2007-11-26T00:48:08Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Alex Riesen writes:\n>\n>> Junio C Hamano, Sun, Nov 25, 2007 19:03:12 +0100:\n>>> Currently ce_path_match() only uses \"the leading directory\" match, and\n>>> does not understand file globs. These do not work:\n>>> \n>>> git diff-files 't/*.sh'\n>>> git diff-index HEAD 'xdiff/*.c'\n>>> git update-index -g 'Documentation/howto/*.txt'\n>>\n>> How should my scripts handle files with \"*\" in names?\n> ...\n> Having said that, I would think that quoting the meta from fnmatch(3)\n> like this:\n>\n> \tgit ls-files 'such-\\*-a-file'\n>\n> would work fine, just like\n>\n> \tls such-\\*-a-file\n>\n> would.\n\nYup. I just did:\n\n\t$ >M\\*kefile\n $ git add 'M\\*kefile'\n $ git ls-files 'M\\*kefile'\n M*kefile\n $ git ls-files 'M*kefile'\n M*kefile\n Makefile\n"},{"id":"60963","messageId":"20071126193813.GB3675@steel.home","threadId":"11007","inReplyTo":"7vlk8lj1b7.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFC] Making ce_path_match() more useful by accepting globs","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-11-26T19:38:13Z","receivedAt":"2007-11-26T19:38:13Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Junio C Hamano, Mon, Nov 26, 2007 01:30:20 +0100:\n> Alex Riesen writes:\n> \n> > Junio C Hamano, Sun, Nov 25, 2007 19:03:12 +0100:\n> >> Currently ce_path_match() only uses \"the leading directory\" match, and\n> >> does not understand file globs. These do not work:\n> >> \n> >> git diff-files 't/*.sh'\n> >> git diff-index HEAD 'xdiff/*.c'\n> >> git update-index -g 'Documentation/howto/*.txt'\n> >\n> > How should my scripts handle files with \"*\" in names?\n> \n> We DO NOT CARE.\n> \n> Why?\n> \n> How would you handle such files from the command line session without\n> git? \"ls such-*-a-file\" will also show such-silly-a-file as well.\n\nIt will break existing setups. Something like\n\n #!/bin/bash\n\n git diff-files --quiet -- \"$@\" || do_something\n\nwill behave differently\n\n> IOW, the user is shooting in the foot --- and at that point I am not all\n> that interested in helping him.\n\nWhat with? What's wrong with a name like \"M*A*S*H\" in your personal IMDB?\n\n> Having said that, I would think that quoting the meta from fnmatch(3)\n> like this:\n> \n> \tgit ls-files 'such-\\*-a-file'\n> \n> would work fine, just like\n> \n> \tls such-\\*-a-file\n> \n> would.\n\nAn existing system will still be broken.\n\n> If \"ls such-*-a-file\" reports only one file,\n> \n> \tgit ls-files 'such-*-a-file'\n> \n> would also report that file as well.\n> \n> So in practice I do not see a problem.\n\nI think I do. There is NO way to pass the filenames to git plumbing\n(only the diff-, or is git update-index --stdin -z also subject to\nglob expansion?) without being sure they are exactly they were on\nfilesystem. Quoting is just PITA for scripting.\n"}]}