{"thread":{"id":"33436","subject":"RFC: two minor tweaks to check-ignore to help git-annex assistant","startedAt":"2013-04-08T18:13:11Z","lastAt":"2013-04-29T22:55:25Z","messageCount":33,"participants":["Adam Spiers","Junio C Hamano","Jeff King","Aaron Schrab"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"213573","messageId":"20130408181311.GA14903@pacific.linksys.moosehall","threadId":"33436","inReplyTo":null,"subject":"RFC: two minor tweaks to check-ignore to help git-annex assistant","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2013-04-08T18:13:11Z","receivedAt":"2013-04-08T18:13:11Z","isPatch":false,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"Hi all,\n\nI was recently informed by the author of git-annex that my\nimplementation of git check-ignore has two minor deficiencies which\ncurrently prevent him from adding .gitignore support to the git-annex\nassistant (web UI):\n\n 1. When accepting a list of files to check via --stdin, no results\n are calculated until EOF is hit. This prevents it being used\n as a persistent background query process which streams results\n to its caller. (This is inconsistent with check-attr, which\n *does* support stream-like behaviour.)\n\n 2. Even if it did support streaming, the lack of output for files\n which don't match any ignore pattern make it impossible for the\n consumer to distinguish between the two cases a) the file\n doesn't match any pattern and b) it does match but the output\n hasn't arrived yet.\n\nBoth of these are pretty trivial to fix. The first is fixed by\nchanging check_ignore_stdin_paths() to invoke check_ignore() per input\nline, rather than collecting all input from STDIN and then invoking\ncheck_ignore() on the whole lot. (I have not implemented this yet,\nbut may well be able to do it this week, thanks to it being one of\nSUSE's hack weeks :-)\n\nI already have a rough fix for the second issue, but I wanted to\nsolicit feedback on the appropriate UI changes before proceeding much\nfurther. Does something like the below patch seem reasonable, modulo\nthe lack of tests? In case the UI changes I am proposing are not\nclear from the patch, here's some example output from running it\ninside a clone of the git source tree:\n\n $ git check-ignore -v -n foo.tar.{gz,bz2}\n .gitignore:203:*.tar.gz foo.tar.gz\n :: foo.tar.bz2\n\nSo the number of output fields does not change depending on whether\nthe pattern matches or not, and any caller can determine whether it\ndoes simply by checking whether the first field is non-empty.\n\nAlso, does it make sense to write a new test to accompany the fix to\nthe first (streaming) issue?\n\nThanks,\nAdam\n\n-- >8 --\nSubject: [PATCH] check-ignore: add -n / --non-matching option\n\nIf `-n` or `--non-matching` are specified, non-matching pathnames will\nalso be output, in which case all fields in each output record except\nfor will be empty. This can be useful when running\nnon-interactively, so that files can be incrementally streamed to\nSTDIN of a long-running check-ignore process, and for each of these\nfiles, STDOUT will indicate whether that file matched a pattern or\nnot. (Without this option, it would be impossible to tell whether the\nabsence of output for a given file meant that it didn't match any\npattern, or that the output hadn't been generated yet.)\n\nSigned-off-by: Adam Spiers \n---\n Documentation/git-check-ignore.txt | 15 +++++++++++++\n builtin/check-ignore.c | 43 ++++++++++++++++++++++++--------------\n 2 files changed, 42 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/git-check-ignore.txt b/Documentation/git-check-ignore.txt\nindex 854e4d0..7e3cabc 100644\n--- a/Documentation/git-check-ignore.txt\n+++ b/Documentation/git-check-ignore.txt\n@@ -39,6 +39,12 @@ OPTIONS\n \tbelow). If `--stdin` is also given, input paths are separated\n \twith a NUL character instead of a linefeed character.\n \n+-n, --non-matching::\n+\tShow given paths which don't match any pattern.\t This only\n+\tmakes sense when `--verbose` is enabled, otherwise it would\n+\tnot be possible to distinguish between paths which match a\n+\tpattern and those which don't.\n+\n OUTPUT\n ------\n \n@@ -65,6 +71,15 @@ are also used instead of colons and hard tabs:\n \n \n \n+If `-n` or `--non-matching` are specified, non-matching pathnames will\n+also be output, in which case all fields in each output record except\n+for will be empty. This can be useful when running\n+non-interactively, so that files can be incrementally streamed to\n+STDIN of a long-running check-ignore process, and for each of these\n+files, STDOUT will indicate whether that file matched a pattern or\n+not. (Without this option, it would be impossible to tell whether the\n+absence of output for a given file meant that it didn't match any\n+pattern, or that the output hadn't been generated yet.)\n \n EXIT STATUS\n -----------\ndiff --git a/builtin/check-ignore.c b/builtin/check-ignore.c\nindex 0240f99..498fd65 100644\n--- a/builtin/check-ignore.c\n+++ b/builtin/check-ignore.c\n@@ -5,7 +5,7 @@\n #include \"pathspec.h\"\n #include \"parse-options.h\"\n \n-static int quiet, verbose, stdin_paths;\n+static int quiet, verbose, stdin_paths, show_non_matching;\n static const char * const check_ignore_usage[] = {\n \"git check-ignore [options] pathname...\",\n \"git check-ignore [options] --stdin < \",\n@@ -22,21 +22,28 @@ static const struct option check_ignore_options[] = {\n \t\t N_(\"read file names from stdin\")),\n \tOPT_BOOLEAN('z', NULL, &null_term_line,\n \t\t N_(\"input paths are terminated by a null character\")),\n+\tOPT_BOOLEAN('n', \"non-matching\", &show_non_matching,\n+\t\t N_(\"show non-matching input paths\")),\n \tOPT_END()\n };\n \n static void output_exclude(const char *path, struct exclude *exclude)\n {\n-\tchar *bang = exclude->flags & EXC_FLAG_NEGATIVE ? \"!\" : \"\";\n-\tchar *slash = exclude->flags & EXC_FLAG_MUSTBEDIR ? \"/\" : \"\";\n+\tchar *bang = (exclude && exclude->flags & EXC_FLAG_NEGATIVE) ? \"!\" : \"\";\n+\tchar *slash = (exclude && exclude->flags & EXC_FLAG_MUSTBEDIR) ? \"/\" : \"\";\n \tif (!null_term_line) {\n \t\tif (!verbose) {\n \t\t\twrite_name_quoted(path, stdout, '\\n');\n \t\t} else {\n-\t\t\tquote_c_style(exclude->el->src, NULL, stdout, 0);\n-\t\t\tprintf(\":%d:%s%s%s\\t\",\n-\t\t\t exclude->srcpos,\n-\t\t\t bang, exclude->pattern, slash);\n+\t\t\tif (exclude) {\n+\t\t\t\tquote_c_style(exclude->el->src, NULL, stdout, 0);\n+\t\t\t\tprintf(\":%d:%s%s%s\\t\",\n+\t\t\t\t exclude->srcpos,\n+\t\t\t\t bang, exclude->pattern, slash);\n+\t\t\t}\n+\t\t\telse {\n+\t\t\t\tprintf(\"::\\t\");\n+\t\t\t}\n \t\t\tquote_c_style(path, NULL, stdout, 0);\n \t\t\tfputc('\\n', stdout);\n \t\t}\n@@ -44,11 +51,14 @@ static void output_exclude(const char *path, struct exclude *exclude)\n \t\tif (!verbose) {\n \t\t\tprintf(\"%s%c\", path, '\\0');\n \t\t} else {\n-\t\t\tprintf(\"%s%c%d%c%s%s%s%c%s%c\",\n-\t\t\t exclude->el->src, '\\0',\n-\t\t\t exclude->srcpos, '\\0',\n-\t\t\t bang, exclude->pattern, slash, '\\0',\n-\t\t\t path, '\\0');\n+\t\t\tif (exclude)\n+\t\t\t\tprintf(\"%s%c%d%c%s%s%s%c%s%c\",\n+\t\t\t\t exclude->el->src, '\\0',\n+\t\t\t\t exclude->srcpos, '\\0',\n+\t\t\t\t bang, exclude->pattern, slash, '\\0',\n+\t\t\t\t path, '\\0');\n+\t\t\telse\n+\t\t\t\tprintf(\"%c%c%c%s%c\", '\\0', '\\0', '\\0', path, '\\0');\n \t\t}\n \t}\n }\n@@ -92,11 +102,10 @@ static int check_ignore(const char *prefix, const char **pathspec)\n \t\tif (!seen[i]) {\n \t\t\texclude = last_exclude_matching_path(&check, full_path,\n \t\t\t\t\t\t\t -1, &dtype);\n-\t\t\tif (exclude) {\n-\t\t\t\tif (!quiet)\n-\t\t\t\t\toutput_exclude(path, exclude);\n+\t\t\tif (!quiet && (exclude || show_non_matching))\n+\t\t\t\toutput_exclude(path, exclude);\n+\t\t\tif (exclude)\n \t\t\t\tnum_ignored++;\n-\t\t\t}\n \t\t}\n \t}\n \tfree(seen);\n@@ -161,6 +170,8 @@ int cmd_check_ignore(int argc, const char **argv, const char *prefix)\n \t\tif (verbose)\n \t\t\tdie(_(\"cannot have both --quiet and --verbose\"));\n \t}\n+\tif (show_non_matching && !verbose)\n+\t\tdie(_(\"--non-matching is only valid with --verbose\"));\n \n \tif (stdin_paths) {\n \t\tnum_ignored = check_ignore_stdin_paths(prefix);\n-- \n1.8.2.242.g8617715.dirty\n"},{"id":"213619","messageId":"7vhajgvg8h.fsf@alter.siamese.dyndns.org","threadId":"33436","inReplyTo":"20130408181311.GA14903@pacific.linksys.moosehall","subject":"Re: RFC: two minor tweaks to check-ignore to help git-annex assistant","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-08T21:56:30Z","receivedAt":"2013-04-08T21:56:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Spiers writes:\n\n> I already have a rough fix for the second issue, but I wanted to\n> solicit feedback on the appropriate UI changes before proceeding much\n> further. Does something like the below patch seem reasonable, modulo\n> the lack of tests? In case the UI changes I am proposing are not\n> clear from the patch, here's some example output from running it\n> inside a clone of the git source tree:\n>\n> $ git check-ignore -v -n foo.tar.{gz,bz2}\n> .gitignore:203:*.tar.gz foo.tar.gz\n> :: foo.tar.bz2\n>\n> So the number of output fields does not change depending on whether\n> the pattern matches or not, and any caller can determine whether it\n> does simply by checking whether the first field is non-empty.\n\nHaven't looked at the proposed patch very carefully, but the design\nlooks sound. The above output screams \"empty! nothing!\", and I do\nnot think there is any other way :: will show up in that position.\n\n> Also, does it make sense to write a new test to accompany the fix to\n> the first (streaming) issue?\n\nWould it be tricky to write safely not to get stuck? You feed one\nline, stop feeding, while checking that the output has arrived, and\nthen kill the whole thing? Feels somewhat yucky, but sounds doable.\n"},{"id":"213625","messageId":"20130408222059.GA12454@sigill.intra.peff.net","threadId":"33436","inReplyTo":"20130408181311.GA14903@pacific.linksys.moosehall","subject":"Re: RFC: two minor tweaks to check-ignore to help git-annex assistant","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-04-08T22:20:59Z","receivedAt":"2013-04-08T22:20:59Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 08, 2013 at 07:13:11PM +0100, Adam Spiers wrote:\n\n> I was recently informed by the author of git-annex that my\n> implementation of git check-ignore has two minor deficiencies which\n> currently prevent him from adding .gitignore support to the git-annex\n> assistant (web UI):\n> \n> 1. When accepting a list of files to check via --stdin, no results\n> are calculated until EOF is hit. This prevents it being used\n> as a persistent background query process which streams results\n> to its caller. (This is inconsistent with check-attr, which\n> *does* support stream-like behaviour.)\n\nI think flushing on each line is reasonable, though you are also\nintroducing a deadlock possibility for callers which do not read back\nthe output in real-time. For example, if I write N paths out then read N\nignore-lines back in, I risk a situation where I am blocked on write()\nto check-ignore, and it is blocked on write back to me. Somebody has to\nbuffer (the pipe buffers give you some leeway, but they are limited).\n\nGiven how new check-ignore is, and that we have not advertised any\nparticular buffering scheme so far, it's probably OK to switch without\nworrying about breaking existing callers.\n\nBut if this is a mode of operation that we expect people to use (here\nand for check-attr), we should advertise the flushing behavior, and\nprobably warn about the deadlock (I don't think adding a \"--no-flush\"\noption is worth it, as it would just mean buffering in check-ignore,\nwhich the caller could just as easily do itself).\n\n-Peff\n"},{"id":"213882","messageId":"1365645575-11428-1-git-send-email-git@adamspiers.org","threadId":"33436","inReplyTo":"20130408181311.GA14903@pacific.linksys.moosehall","subject":"[PATCH 1/5] check-ignore: move setup into cmd_check_ignore()","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2013-04-11T01:59:31Z","receivedAt":"2013-04-11T01:59:31Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"Initialisation of the dir_struct and path_exclude_check structs was\npreviously done within check_ignore(). This was acceptable since\ncheck_ignore() was only called once per check-ignore invocation;\nhowever the next commit will convert it into an inner loop which is\ncalled once per line of STDIN when --stdin is given. Therefore moving\nthe initialisation code out into cmd_check_ignore() ensures that\ninitialisation is still only performed once per check-ignore\ninvocation, and consequently that the output is identical whether\npathspecs are provided as CLI arguments or via STDIN.\n\nSigned-off-by: Adam Spiers \n---\n builtin/check-ignore.c | 39 ++++++++++++++++++++-------------------\n 1 file changed, 20 insertions(+), 19 deletions(-)\n\ndiff --git a/builtin/check-ignore.c b/builtin/check-ignore.c\nindex 0240f99..0a4eef1 100644\n--- a/builtin/check-ignore.c\n+++ b/builtin/check-ignore.c\n@@ -53,30 +53,20 @@ static void output_exclude(const char *path, struct exclude *exclude)\n \t}\n }\n \n-static int check_ignore(const char *prefix, const char **pathspec)\n+static int check_ignore(struct path_exclude_check check,\n+\t\t\tconst char *prefix, const char **pathspec)\n {\n-\tstruct dir_struct dir;\n \tconst char *path, *full_path;\n \tchar *seen;\n \tint num_ignored = 0, dtype = DT_UNKNOWN, i;\n-\tstruct path_exclude_check check;\n \tstruct exclude *exclude;\n \n-\t/* read_cache() is only necessary so we can watch out for submodules. */\n-\tif (read_cache() < 0)\n-\t\tdie(_(\"index file corrupt\"));\n-\n-\tmemset(&dir, 0, sizeof(dir));\n-\tdir.flags |= DIR_COLLECT_IGNORED;\n-\tsetup_standard_excludes(&dir);\n-\n \tif (!pathspec || !*pathspec) {\n \t\tif (!quiet)\n \t\t\tfprintf(stderr, \"no pathspec given.\\n\");\n \t\treturn 0;\n \t}\n \n-\tpath_exclude_check_init(&check, &dir);\n \t/*\n \t * look for pathspecs matching entries in the index, since these\n \t * should not be ignored, in order to be consistent with\n@@ -100,13 +90,11 @@ static int check_ignore(const char *prefix, const char **pathspec)\n \t\t}\n \t}\n \tfree(seen);\n-\tclear_directory(&dir);\n-\tpath_exclude_check_clear(&check);\n \n \treturn num_ignored;\n }\n \n-static int check_ignore_stdin_paths(const char *prefix)\n+static int check_ignore_stdin_paths(struct path_exclude_check check, const char *prefix)\n {\n \tstruct strbuf buf, nbuf;\n \tchar **pathspec = NULL;\n@@ -129,17 +117,18 @@ static int check_ignore_stdin_paths(const char *prefix)\n \t}\n \tALLOC_GROW(pathspec, nr + 1, alloc);\n \tpathspec[nr] = NULL;\n-\tnum_ignored = check_ignore(prefix, (const char **)pathspec);\n+\tnum_ignored = check_ignore(check, prefix, (const char **)pathspec);\n \tmaybe_flush_or_die(stdout, \"attribute to stdout\");\n \tstrbuf_release(&buf);\n \tstrbuf_release(&nbuf);\n-\tfree(pathspec);\n \treturn num_ignored;\n }\n \n int cmd_check_ignore(int argc, const char **argv, const char *prefix)\n {\n \tint num_ignored;\n+\tstruct dir_struct dir;\n+\tstruct path_exclude_check check;\n \n \tgit_config(git_default_config, NULL);\n \n@@ -162,12 +151,24 @@ int cmd_check_ignore(int argc, const char **argv, const char *prefix)\n \t\t\tdie(_(\"cannot have both --quiet and --verbose\"));\n \t}\n \n+\t/* read_cache() is only necessary so we can watch out for submodules. */\n+\tif (read_cache() < 0)\n+\t\tdie(_(\"index file corrupt\"));\n+\n+\tmemset(&dir, 0, sizeof(dir));\n+\tdir.flags |= DIR_COLLECT_IGNORED;\n+\tsetup_standard_excludes(&dir);\n+\n+\tpath_exclude_check_init(&check, &dir);\n \tif (stdin_paths) {\n-\t\tnum_ignored = check_ignore_stdin_paths(prefix);\n+\t\tnum_ignored = check_ignore_stdin_paths(check, prefix);\n \t} else {\n-\t\tnum_ignored = check_ignore(prefix, argv);\n+\t\tnum_ignored = check_ignore(check, prefix, argv);\n \t\tmaybe_flush_or_die(stdout, \"ignore to stdout\");\n \t}\n \n+\tclear_directory(&dir);\n+\tpath_exclude_check_clear(&check);\n+\n \treturn !num_ignored;\n }\n-- \n1.8.2.1.347.g37e0606\n"},{"id":"213881","messageId":"1365645575-11428-2-git-send-email-git@adamspiers.org","threadId":"33436","inReplyTo":"1365645575-11428-1-git-send-email-git@adamspiers.org","subject":"[PATCH 2/5] check-ignore: allow incremental streaming of queries via --stdin","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2013-04-11T01:59:32Z","receivedAt":"2013-04-11T01:59:32Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"Some callers, such as the git-annex web assistant, find it useful to\ninvoke git check-ignore as a persistent background process, which can\nthen have queries fed to its STDIN at any point, and the corresponding\nresponse consumed from its STDOUT. For this we need to invoke\ncheck_ignore() once per line of standard input, and flush standard\noutput after each result.\n\nThe above use case suggests that empty STDIN is actually a reasonable\nscenario (e.g. when the caller doesn't know in advance whether any\nqueries need to be fed to the background process until after it's\nalready started), so we make the minor behavioural change that \"no\npathspec given.\" is no longer emitted in when STDIN is empty.\n\nEven though check_ignore() could now be changed to operate on a single\npathspec, we keep it operating on an array of pathspecs since that is\na more convenient way of consuming the existing pathspec API.\n\nSigned-off-by: Adam Spiers \n---\n builtin/check-ignore.c | 16 ++++++----------\n t/t0008-ignores.sh | 29 ++++++++++++++++++++++++-----\n 2 files changed, 30 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/check-ignore.c b/builtin/check-ignore.c\nindex 0a4eef1..ce4b1ad 100644\n--- a/builtin/check-ignore.c\n+++ b/builtin/check-ignore.c\n@@ -97,10 +97,9 @@ static int check_ignore(struct path_exclude_check check,\n static int check_ignore_stdin_paths(struct path_exclude_check check, const char *prefix)\n {\n \tstruct strbuf buf, nbuf;\n-\tchar **pathspec = NULL;\n-\tsize_t nr = 0, alloc = 0;\n+\tchar *pathspec[2] = { NULL, NULL };\n \tint line_termination = null_term_line ? 0 : '\\n';\n-\tint num_ignored;\n+\tint num_ignored = 0;\n \n \tstrbuf_init(&buf, 0);\n \tstrbuf_init(&nbuf, 0);\n@@ -111,14 +110,11 @@ static int check_ignore_stdin_paths(struct path_exclude_check check, const char\n \t\t\t\tdie(\"line is badly quoted\");\n \t\t\tstrbuf_swap(&buf, &nbuf);\n \t\t}\n-\t\tALLOC_GROW(pathspec, nr + 1, alloc);\n-\t\tpathspec[nr] = xcalloc(strlen(buf.buf) + 1, sizeof(*buf.buf));\n-\t\tstrcpy(pathspec[nr++], buf.buf);\n+\t\tpathspec[0] = xcalloc(strlen(buf.buf) + 1, sizeof(*buf.buf));\n+\t\tstrcpy(pathspec[0], buf.buf);\n+\t\tnum_ignored += check_ignore(check, prefix, (const char **)pathspec);\n+\t\tmaybe_flush_or_die(stdout, \"check-ignore to stdout\");\n \t}\n-\tALLOC_GROW(pathspec, nr + 1, alloc);\n-\tpathspec[nr] = NULL;\n-\tnum_ignored = check_ignore(check, prefix, (const char **)pathspec);\n-\tmaybe_flush_or_die(stdout, \"attribute to stdout\");\n \tstrbuf_release(&buf);\n \tstrbuf_release(&nbuf);\n \treturn num_ignored;\ndiff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh\nindex 9c1bde1..0dd3ef7 100755\n--- a/t/t0008-ignores.sh\n+++ b/t/t0008-ignores.sh\n@@ -189,11 +189,7 @@ test_expect_success_multi 'empty command line' '' '\n \n test_expect_success_multi '--stdin with empty STDIN' '' '\n \ttest_check_ignore \"--stdin\" 1 /dev/null 2>&1\n+\tthen\n+\t\ttest_set_prereq STDBUF\n+\tfi\n+'\n+\n+test_expect_success STDBUF 'streaming support for --stdin' '\n+\t(\n+\t\techo one\n+\t\tsleep 2\n+\t\techo two\n+\t) | stdbuf -oL git check-ignore -v -n --stdin >out &\n+\tpid=$! &&\n+\tsleep 1 &&\n+\tcat out &&\n+\tgrep \"^\\.gitignore:1:one\tone\" out &&\n+\ttest $( wc -l /dev/null\n+'\n \n test_done\n-- \n1.8.2.1.347.g37e0606\n"},{"id":"213880","messageId":"1365645575-11428-3-git-send-email-git@adamspiers.org","threadId":"33436","inReplyTo":"1365645575-11428-1-git-send-email-git@adamspiers.org","subject":"[PATCH 3/5] Documentation: add caveats about I/O buffering for check-{attr,ignore}","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2013-04-11T01:59:33Z","receivedAt":"2013-04-11T01:59:33Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"check-attr and check-ignore have the potential to deadlock callers\nwhich do not read back the output in real-time. For example, if a\ncaller writes N paths out and then reads N lines back in, it risks\nbecoming blocked on write() to check-*, and check-* is blocked on\nwrite back to the caller. Somebody has to buffer; the pipe buffers\nprovide some leeway, but they are limited.\n\nThanks to Peff for pointing this out:\n\n http://article.gmane.org/gmane.comp.version-control.git/220534\n\nSigned-off-by: Adam Spiers \n---\n Documentation/git-check-attr.txt | 5 +++++\n Documentation/git-check-ignore.txt | 5 +++++\n Documentation/git.txt | 16 +++++++++-------\n 3 files changed, 19 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-check-attr.txt b/Documentation/git-check-attr.txt\nindex 5abdbaa..a7be80d 100644\n--- a/Documentation/git-check-attr.txt\n+++ b/Documentation/git-check-attr.txt\n@@ -56,6 +56,11 @@ being queried and can be either:\n 'set';;\t\twhen the attribute is defined as true.\n ;;\twhen a value has been assigned to the attribute.\n \n+Buffering happens as documented under the `GIT_FLUSH` option in\n+linkgit:git[1]. The caller is responsible for avoiding deadlocks\n+caused by overfilling an input buffer or reading from an empty output\n+buffer.\n+\n EXAMPLES\n --------\n \ndiff --git a/Documentation/git-check-ignore.txt b/Documentation/git-check-ignore.txt\nindex 854e4d0..4014d28 100644\n--- a/Documentation/git-check-ignore.txt\n+++ b/Documentation/git-check-ignore.txt\n@@ -66,6 +66,11 @@ are also used instead of colons and hard tabs:\n \n \n \n+Buffering happens as documented under the `GIT_FLUSH` option in\n+linkgit:git[1]. The caller is responsible for avoiding deadlocks\n+caused by overfilling an input buffer or reading from an empty output\n+buffer.\n+\n EXIT STATUS\n -----------\n \ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 6a875f2..eecdb15 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -808,13 +808,15 @@ for further details.\n \n 'GIT_FLUSH'::\n \tIf this environment variable is set to \"1\", then commands such\n-\tas 'git blame' (in incremental mode), 'git rev-list', 'git log',\n-\tand 'git whatchanged' will force a flush of the output stream\n-\tafter each commit-oriented record have been flushed. If this\n-\tvariable is set to \"0\", the output of these commands will be done\n-\tusing completely buffered I/O. If this environment variable is\n-\tnot set, Git will choose buffered or record-oriented flushing\n-\tbased on whether stdout appears to be redirected to a file or not.\n+\tas 'git blame' (in incremental mode), 'git rev-list', 'git\n+\tlog', 'git check-attr', 'git check-ignore', and 'git\n+\twhatchanged' will force a flush of the output stream after\n+\teach commit-oriented record have been flushed. If this\n+\tvariable is set to \"0\", the output of these commands will be\n+\tdone using completely buffered I/O. If this environment\n+\tvariable is not set, Git will choose buffered or\n+\trecord-oriented flushing based on whether stdout appears to be\n+\tredirected to a file or not.\n \n 'GIT_TRACE'::\n \tIf this variable is set to \"1\", \"2\" or \"true\" (comparison\n-- \n1.8.2.1.347.g37e0606\n"},{"id":"213884","messageId":"1365645575-11428-4-git-send-email-git@adamspiers.org","threadId":"33436","inReplyTo":"1365645575-11428-1-git-send-email-git@adamspiers.org","subject":"[PATCH 4/5] t0008: remove duplicated test fixture data","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2013-04-11T01:59:34Z","receivedAt":"2013-04-11T01:59:34Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"The expected contents of STDOUT for the final --stdin tests can be\nderived from the expected contents of STDOUT for the same tests when\n--verbose is given, in the same way that test_expect_success_multi\nderives this for earlier tests.\n\nSigned-off-by: Adam Spiers \n---\n t/t0008-ignores.sh | 16 +---------------\n 1 file changed, 1 insertion(+), 15 deletions(-)\n\ndiff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh\nindex 0dd3ef7..80b731a 100755\n--- a/t/t0008-ignores.sh\n+++ b/t/t0008-ignores.sh\n@@ -571,21 +571,6 @@ cat <<-\\EOF >stdin\n \tb/globaltwo\n \t../b/globaltwo\n EOF\n-cat <<-\\EOF >expected-default\n-\t../one\n-\tone\n-\tb/on\n-\tb/one\n-\tb/one one\n-\tb/one two\n-\t\"b/one\\\"three\"\n-\tb/two\n-\tb/twooo\n-\t../globaltwo\n-\tglobaltwo\n-\tb/globaltwo\n-\t../b/globaltwo\n-EOF\n cat <<-EOF >expected-verbose\n \t.gitignore:1:one\t../one\n \t.gitignore:1:one\tone\n@@ -601,6 +586,7 @@ cat <<-EOF >expected-verbose\n \t$global_excludes:2:!globaltwo\tb/globaltwo\n \t$global_excludes:2:!globaltwo\t../b/globaltwo\n EOF\n+sed -e 's/.*\t//' expected-verbose >expected-default\n \n sed -e 's/^\"//' -e 's/\\\\//' -e 's/\"$//' stdin | \\\n \ttr \"\\n\" \"\\0\" >stdin0\n-- \n1.8.2.1.347.g37e0606\n"},{"id":"213883","messageId":"1365645575-11428-5-git-send-email-git@adamspiers.org","threadId":"33436","inReplyTo":"1365645575-11428-1-git-send-email-git@adamspiers.org","subject":"[PATCH 5/5] check-ignore: add -n / --non-matching option","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2013-04-11T01:59:35Z","receivedAt":"2013-04-11T01:59:35Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"If `-n` or `--non-matching` are specified, non-matching pathnames will\nalso be output, in which case all fields in each output record except\nfor will be empty. This can be useful when running\ncheck-ignore as a background process, so that files can be\nincrementally streamed to STDIN, and for each of these files, STDOUT\nwill indicate whether that file matched a pattern or not. (Without\nthis option, it would be impossible to tell whether the absence of\noutput for a given file meant that it didn't match any pattern, or\nthat the result simply hadn't been flushed to STDOUT yet.)\n\nSigned-off-by: Adam Spiers \n---\n Documentation/git-check-ignore.txt | 15 +++++\n builtin/check-ignore.c | 46 ++++++++------\n t/t0008-ignores.sh | 122 +++++++++++++++++++++++++++----------\n 3 files changed, 134 insertions(+), 49 deletions(-)\n\ndiff --git a/Documentation/git-check-ignore.txt b/Documentation/git-check-ignore.txt\nindex 4014d28..8e1f7ab 100644\n--- a/Documentation/git-check-ignore.txt\n+++ b/Documentation/git-check-ignore.txt\n@@ -39,6 +39,12 @@ OPTIONS\n \tbelow). If `--stdin` is also given, input paths are separated\n \twith a NUL character instead of a linefeed character.\n \n+-n, --non-matching::\n+\tShow given paths which don't match any pattern.\t This only\n+\tmakes sense when `--verbose` is enabled, otherwise it would\n+\tnot be possible to distinguish between paths which match a\n+\tpattern and those which don't.\n+\n OUTPUT\n ------\n \n@@ -65,6 +71,15 @@ are also used instead of colons and hard tabs:\n \n \n \n+If `-n` or `--non-matching` are specified, non-matching pathnames will\n+also be output, in which case all fields in each output record except\n+for will be empty. This can be useful when running\n+non-interactively, so that files can be incrementally streamed to\n+STDIN of a long-running check-ignore process, and for each of these\n+files, STDOUT will indicate whether that file matched a pattern or\n+not. (Without this option, it would be impossible to tell whether the\n+absence of output for a given file meant that it didn't match any\n+pattern, or that the output hadn't been generated yet.)\n \n Buffering happens as documented under the `GIT_FLUSH` option in\n linkgit:git[1]. The caller is responsible for avoiding deadlocks\ndiff --git a/builtin/check-ignore.c b/builtin/check-ignore.c\nindex ce4b1ad..03ddada 100644\n--- a/builtin/check-ignore.c\n+++ b/builtin/check-ignore.c\n@@ -5,7 +5,7 @@\n #include \"pathspec.h\"\n #include \"parse-options.h\"\n \n-static int quiet, verbose, stdin_paths;\n+static int quiet, verbose, stdin_paths, show_non_matching;\n static const char * const check_ignore_usage[] = {\n \"git check-ignore [options] pathname...\",\n \"git check-ignore [options] --stdin < \",\n@@ -22,21 +22,28 @@ static const struct option check_ignore_options[] = {\n \t\t N_(\"read file names from stdin\")),\n \tOPT_BOOLEAN('z', NULL, &null_term_line,\n \t\t N_(\"input paths are terminated by a null character\")),\n+\tOPT_BOOLEAN('n', \"non-matching\", &show_non_matching,\n+\t\t N_(\"show non-matching input paths\")),\n \tOPT_END()\n };\n \n static void output_exclude(const char *path, struct exclude *exclude)\n {\n-\tchar *bang = exclude->flags & EXC_FLAG_NEGATIVE ? \"!\" : \"\";\n-\tchar *slash = exclude->flags & EXC_FLAG_MUSTBEDIR ? \"/\" : \"\";\n+\tchar *bang = (exclude && exclude->flags & EXC_FLAG_NEGATIVE) ? \"!\" : \"\";\n+\tchar *slash = (exclude && exclude->flags & EXC_FLAG_MUSTBEDIR) ? \"/\" : \"\";\n \tif (!null_term_line) {\n \t\tif (!verbose) {\n \t\t\twrite_name_quoted(path, stdout, '\\n');\n \t\t} else {\n-\t\t\tquote_c_style(exclude->el->src, NULL, stdout, 0);\n-\t\t\tprintf(\":%d:%s%s%s\\t\",\n-\t\t\t exclude->srcpos,\n-\t\t\t bang, exclude->pattern, slash);\n+\t\t\tif (exclude) {\n+\t\t\t\tquote_c_style(exclude->el->src, NULL, stdout, 0);\n+\t\t\t\tprintf(\":%d:%s%s%s\\t\",\n+\t\t\t\t exclude->srcpos,\n+\t\t\t\t bang, exclude->pattern, slash);\n+\t\t\t}\n+\t\t\telse {\n+\t\t\t\tprintf(\"::\\t\");\n+\t\t\t}\n \t\t\tquote_c_style(path, NULL, stdout, 0);\n \t\t\tfputc('\\n', stdout);\n \t\t}\n@@ -44,11 +51,14 @@ static void output_exclude(const char *path, struct exclude *exclude)\n \t\tif (!verbose) {\n \t\t\tprintf(\"%s%c\", path, '\\0');\n \t\t} else {\n-\t\t\tprintf(\"%s%c%d%c%s%s%s%c%s%c\",\n-\t\t\t exclude->el->src, '\\0',\n-\t\t\t exclude->srcpos, '\\0',\n-\t\t\t bang, exclude->pattern, slash, '\\0',\n-\t\t\t path, '\\0');\n+\t\t\tif (exclude)\n+\t\t\t\tprintf(\"%s%c%d%c%s%s%s%c%s%c\",\n+\t\t\t\t exclude->el->src, '\\0',\n+\t\t\t\t exclude->srcpos, '\\0',\n+\t\t\t\t bang, exclude->pattern, slash, '\\0',\n+\t\t\t\t path, '\\0');\n+\t\t\telse\n+\t\t\t\tprintf(\"%c%c%c%s%c\", '\\0', '\\0', '\\0', path, '\\0');\n \t\t}\n \t}\n }\n@@ -79,15 +89,15 @@ static int check_ignore(struct path_exclude_check check,\n \t\t\t\t\t? strlen(prefix) : 0, path);\n \t\tfull_path = check_path_for_gitlink(full_path);\n \t\tdie_if_path_beyond_symlink(full_path, prefix);\n+\t\texclude = NULL;\n \t\tif (!seen[i]) {\n \t\t\texclude = last_exclude_matching_path(&check, full_path,\n \t\t\t\t\t\t\t -1, &dtype);\n-\t\t\tif (exclude) {\n-\t\t\t\tif (!quiet)\n-\t\t\t\t\toutput_exclude(path, exclude);\n-\t\t\t\tnum_ignored++;\n-\t\t\t}\n \t\t}\n+\t\tif (!quiet && (exclude || show_non_matching))\n+\t\t\toutput_exclude(path, exclude);\n+\t\tif (exclude)\n+\t\t\tnum_ignored++;\n \t}\n \tfree(seen);\n \n@@ -146,6 +156,8 @@ int cmd_check_ignore(int argc, const char **argv, const char *prefix)\n \t\tif (verbose)\n \t\t\tdie(_(\"cannot have both --quiet and --verbose\"));\n \t}\n+\tif (show_non_matching && !verbose)\n+\t\tdie(_(\"--non-matching is only valid with --verbose\"));\n \n \t/* read_cache() is only necessary so we can watch out for submodules. */\n \tif (read_cache() < 0)\ndiff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh\nindex 80b731a..86b8bf8 100755\n--- a/t/t0008-ignores.sh\n+++ b/t/t0008-ignores.sh\n@@ -66,16 +66,23 @@ test_check_ignore () {\n \n \tinit_vars &&\n \trm -f \"$HOME/stdout\" \"$HOME/stderr\" \"$HOME/cmd\" &&\n-\techo git $global_args check-ignore $quiet_opt $verbose_opt $args \\\n+\techo git $global_args check-ignore $quiet_opt $verbose_opt $non_matching_opt $args \\\n \t\t>\"$HOME/cmd\" &&\n+\techo \"$expect_code\" >\"$HOME/expected-exit-code\" &&\n \ttest_expect_code \"$expect_code\" \\\n-\t\tgit $global_args check-ignore $quiet_opt $verbose_opt $args \\\n+\t\tgit $global_args check-ignore $quiet_opt $verbose_opt $non_matching_opt $args \\\n \t\t>\"$HOME/stdout\" 2>\"$HOME/stderr\" &&\n \ttest_cmp \"$HOME/expected-stdout\" \"$HOME/stdout\" &&\n \tstderr_empty_on_success \"$expect_code\"\n }\n \n-# Runs the same code with 3 different levels of output verbosity,\n+# Runs the same code with 4 different levels of output verbosity:\n+#\n+# 1. with -q / --quiet\n+# 2. with default verbosity\n+# 3. with -v / --verbose\n+# 4. with -v / --verbose, *and* -n / --non-matching\n+#\n # expecting success each time. Takes advantage of the fact that\n # check-ignore --verbose output is the same as normal output except\n # for the extra first column.\n@@ -83,7 +90,9 @@ test_check_ignore () {\n # Arguments:\n # - (optional) prereqs for this test, e.g. 'SYMLINKS'\n # - test name\n-# - output to expect from -v / --verbose mode\n+# - output to expect from the fourth verbosity mode (the output\n+# from the other verbosity modes is automatically inferred\n+# from this value)\n # - code to run (should invoke test_check_ignore)\n test_expect_success_multi () {\n \tprereq=\n@@ -92,8 +101,9 @@ test_expect_success_multi () {\n \t\tprereq=$1\n \t\tshift\n \tfi\n-\ttestname=\"$1\" expect_verbose=\"$2\" code=\"$3\"\n+\ttestname=\"$1\" expect_all=\"$2\" code=\"$3\"\n \n+\texpect_verbose=$( echo \"$expect_all\" | grep -v '^::\t' )\n \texpect=$( echo \"$expect_verbose\" | sed -e 's/.*\t//' )\n \n \ttest_expect_success $prereq \"$testname\" '\n@@ -101,23 +111,40 @@ test_expect_success_multi () {\n \t\teval \"$code\"\n \t'\n \n-\tfor quiet_opt in '-q' '--quiet'\n-\tdo\n-\t\ttest_expect_success $prereq \"$testname${quiet_opt:+ with $quiet_opt}\" \"\n+\t# --quiet is only valid when a single pattern is passed\n+\tif test $( echo \"$expect_all\" | wc -l ) = 1\n+\tthen\n+\t\tfor quiet_opt in '-q' '--quiet'\n+\t\tdo\n+\t\t\ttest_expect_success $prereq \"$testname${quiet_opt:+ with $quiet_opt}\" \"\n \t\t\texpect '' &&\n \t\t\t$code\n \t\t\"\n-\tdone\n-\tquiet_opt=\n+\t\tdone\n+\t\tquiet_opt=\n+\tfi\n \n \tfor verbose_opt in '-v' '--verbose'\n \tdo\n-\t\ttest_expect_success $prereq \"$testname${verbose_opt:+ with $verbose_opt}\" \"\n-\t\t\texpect '$expect_verbose' &&\n-\t\t\t$code\n-\t\t\"\n+\t\tfor non_matching_opt in '' ' -n' ' --non-matching'\n+\t\tdo\n+\t\t\tif test -n \"$non_matching_opt\"\n+\t\t\tthen\n+\t\t\t\tmy_expect=\"$expect_all\"\n+\t\t\telse\n+\t\t\t\tmy_expect=\"$expect_verbose\"\n+\t\t\tfi\n+\n+\t\t\ttest_code=\"\n+\t\t\t\texpect '$my_expect' &&\n+\t\t\t\t$code\n+\t\t\t\"\n+\t\t\topts=\"$verbose_opt$non_matching_opt\"\n+\t\t\ttest_expect_success $prereq \"$testname${opts:+ with $opts}\" \"$test_code\"\n+\t\tdone\n \tdone\n \tverbose_opt=\n+\tnon_matching_opt=\n }\n \n test_expect_success 'setup' '\n@@ -178,7 +205,7 @@ test_expect_success 'setup' '\n #\n # test invalid inputs\n \n-test_expect_success_multi '. corner-case' '' '\n+test_expect_success_multi '. corner-case' '::\t.' '\n \ttest_check_ignore . 1\n '\n \n@@ -272,27 +299,39 @@ do\n \t\twhere=\"in subdir $subdir\"\n \tfi\n \n-\ttest_expect_success_multi \"non-existent file $where not ignored\" '' \"\n-\t\ttest_check_ignore '${subdir}non-existent' 1\n-\t\"\n+\ttest_expect_success_multi \"non-existent file $where not ignored\" \\\n+\t\t\"::\t${subdir}non-existent\" \\\n+\t\t\"test_check_ignore '${subdir}non-existent' 1\"\n \n \ttest_expect_success_multi \"non-existent file $where ignored\" \\\n-\t\t\".gitignore:1:one\t${subdir}one\" \"\n-\t\ttest_check_ignore '${subdir}one'\n-\t\"\n+\t\t\".gitignore:1:one\t${subdir}one\" \\\n+\t\t\"test_check_ignore '${subdir}one'\"\n \n-\ttest_expect_success_multi \"existing untracked file $where not ignored\" '' \"\n-\t\ttest_check_ignore '${subdir}not-ignored' 1\n-\t\"\n+\ttest_expect_success_multi \"existing untracked file $where not ignored\" \\\n+\t\t\"::\t${subdir}not-ignored\" \\\n+\t\t\"test_check_ignore '${subdir}not-ignored' 1\"\n \n-\ttest_expect_success_multi \"existing tracked file $where not ignored\" '' \"\n-\t\ttest_check_ignore '${subdir}ignored-but-in-index' 1\n-\t\"\n+\ttest_expect_success_multi \"existing tracked file $where not ignored\" \\\n+\t\t\"::\t${subdir}ignored-but-in-index\" \\\n+\t\t\"test_check_ignore '${subdir}ignored-but-in-index' 1\"\n \n \ttest_expect_success_multi \"existing untracked file $where ignored\" \\\n-\t\t\".gitignore:2:ignored-*\t${subdir}ignored-and-untracked\" \"\n-\t\ttest_check_ignore '${subdir}ignored-and-untracked'\n-\t\"\n+\t\t\".gitignore:2:ignored-*\t${subdir}ignored-and-untracked\" \\\n+\t\t\"test_check_ignore '${subdir}ignored-and-untracked'\"\n+\n+\ttest_expect_success_multi \"mix of file types $where\" \\\n+\"::\t${subdir}non-existent\n+.gitignore:1:one\t${subdir}one\n+::\t${subdir}not-ignored\n+::\t${subdir}ignored-but-in-index\n+.gitignore:2:ignored-*\t${subdir}ignored-and-untracked\" \\\n+\t\t\"test_check_ignore '\n+\t\t\t${subdir}non-existent\n+\t\t\t${subdir}one\n+\t\t\t${subdir}not-ignored\n+\t\t\t${subdir}ignored-but-in-index\n+\t\t\t${subdir}ignored-and-untracked'\n+\t\t\"\n done\n \n # Having established the above, from now on we mostly test against\n@@ -387,7 +426,7 @@ test_expect_success 'cd to ignored sub-directory with -v' '\n #\n # test handling of symlinks\n \n-test_expect_success_multi SYMLINKS 'symlink' '' '\n+test_expect_success_multi SYMLINKS 'symlink' '::\ta/symlink' '\n \ttest_check_ignore \"a/symlink\" 1\n '\n \n@@ -570,22 +609,33 @@ cat <<-\\EOF >stdin\n \tglobaltwo\n \tb/globaltwo\n \t../b/globaltwo\n+\tc/not-ignored\n EOF\n-cat <<-EOF >expected-verbose\n+# N.B. we deliberately end STDIN with a non-matching pattern in order\n+# to test that the exit code indicates that one or more of the\n+# provided paths is ignored - in other words, that it represents an\n+# aggregation of all the results, not just the final result.\n+\n+cat <<-EOF >expected-all\n \t.gitignore:1:one\t../one\n+\t::\t../not-ignored\n \t.gitignore:1:one\tone\n+\t::\tnot-ignored\n \ta/b/.gitignore:8:!on*\tb/on\n \ta/b/.gitignore:8:!on*\tb/one\n \ta/b/.gitignore:8:!on*\tb/one one\n \ta/b/.gitignore:8:!on*\tb/one two\n \ta/b/.gitignore:8:!on*\t\"b/one\\\"three\"\n \ta/b/.gitignore:9:!two\tb/two\n+\t::\tb/not-ignored\n \ta/.gitignore:1:two*\tb/twooo\n \t$global_excludes:2:!globaltwo\t../globaltwo\n \t$global_excludes:2:!globaltwo\tglobaltwo\n \t$global_excludes:2:!globaltwo\tb/globaltwo\n \t$global_excludes:2:!globaltwo\t../b/globaltwo\n+\t::\tc/not-ignored\n EOF\n+grep -v '^::\t' expected-all >expected-verbose\n sed -e 's/.*\t//' expected-verbose >expected-default\n \n sed -e 's/^\"//' -e 's/\\\\//' -e 's/\"$//' stdin | \\\n@@ -611,6 +661,14 @@ test_expect_success '--stdin from subdirectory with -v' '\n \t)\n '\n \n+test_expect_success '--stdin from subdirectory with -v -n' '\n+\texpect_from_stdin Initialisation of the dir_struct and path_exclude_check structs was\n> previously done within check_ignore(). This was acceptable since\n> check_ignore() was only called once per check-ignore invocation;\n> however the next commit will convert it into an inner loop which is\n> called once per line of STDIN when --stdin is given. Therefore moving\n> the initialisation code out into cmd_check_ignore() ensures that\n> initialisation is still only performed once per check-ignore\n> invocation, and consequently that the output is identical whether\n> pathspecs are provided as CLI arguments or via STDIN.\n> \n> Signed-off-by: Adam Spiers \n> ---\n> builtin/check-ignore.c | 39 ++++++++++++++++++++-------------------\n> 1 file changed, 20 insertions(+), 19 deletions(-)\n> \n> diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c\n> index 0240f99..0a4eef1 100644\n> --- a/builtin/check-ignore.c\n> +++ b/builtin/check-ignore.c\n> @@ -53,30 +53,20 @@ static void output_exclude(const char *path, struct exclude *exclude)\n> \t}\n> }\n> \n> -static int check_ignore(const char *prefix, const char **pathspec)\n> +static int check_ignore(struct path_exclude_check check,\n> +\t\t\tconst char *prefix, const char **pathspec)\n\nDid you mean to pass the struct by value here? If it is truly a per-path\nvalue, shouldn't it be declared and initialized inside here? Otherwise\nyou risk one invocation munging things that the struct points to, but\nthe caller's copy does not know about the change.\n\nIn particular, I see that the struct includes a strbuf. What happens\nwhen one invocation of check_ignore grows the strbuf, then returns? The\ncopy of the struct in the caller will not know that the buffer it is\npointing to is now bogus.\n\n> -static int check_ignore_stdin_paths(const char *prefix)\n> +static int check_ignore_stdin_paths(struct path_exclude_check check, const char *prefix)\n\nDitto here.\n\n-Peff\n"},{"id":"213902","messageId":"20130411053145.GB28915@sigill.intra.peff.net","threadId":"33436","inReplyTo":"1365645575-11428-2-git-send-email-git@adamspiers.org","subject":"Re: [PATCH 2/5] check-ignore: allow incremental streaming of queries via --stdin","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-04-11T05:31:45Z","receivedAt":"2013-04-11T05:31:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 11, 2013 at 02:59:32AM +0100, Adam Spiers wrote:\n\n> @@ -111,14 +110,11 @@ static int check_ignore_stdin_paths(struct path_exclude_check check, const char\n> \t\t\t\tdie(\"line is badly quoted\");\n> \t\t\tstrbuf_swap(&buf, &nbuf);\n> \t\t}\n> -\t\tALLOC_GROW(pathspec, nr + 1, alloc);\n> -\t\tpathspec[nr] = xcalloc(strlen(buf.buf) + 1, sizeof(*buf.buf));\n> -\t\tstrcpy(pathspec[nr++], buf.buf);\n> +\t\tpathspec[0] = xcalloc(strlen(buf.buf) + 1, sizeof(*buf.buf));\n> +\t\tstrcpy(pathspec[0], buf.buf);\n> +\t\tnum_ignored += check_ignore(check, prefix, (const char **)pathspec);\n> +\t\tmaybe_flush_or_die(stdout, \"check-ignore to stdout\");\n\nNow that you are not storing the whole pathspec at once, the pathspec\nbuffer only needs to be valid for the length of check_ignore, right?\nThat means you can drop this extra copy and just pass in buf.buf:\n\n pathspec[0] = buf.buf;\n num_ignored += check_ignore(check, prefix, pathspec);\n\n> +test_expect_success 'setup: have stdbuf?' '\n> +\tif which stdbuf >/dev/null 2>&1\n> +\tthen\n> +\t\ttest_set_prereq STDBUF\n> +\tfi\n> +'\n\nHmm. Today I learned about stdbuf. :)\n\n-Peff\n"},{"id":"213903","messageId":"20130411053154.GD27795@sigill.intra.peff.net","threadId":"33436","inReplyTo":"1365645575-11428-3-git-send-email-git@adamspiers.org","subject":"Re: [PATCH 3/5] Documentation: add caveats about I/O buffering for check-{attr,ignore}","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-04-11T05:31:54Z","receivedAt":"2013-04-11T05:31:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 11, 2013 at 02:59:33AM +0100, Adam Spiers wrote:\n\n> check-attr and check-ignore have the potential to deadlock callers\n> which do not read back the output in real-time. For example, if a\n> caller writes N paths out and then reads N lines back in, it risks\n> becoming blocked on write() to check-*, and check-* is blocked on\n> write back to the caller. Somebody has to buffer; the pipe buffers\n> provide some leeway, but they are limited.\n> \n> Thanks to Peff for pointing this out:\n> \n> http://article.gmane.org/gmane.comp.version-control.git/220534\n> \n> Signed-off-by: Adam Spiers \n\nThanks, I think the documentation changes look sane.\n\n-Peff\n"},{"id":"213922","messageId":"20130411105514.GA24296@pacific.linksys.moosehall","threadId":"33436","inReplyTo":"20130411053145.GB28915@sigill.intra.peff.net","subject":"Re: [PATCH 2/5] check-ignore: allow incremental streaming of queries via --stdin","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2013-04-11T10:55:14Z","receivedAt":"2013-04-11T10:55:14Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Thu, Apr 11, 2013 at 01:31:45AM -0400, Jeff King wrote:\n> On Thu, Apr 11, 2013 at 02:59:32AM +0100, Adam Spiers wrote:\n> \n> > @@ -111,14 +110,11 @@ static int check_ignore_stdin_paths(struct path_exclude_check check, const char\n> > \t\t\t\tdie(\"line is badly quoted\");\n> > \t\t\tstrbuf_swap(&buf, &nbuf);\n> > \t\t}\n> > -\t\tALLOC_GROW(pathspec, nr + 1, alloc);\n> > -\t\tpathspec[nr] = xcalloc(strlen(buf.buf) + 1, sizeof(*buf.buf));\n> > -\t\tstrcpy(pathspec[nr++], buf.buf);\n> > +\t\tpathspec[0] = xcalloc(strlen(buf.buf) + 1, sizeof(*buf.buf));\n> > +\t\tstrcpy(pathspec[0], buf.buf);\n> > +\t\tnum_ignored += check_ignore(check, prefix, (const char **)pathspec);\n> > +\t\tmaybe_flush_or_die(stdout, \"check-ignore to stdout\");\n> \n> Now that you are not storing the whole pathspec at once, the pathspec\n> buffer only needs to be valid for the length of check_ignore, right?\n> That means you can drop this extra copy and just pass in buf.buf:\n> \n> pathspec[0] = buf.buf;\n> num_ignored += check_ignore(check, prefix, pathspec);\n\nOops, good point - thanks. I've made that change.\n\n> > +test_expect_success 'setup: have stdbuf?' '\n> > +\tif which stdbuf >/dev/null 2>&1\n> > +\tthen\n> > +\t\ttest_set_prereq STDBUF\n> > +\tfi\n> > +'\n> \n> Hmm. Today I learned about stdbuf. :)\n\nYeah, it's a relatively recent addition to coreutils.\n"},{"id":"213924","messageId":"20130411110511.GB24296@pacific.linksys.moosehall","threadId":"33436","inReplyTo":"20130411052553.GA28915@sigill.intra.peff.net","subject":"Re: [PATCH 1/5] check-ignore: move setup into cmd_check_ignore()","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2013-04-11T11:05:11Z","receivedAt":"2013-04-11T11:05:11Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Thu, Apr 11, 2013 at 01:25:53AM -0400, Jeff King wrote:\n> On Thu, Apr 11, 2013 at 02:59:31AM +0100, Adam Spiers wrote:\n> > -static int check_ignore(const char *prefix, const char **pathspec)\n> > +static int check_ignore(struct path_exclude_check check,\n> > +\t\t\tconst char *prefix, const char **pathspec)\n> \n> Did you mean to pass the struct by value here? If it is truly a per-path\n> value, shouldn't it be declared and initialized inside here? Otherwise\n> you risk one invocation munging things that the struct points to, but\n> the caller's copy does not know about the change.\n> \n> In particular, I see that the struct includes a strbuf. What happens\n> when one invocation of check_ignore grows the strbuf, then returns? The\n> copy of the struct in the caller will not know that the buffer it is\n> pointing to is now bogus.\n> \n> > -static int check_ignore_stdin_paths(const char *prefix)\n> > +static int check_ignore_stdin_paths(struct path_exclude_check check, const char *prefix)\n> \n> Ditto here.\n\nIt's not a per-path value; it's supposed to be reused across checks\nfor multiple paths, as explained in the comments above\nlast_exclude_matching_path():\n\n ...\n * A path to a directory known to be excluded is left in check->path to\n * optimize for repeated checks for files in the same excluded directory.\n */\n struct exclude *last_exclude_matching_path(struct path_exclude_check *check,\n ...\n\nSo I think you're probably right that there is potential for\ncheck->path to become effectively corrupted due to the caller not\nseeing the reallocation. I'll change this too.\n"},{"id":"213925","messageId":"20130411112000.GC24296@pacific.linksys.moosehall","threadId":"33436","inReplyTo":"1365645575-11428-2-git-send-email-git@adamspiers.org","subject":"Re: [PATCH 2/5] check-ignore: allow incremental streaming of queries via --stdin","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2013-04-11T11:20:00Z","receivedAt":"2013-04-11T11:20:00Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Thu, Apr 11, 2013 at 02:59:32AM +0100, Adam Spiers wrote:\n> +test_expect_success STDBUF 'streaming support for --stdin' '\n> +\t(\n> +\t\techo one\n> +\t\tsleep 2\n> +\t\techo two\n> +\t) | stdbuf -oL git check-ignore -v -n --stdin >out &\n\nI just noticed that this patch precedes the one in the same series\nwhich adds -n support. I'll reorder them accordingly to avoid\nbreaking git bisect.\n"},{"id":"213927","messageId":"1365681913-7059-1-git-send-email-git@adamspiers.org","threadId":"33436","inReplyTo":"20130411110511.GB24296@pacific.linksys.moosehall","subject":"[PATCH v2 1/5] t0008: remove duplicated test fixture data","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2013-04-11T12:05:09Z","receivedAt":"2013-04-11T12:05:09Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"The expected contents of STDOUT for the final --stdin tests can be\nderived from the expected contents of STDOUT for the same tests when\n--verbose is given, in the same way that test_expect_success_multi\nderives this for earlier tests.\n\nSigned-off-by: Adam Spiers \n---\n t/t0008-ignores.sh | 16 +---------------\n 1 file changed, 1 insertion(+), 15 deletions(-)\n\ndiff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh\nindex 9c1bde1..314a86d 100755\n--- a/t/t0008-ignores.sh\n+++ b/t/t0008-ignores.sh\n@@ -575,21 +575,6 @@ cat <<-\\EOF >stdin\n \tb/globaltwo\n \t../b/globaltwo\n EOF\n-cat <<-\\EOF >expected-default\n-\t../one\n-\tone\n-\tb/on\n-\tb/one\n-\tb/one one\n-\tb/one two\n-\t\"b/one\\\"three\"\n-\tb/two\n-\tb/twooo\n-\t../globaltwo\n-\tglobaltwo\n-\tb/globaltwo\n-\t../b/globaltwo\n-EOF\n cat <<-EOF >expected-verbose\n \t.gitignore:1:one\t../one\n \t.gitignore:1:one\tone\n@@ -605,6 +590,7 @@ cat <<-EOF >expected-verbose\n \t$global_excludes:2:!globaltwo\tb/globaltwo\n \t$global_excludes:2:!globaltwo\t../b/globaltwo\n EOF\n+sed -e 's/.*\t//' expected-verbose >expected-default\n \n sed -e 's/^\"//' -e 's/\\\\//' -e 's/\"$//' stdin | \\\n \ttr \"\\n\" \"\\0\" >stdin0\n-- \n1.8.2.1.342.gfa7285d\n"},{"id":"213929","messageId":"1365681913-7059-2-git-send-email-git@adamspiers.org","threadId":"33436","inReplyTo":"1365681913-7059-1-git-send-email-git@adamspiers.org","subject":"[PATCH v2 2/5] check-ignore: add -n / --non-matching option","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2013-04-11T12:05:10Z","receivedAt":"2013-04-11T12:05:10Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"If `-n` or `--non-matching` are specified, non-matching pathnames will\nalso be output, in which case all fields in each output record except\nfor will be empty. This can be useful when running\ncheck-ignore as a background process, so that files can be\nincrementally streamed to STDIN, and for each of these files, STDOUT\nwill indicate whether that file matched a pattern or not. (Without\nthis option, it would be impossible to tell whether the absence of\noutput for a given file meant that it didn't match any pattern, or\nthat the result simply hadn't been flushed to STDOUT yet.)\n\nSigned-off-by: Adam Spiers \n---\n Documentation/git-check-ignore.txt | 15 +++++\n builtin/check-ignore.c | 46 ++++++++------\n t/t0008-ignores.sh | 122 +++++++++++++++++++++++++++----------\n 3 files changed, 134 insertions(+), 49 deletions(-)\n\ndiff --git a/Documentation/git-check-ignore.txt b/Documentation/git-check-ignore.txt\nindex 854e4d0..7e3cabc 100644\n--- a/Documentation/git-check-ignore.txt\n+++ b/Documentation/git-check-ignore.txt\n@@ -39,6 +39,12 @@ OPTIONS\n \tbelow). If `--stdin` is also given, input paths are separated\n \twith a NUL character instead of a linefeed character.\n \n+-n, --non-matching::\n+\tShow given paths which don't match any pattern.\t This only\n+\tmakes sense when `--verbose` is enabled, otherwise it would\n+\tnot be possible to distinguish between paths which match a\n+\tpattern and those which don't.\n+\n OUTPUT\n ------\n \n@@ -65,6 +71,15 @@ are also used instead of colons and hard tabs:\n \n \n \n+If `-n` or `--non-matching` are specified, non-matching pathnames will\n+also be output, in which case all fields in each output record except\n+for will be empty. This can be useful when running\n+non-interactively, so that files can be incrementally streamed to\n+STDIN of a long-running check-ignore process, and for each of these\n+files, STDOUT will indicate whether that file matched a pattern or\n+not. (Without this option, it would be impossible to tell whether the\n+absence of output for a given file meant that it didn't match any\n+pattern, or that the output hadn't been generated yet.)\n \n EXIT STATUS\n -----------\ndiff --git a/builtin/check-ignore.c b/builtin/check-ignore.c\nindex 0240f99..59acf74 100644\n--- a/builtin/check-ignore.c\n+++ b/builtin/check-ignore.c\n@@ -5,7 +5,7 @@\n #include \"pathspec.h\"\n #include \"parse-options.h\"\n \n-static int quiet, verbose, stdin_paths;\n+static int quiet, verbose, stdin_paths, show_non_matching;\n static const char * const check_ignore_usage[] = {\n \"git check-ignore [options] pathname...\",\n \"git check-ignore [options] --stdin < \",\n@@ -22,21 +22,28 @@ static const struct option check_ignore_options[] = {\n \t\t N_(\"read file names from stdin\")),\n \tOPT_BOOLEAN('z', NULL, &null_term_line,\n \t\t N_(\"input paths are terminated by a null character\")),\n+\tOPT_BOOLEAN('n', \"non-matching\", &show_non_matching,\n+\t\t N_(\"show non-matching input paths\")),\n \tOPT_END()\n };\n \n static void output_exclude(const char *path, struct exclude *exclude)\n {\n-\tchar *bang = exclude->flags & EXC_FLAG_NEGATIVE ? \"!\" : \"\";\n-\tchar *slash = exclude->flags & EXC_FLAG_MUSTBEDIR ? \"/\" : \"\";\n+\tchar *bang = (exclude && exclude->flags & EXC_FLAG_NEGATIVE) ? \"!\" : \"\";\n+\tchar *slash = (exclude && exclude->flags & EXC_FLAG_MUSTBEDIR) ? \"/\" : \"\";\n \tif (!null_term_line) {\n \t\tif (!verbose) {\n \t\t\twrite_name_quoted(path, stdout, '\\n');\n \t\t} else {\n-\t\t\tquote_c_style(exclude->el->src, NULL, stdout, 0);\n-\t\t\tprintf(\":%d:%s%s%s\\t\",\n-\t\t\t exclude->srcpos,\n-\t\t\t bang, exclude->pattern, slash);\n+\t\t\tif (exclude) {\n+\t\t\t\tquote_c_style(exclude->el->src, NULL, stdout, 0);\n+\t\t\t\tprintf(\":%d:%s%s%s\\t\",\n+\t\t\t\t exclude->srcpos,\n+\t\t\t\t bang, exclude->pattern, slash);\n+\t\t\t}\n+\t\t\telse {\n+\t\t\t\tprintf(\"::\\t\");\n+\t\t\t}\n \t\t\tquote_c_style(path, NULL, stdout, 0);\n \t\t\tfputc('\\n', stdout);\n \t\t}\n@@ -44,11 +51,14 @@ static void output_exclude(const char *path, struct exclude *exclude)\n \t\tif (!verbose) {\n \t\t\tprintf(\"%s%c\", path, '\\0');\n \t\t} else {\n-\t\t\tprintf(\"%s%c%d%c%s%s%s%c%s%c\",\n-\t\t\t exclude->el->src, '\\0',\n-\t\t\t exclude->srcpos, '\\0',\n-\t\t\t bang, exclude->pattern, slash, '\\0',\n-\t\t\t path, '\\0');\n+\t\t\tif (exclude)\n+\t\t\t\tprintf(\"%s%c%d%c%s%s%s%c%s%c\",\n+\t\t\t\t exclude->el->src, '\\0',\n+\t\t\t\t exclude->srcpos, '\\0',\n+\t\t\t\t bang, exclude->pattern, slash, '\\0',\n+\t\t\t\t path, '\\0');\n+\t\t\telse\n+\t\t\t\tprintf(\"%c%c%c%s%c\", '\\0', '\\0', '\\0', path, '\\0');\n \t\t}\n \t}\n }\n@@ -89,15 +99,15 @@ static int check_ignore(const char *prefix, const char **pathspec)\n \t\t\t\t\t? strlen(prefix) : 0, path);\n \t\tfull_path = check_path_for_gitlink(full_path);\n \t\tdie_if_path_beyond_symlink(full_path, prefix);\n+\t\texclude = NULL;\n \t\tif (!seen[i]) {\n \t\t\texclude = last_exclude_matching_path(&check, full_path,\n \t\t\t\t\t\t\t -1, &dtype);\n-\t\t\tif (exclude) {\n-\t\t\t\tif (!quiet)\n-\t\t\t\t\toutput_exclude(path, exclude);\n-\t\t\t\tnum_ignored++;\n-\t\t\t}\n \t\t}\n+\t\tif (!quiet && (exclude || show_non_matching))\n+\t\t\toutput_exclude(path, exclude);\n+\t\tif (exclude)\n+\t\t\tnum_ignored++;\n \t}\n \tfree(seen);\n \tclear_directory(&dir);\n@@ -161,6 +171,8 @@ int cmd_check_ignore(int argc, const char **argv, const char *prefix)\n \t\tif (verbose)\n \t\t\tdie(_(\"cannot have both --quiet and --verbose\"));\n \t}\n+\tif (show_non_matching && !verbose)\n+\t\tdie(_(\"--non-matching is only valid with --verbose\"));\n \n \tif (stdin_paths) {\n \t\tnum_ignored = check_ignore_stdin_paths(prefix);\ndiff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh\nindex 314a86d..7af93ba 100755\n--- a/t/t0008-ignores.sh\n+++ b/t/t0008-ignores.sh\n@@ -66,16 +66,23 @@ test_check_ignore () {\n \n \tinit_vars &&\n \trm -f \"$HOME/stdout\" \"$HOME/stderr\" \"$HOME/cmd\" &&\n-\techo git $global_args check-ignore $quiet_opt $verbose_opt $args \\\n+\techo git $global_args check-ignore $quiet_opt $verbose_opt $non_matching_opt $args \\\n \t\t>\"$HOME/cmd\" &&\n+\techo \"$expect_code\" >\"$HOME/expected-exit-code\" &&\n \ttest_expect_code \"$expect_code\" \\\n-\t\tgit $global_args check-ignore $quiet_opt $verbose_opt $args \\\n+\t\tgit $global_args check-ignore $quiet_opt $verbose_opt $non_matching_opt $args \\\n \t\t>\"$HOME/stdout\" 2>\"$HOME/stderr\" &&\n \ttest_cmp \"$HOME/expected-stdout\" \"$HOME/stdout\" &&\n \tstderr_empty_on_success \"$expect_code\"\n }\n \n-# Runs the same code with 3 different levels of output verbosity,\n+# Runs the same code with 4 different levels of output verbosity:\n+#\n+# 1. with -q / --quiet\n+# 2. with default verbosity\n+# 3. with -v / --verbose\n+# 4. with -v / --verbose, *and* -n / --non-matching\n+#\n # expecting success each time. Takes advantage of the fact that\n # check-ignore --verbose output is the same as normal output except\n # for the extra first column.\n@@ -83,7 +90,9 @@ test_check_ignore () {\n # Arguments:\n # - (optional) prereqs for this test, e.g. 'SYMLINKS'\n # - test name\n-# - output to expect from -v / --verbose mode\n+# - output to expect from the fourth verbosity mode (the output\n+# from the other verbosity modes is automatically inferred\n+# from this value)\n # - code to run (should invoke test_check_ignore)\n test_expect_success_multi () {\n \tprereq=\n@@ -92,8 +101,9 @@ test_expect_success_multi () {\n \t\tprereq=$1\n \t\tshift\n \tfi\n-\ttestname=\"$1\" expect_verbose=\"$2\" code=\"$3\"\n+\ttestname=\"$1\" expect_all=\"$2\" code=\"$3\"\n \n+\texpect_verbose=$( echo \"$expect_all\" | grep -v '^::\t' )\n \texpect=$( echo \"$expect_verbose\" | sed -e 's/.*\t//' )\n \n \ttest_expect_success $prereq \"$testname\" '\n@@ -101,23 +111,40 @@ test_expect_success_multi () {\n \t\teval \"$code\"\n \t'\n \n-\tfor quiet_opt in '-q' '--quiet'\n-\tdo\n-\t\ttest_expect_success $prereq \"$testname${quiet_opt:+ with $quiet_opt}\" \"\n+\t# --quiet is only valid when a single pattern is passed\n+\tif test $( echo \"$expect_all\" | wc -l ) = 1\n+\tthen\n+\t\tfor quiet_opt in '-q' '--quiet'\n+\t\tdo\n+\t\t\ttest_expect_success $prereq \"$testname${quiet_opt:+ with $quiet_opt}\" \"\n \t\t\texpect '' &&\n \t\t\t$code\n \t\t\"\n-\tdone\n-\tquiet_opt=\n+\t\tdone\n+\t\tquiet_opt=\n+\tfi\n \n \tfor verbose_opt in '-v' '--verbose'\n \tdo\n-\t\ttest_expect_success $prereq \"$testname${verbose_opt:+ with $verbose_opt}\" \"\n-\t\t\texpect '$expect_verbose' &&\n-\t\t\t$code\n-\t\t\"\n+\t\tfor non_matching_opt in '' ' -n' ' --non-matching'\n+\t\tdo\n+\t\t\tif test -n \"$non_matching_opt\"\n+\t\t\tthen\n+\t\t\t\tmy_expect=\"$expect_all\"\n+\t\t\telse\n+\t\t\t\tmy_expect=\"$expect_verbose\"\n+\t\t\tfi\n+\n+\t\t\ttest_code=\"\n+\t\t\t\texpect '$my_expect' &&\n+\t\t\t\t$code\n+\t\t\t\"\n+\t\t\topts=\"$verbose_opt$non_matching_opt\"\n+\t\t\ttest_expect_success $prereq \"$testname${opts:+ with $opts}\" \"$test_code\"\n+\t\tdone\n \tdone\n \tverbose_opt=\n+\tnon_matching_opt=\n }\n \n test_expect_success 'setup' '\n@@ -178,7 +205,7 @@ test_expect_success 'setup' '\n #\n # test invalid inputs\n \n-test_expect_success_multi '. corner-case' '' '\n+test_expect_success_multi '. corner-case' '::\t.' '\n \ttest_check_ignore . 1\n '\n \n@@ -276,27 +303,39 @@ do\n \t\twhere=\"in subdir $subdir\"\n \tfi\n \n-\ttest_expect_success_multi \"non-existent file $where not ignored\" '' \"\n-\t\ttest_check_ignore '${subdir}non-existent' 1\n-\t\"\n+\ttest_expect_success_multi \"non-existent file $where not ignored\" \\\n+\t\t\"::\t${subdir}non-existent\" \\\n+\t\t\"test_check_ignore '${subdir}non-existent' 1\"\n \n \ttest_expect_success_multi \"non-existent file $where ignored\" \\\n-\t\t\".gitignore:1:one\t${subdir}one\" \"\n-\t\ttest_check_ignore '${subdir}one'\n-\t\"\n+\t\t\".gitignore:1:one\t${subdir}one\" \\\n+\t\t\"test_check_ignore '${subdir}one'\"\n \n-\ttest_expect_success_multi \"existing untracked file $where not ignored\" '' \"\n-\t\ttest_check_ignore '${subdir}not-ignored' 1\n-\t\"\n+\ttest_expect_success_multi \"existing untracked file $where not ignored\" \\\n+\t\t\"::\t${subdir}not-ignored\" \\\n+\t\t\"test_check_ignore '${subdir}not-ignored' 1\"\n \n-\ttest_expect_success_multi \"existing tracked file $where not ignored\" '' \"\n-\t\ttest_check_ignore '${subdir}ignored-but-in-index' 1\n-\t\"\n+\ttest_expect_success_multi \"existing tracked file $where not ignored\" \\\n+\t\t\"::\t${subdir}ignored-but-in-index\" \\\n+\t\t\"test_check_ignore '${subdir}ignored-but-in-index' 1\"\n \n \ttest_expect_success_multi \"existing untracked file $where ignored\" \\\n-\t\t\".gitignore:2:ignored-*\t${subdir}ignored-and-untracked\" \"\n-\t\ttest_check_ignore '${subdir}ignored-and-untracked'\n-\t\"\n+\t\t\".gitignore:2:ignored-*\t${subdir}ignored-and-untracked\" \\\n+\t\t\"test_check_ignore '${subdir}ignored-and-untracked'\"\n+\n+\ttest_expect_success_multi \"mix of file types $where\" \\\n+\"::\t${subdir}non-existent\n+.gitignore:1:one\t${subdir}one\n+::\t${subdir}not-ignored\n+::\t${subdir}ignored-but-in-index\n+.gitignore:2:ignored-*\t${subdir}ignored-and-untracked\" \\\n+\t\t\"test_check_ignore '\n+\t\t\t${subdir}non-existent\n+\t\t\t${subdir}one\n+\t\t\t${subdir}not-ignored\n+\t\t\t${subdir}ignored-but-in-index\n+\t\t\t${subdir}ignored-and-untracked'\n+\t\t\"\n done\n \n # Having established the above, from now on we mostly test against\n@@ -391,7 +430,7 @@ test_expect_success 'cd to ignored sub-directory with -v' '\n #\n # test handling of symlinks\n \n-test_expect_success_multi SYMLINKS 'symlink' '' '\n+test_expect_success_multi SYMLINKS 'symlink' '::\ta/symlink' '\n \ttest_check_ignore \"a/symlink\" 1\n '\n \n@@ -574,22 +613,33 @@ cat <<-\\EOF >stdin\n \tglobaltwo\n \tb/globaltwo\n \t../b/globaltwo\n+\tc/not-ignored\n EOF\n-cat <<-EOF >expected-verbose\n+# N.B. we deliberately end STDIN with a non-matching pattern in order\n+# to test that the exit code indicates that one or more of the\n+# provided paths is ignored - in other words, that it represents an\n+# aggregation of all the results, not just the final result.\n+\n+cat <<-EOF >expected-all\n \t.gitignore:1:one\t../one\n+\t::\t../not-ignored\n \t.gitignore:1:one\tone\n+\t::\tnot-ignored\n \ta/b/.gitignore:8:!on*\tb/on\n \ta/b/.gitignore:8:!on*\tb/one\n \ta/b/.gitignore:8:!on*\tb/one one\n \ta/b/.gitignore:8:!on*\tb/one two\n \ta/b/.gitignore:8:!on*\t\"b/one\\\"three\"\n \ta/b/.gitignore:9:!two\tb/two\n+\t::\tb/not-ignored\n \ta/.gitignore:1:two*\tb/twooo\n \t$global_excludes:2:!globaltwo\t../globaltwo\n \t$global_excludes:2:!globaltwo\tglobaltwo\n \t$global_excludes:2:!globaltwo\tb/globaltwo\n \t$global_excludes:2:!globaltwo\t../b/globaltwo\n+\t::\tc/not-ignored\n EOF\n+grep -v '^::\t' expected-all >expected-verbose\n sed -e 's/.*\t//' expected-verbose >expected-default\n \n sed -e 's/^\"//' -e 's/\\\\//' -e 's/\"$//' stdin | \\\n@@ -615,6 +665,14 @@ test_expect_success '--stdin from subdirectory with -v' '\n \t)\n '\n \n+test_expect_success '--stdin from subdirectory with -v -n' '\n+\texpect_from_stdin \n---\n builtin/check-ignore.c | 41 +++++++++++++++++++++--------------------\n 1 file changed, 21 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin/check-ignore.c b/builtin/check-ignore.c\nindex 59acf74..e2d3006 100644\n--- a/builtin/check-ignore.c\n+++ b/builtin/check-ignore.c\n@@ -63,30 +63,20 @@ static void output_exclude(const char *path, struct exclude *exclude)\n \t}\n }\n \n-static int check_ignore(const char *prefix, const char **pathspec)\n+static int check_ignore(struct path_exclude_check *check,\n+\t\t\tconst char *prefix, const char **pathspec)\n {\n-\tstruct dir_struct dir;\n \tconst char *path, *full_path;\n \tchar *seen;\n \tint num_ignored = 0, dtype = DT_UNKNOWN, i;\n-\tstruct path_exclude_check check;\n \tstruct exclude *exclude;\n \n-\t/* read_cache() is only necessary so we can watch out for submodules. */\n-\tif (read_cache() < 0)\n-\t\tdie(_(\"index file corrupt\"));\n-\n-\tmemset(&dir, 0, sizeof(dir));\n-\tdir.flags |= DIR_COLLECT_IGNORED;\n-\tsetup_standard_excludes(&dir);\n-\n \tif (!pathspec || !*pathspec) {\n \t\tif (!quiet)\n \t\t\tfprintf(stderr, \"no pathspec given.\\n\");\n \t\treturn 0;\n \t}\n \n-\tpath_exclude_check_init(&check, &dir);\n \t/*\n \t * look for pathspecs matching entries in the index, since these\n \t * should not be ignored, in order to be consistent with\n@@ -101,7 +91,7 @@ static int check_ignore(const char *prefix, const char **pathspec)\n \t\tdie_if_path_beyond_symlink(full_path, prefix);\n \t\texclude = NULL;\n \t\tif (!seen[i]) {\n-\t\t\texclude = last_exclude_matching_path(&check, full_path,\n+\t\t\texclude = last_exclude_matching_path(check, full_path,\n \t\t\t\t\t\t\t -1, &dtype);\n \t\t}\n \t\tif (!quiet && (exclude || show_non_matching))\n@@ -110,13 +100,11 @@ static int check_ignore(const char *prefix, const char **pathspec)\n \t\t\tnum_ignored++;\n \t}\n \tfree(seen);\n-\tclear_directory(&dir);\n-\tpath_exclude_check_clear(&check);\n \n \treturn num_ignored;\n }\n \n-static int check_ignore_stdin_paths(const char *prefix)\n+static int check_ignore_stdin_paths(struct path_exclude_check *check, const char *prefix)\n {\n \tstruct strbuf buf, nbuf;\n \tchar **pathspec = NULL;\n@@ -139,17 +127,18 @@ static int check_ignore_stdin_paths(const char *prefix)\n \t}\n \tALLOC_GROW(pathspec, nr + 1, alloc);\n \tpathspec[nr] = NULL;\n-\tnum_ignored = check_ignore(prefix, (const char **)pathspec);\n+\tnum_ignored = check_ignore(check, prefix, (const char **)pathspec);\n \tmaybe_flush_or_die(stdout, \"attribute to stdout\");\n \tstrbuf_release(&buf);\n \tstrbuf_release(&nbuf);\n-\tfree(pathspec);\n \treturn num_ignored;\n }\n \n int cmd_check_ignore(int argc, const char **argv, const char *prefix)\n {\n \tint num_ignored;\n+\tstruct dir_struct dir;\n+\tstruct path_exclude_check check;\n \n \tgit_config(git_default_config, NULL);\n \n@@ -174,12 +163,24 @@ int cmd_check_ignore(int argc, const char **argv, const char *prefix)\n \tif (show_non_matching && !verbose)\n \t\tdie(_(\"--non-matching is only valid with --verbose\"));\n \n+\t/* read_cache() is only necessary so we can watch out for submodules. */\n+\tif (read_cache() < 0)\n+\t\tdie(_(\"index file corrupt\"));\n+\n+\tmemset(&dir, 0, sizeof(dir));\n+\tdir.flags |= DIR_COLLECT_IGNORED;\n+\tsetup_standard_excludes(&dir);\n+\n+\tpath_exclude_check_init(&check, &dir);\n \tif (stdin_paths) {\n-\t\tnum_ignored = check_ignore_stdin_paths(prefix);\n+\t\tnum_ignored = check_ignore_stdin_paths(&check, prefix);\n \t} else {\n-\t\tnum_ignored = check_ignore(prefix, argv);\n+\t\tnum_ignored = check_ignore(&check, prefix, argv);\n \t\tmaybe_flush_or_die(stdout, \"ignore to stdout\");\n \t}\n \n+\tclear_directory(&dir);\n+\tpath_exclude_check_clear(&check);\n+\n \treturn !num_ignored;\n }\n-- \n1.8.2.1.342.gfa7285d\n"},{"id":"213930","messageId":"1365681913-7059-4-git-send-email-git@adamspiers.org","threadId":"33436","inReplyTo":"1365681913-7059-1-git-send-email-git@adamspiers.org","subject":"[PATCH v2 4/5] check-ignore: allow incremental streaming of queries via --stdin","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2013-04-11T12:05:12Z","receivedAt":"2013-04-11T12:05:12Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"Some callers, such as the git-annex web assistant, find it useful to\ninvoke git check-ignore as a persistent background process, which can\nthen have queries fed to its STDIN at any point, and the corresponding\nresponse consumed from its STDOUT. For this we need to invoke\ncheck_ignore() once per line of standard input, and flush standard\noutput after each result.\n\nThe above use case suggests that empty STDIN is actually a reasonable\nscenario (e.g. when the caller doesn't know in advance whether any\nqueries need to be fed to the background process until after it's\nalready started), so we make the minor behavioural change that \"no\npathspec given.\" is no longer emitted in when STDIN is empty.\n\nEven though check_ignore() could now be changed to operate on a single\npathspec, we keep it operating on an array of pathspecs since that is\na more convenient way of consuming the existing pathspec API.\n\nSigned-off-by: Adam Spiers \n---\n builtin/check-ignore.c | 15 +++++----------\n t/t0008-ignores.sh | 28 +++++++++++++++++++++++-----\n 2 files changed, 28 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/check-ignore.c b/builtin/check-ignore.c\nindex e2d3006..c00a7d6 100644\n--- a/builtin/check-ignore.c\n+++ b/builtin/check-ignore.c\n@@ -107,10 +107,9 @@ static int check_ignore(struct path_exclude_check *check,\n static int check_ignore_stdin_paths(struct path_exclude_check *check, const char *prefix)\n {\n \tstruct strbuf buf, nbuf;\n-\tchar **pathspec = NULL;\n-\tsize_t nr = 0, alloc = 0;\n+\tchar *pathspec[2] = { NULL, NULL };\n \tint line_termination = null_term_line ? 0 : '\\n';\n-\tint num_ignored;\n+\tint num_ignored = 0;\n \n \tstrbuf_init(&buf, 0);\n \tstrbuf_init(&nbuf, 0);\n@@ -121,14 +120,10 @@ static int check_ignore_stdin_paths(struct path_exclude_check *check, const char\n \t\t\t\tdie(\"line is badly quoted\");\n \t\t\tstrbuf_swap(&buf, &nbuf);\n \t\t}\n-\t\tALLOC_GROW(pathspec, nr + 1, alloc);\n-\t\tpathspec[nr] = xcalloc(strlen(buf.buf) + 1, sizeof(*buf.buf));\n-\t\tstrcpy(pathspec[nr++], buf.buf);\n+\t\tpathspec[0] = buf.buf;\n+\t\tnum_ignored += check_ignore(check, prefix, (const char **)pathspec);\n+\t\tmaybe_flush_or_die(stdout, \"check-ignore to stdout\");\n \t}\n-\tALLOC_GROW(pathspec, nr + 1, alloc);\n-\tpathspec[nr] = NULL;\n-\tnum_ignored = check_ignore(check, prefix, (const char **)pathspec);\n-\tmaybe_flush_or_die(stdout, \"attribute to stdout\");\n \tstrbuf_release(&buf);\n \tstrbuf_release(&nbuf);\n \treturn num_ignored;\ndiff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh\nindex 7af93ba..fbf12ae 100755\n--- a/t/t0008-ignores.sh\n+++ b/t/t0008-ignores.sh\n@@ -216,11 +216,7 @@ test_expect_success_multi 'empty command line' '' '\n \n test_expect_success_multi '--stdin with empty STDIN' '' '\n \ttest_check_ignore \"--stdin\" 1 /dev/null 2>&1\n+\tthen\n+\t\ttest_set_prereq STDBUF\n+\tfi\n+'\n+\n+test_expect_success STDBUF 'streaming support for --stdin' '\n+\t(\n+\t\techo one\n+\t\tsleep 2\n+\t\techo two\n+\t) | stdbuf -oL git check-ignore -v -n --stdin >out &\n+\tpid=$! &&\n+\tsleep 1 &&\n+\tgrep \"^\\.gitignore:1:one\tone\" out &&\n+\ttest $( wc -l /dev/null\n+'\n \n test_done\n-- \n1.8.2.1.342.gfa7285d\n"},{"id":"213931","messageId":"1365681913-7059-5-git-send-email-git@adamspiers.org","threadId":"33436","inReplyTo":"1365681913-7059-1-git-send-email-git@adamspiers.org","subject":"[PATCH v2 5/5] Documentation: add caveats about I/O buffering for check-{attr,ignore}","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2013-04-11T12:05:13Z","receivedAt":"2013-04-11T12:05:13Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"check-attr and check-ignore have the potential to deadlock callers\nwhich do not read back the output in real-time. For example, if a\ncaller writes N paths out and then reads N lines back in, it risks\nbecoming blocked on write() to check-*, and check-* is blocked on\nwrite back to the caller. Somebody has to buffer; the pipe buffers\nprovide some leeway, but they are limited.\n\nThanks to Peff for pointing this out:\n\n http://article.gmane.org/gmane.comp.version-control.git/220534\n\nSigned-off-by: Adam Spiers \n---\n Documentation/git-check-attr.txt | 5 +++++\n Documentation/git-check-ignore.txt | 5 +++++\n Documentation/git.txt | 16 +++++++++-------\n 3 files changed, 19 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-check-attr.txt b/Documentation/git-check-attr.txt\nindex 5abdbaa..a7be80d 100644\n--- a/Documentation/git-check-attr.txt\n+++ b/Documentation/git-check-attr.txt\n@@ -56,6 +56,11 @@ being queried and can be either:\n 'set';;\t\twhen the attribute is defined as true.\n ;;\twhen a value has been assigned to the attribute.\n \n+Buffering happens as documented under the `GIT_FLUSH` option in\n+linkgit:git[1]. The caller is responsible for avoiding deadlocks\n+caused by overfilling an input buffer or reading from an empty output\n+buffer.\n+\n EXAMPLES\n --------\n \ndiff --git a/Documentation/git-check-ignore.txt b/Documentation/git-check-ignore.txt\nindex 7e3cabc..8e1f7ab 100644\n--- a/Documentation/git-check-ignore.txt\n+++ b/Documentation/git-check-ignore.txt\n@@ -81,6 +81,11 @@ not. (Without this option, it would be impossible to tell whether the\n absence of output for a given file meant that it didn't match any\n pattern, or that the output hadn't been generated yet.)\n \n+Buffering happens as documented under the `GIT_FLUSH` option in\n+linkgit:git[1]. The caller is responsible for avoiding deadlocks\n+caused by overfilling an input buffer or reading from an empty output\n+buffer.\n+\n EXIT STATUS\n -----------\n \ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 6a875f2..eecdb15 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -808,13 +808,15 @@ for further details.\n \n 'GIT_FLUSH'::\n \tIf this environment variable is set to \"1\", then commands such\n-\tas 'git blame' (in incremental mode), 'git rev-list', 'git log',\n-\tand 'git whatchanged' will force a flush of the output stream\n-\tafter each commit-oriented record have been flushed. If this\n-\tvariable is set to \"0\", the output of these commands will be done\n-\tusing completely buffered I/O. If this environment variable is\n-\tnot set, Git will choose buffered or record-oriented flushing\n-\tbased on whether stdout appears to be redirected to a file or not.\n+\tas 'git blame' (in incremental mode), 'git rev-list', 'git\n+\tlog', 'git check-attr', 'git check-ignore', and 'git\n+\twhatchanged' will force a flush of the output stream after\n+\teach commit-oriented record have been flushed. If this\n+\tvariable is set to \"0\", the output of these commands will be\n+\tdone using completely buffered I/O. If this environment\n+\tvariable is not set, Git will choose buffered or\n+\trecord-oriented flushing based on whether stdout appears to be\n+\tredirected to a file or not.\n \n 'GIT_TRACE'::\n \tIf this variable is set to \"1\", \"2\" or \"true\" (comparison\n-- \n1.8.2.1.342.gfa7285d\n"},{"id":"213981","messageId":"7vsj2xhrc7.fsf@alter.siamese.dyndns.org","threadId":"33436","inReplyTo":"1365681913-7059-5-git-send-email-git@adamspiers.org","subject":"Re: [PATCH v2 5/5] Documentation: add caveats about I/O buffering for check-{attr,ignore}","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-11T18:09:28Z","receivedAt":"2013-04-11T18:09:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Spiers writes:\n\n> diff --git a/Documentation/git-check-ignore.txt b/Documentation/git-check-ignore.txt\n> index 7e3cabc..8e1f7ab 100644\n> --- a/Documentation/git-check-ignore.txt\n> +++ b/Documentation/git-check-ignore.txt\n> @@ -81,6 +81,11 @@ not. (Without this option, it would be impossible to tell whether the\n> absence of output for a given file meant that it didn't match any\n> pattern, or that the output hadn't been generated yet.)\n> \n> +Buffering happens as documented under the `GIT_FLUSH` option in\n> +linkgit:git[1]. The caller is responsible for avoiding deadlocks\n> +caused by overfilling an input buffer or reading from an empty output\n> +buffer.\n> +\n> EXIT STATUS\n> -----------\n> \n> diff --git a/Documentation/git.txt b/Documentation/git.txt\n> index 6a875f2..eecdb15 100644\n> --- a/Documentation/git.txt\n> +++ b/Documentation/git.txt\n> @@ -808,13 +808,15 @@ for further details.\n> \n> 'GIT_FLUSH'::\n> \tIf this environment variable is set to \"1\", then commands such\n> -\tas 'git blame' (in incremental mode), 'git rev-list', 'git log',\n> -\tand 'git whatchanged' will force a flush of the output stream\n> -\tafter each commit-oriented record have been flushed. If this\n> -\tvariable is set to \"0\", the output of these commands will be done\n> -\tusing completely buffered I/O. If this environment variable is\n> -\tnot set, Git will choose buffered or record-oriented flushing\n> -\tbased on whether stdout appears to be redirected to a file or not.\n> +\tas 'git blame' (in incremental mode), 'git rev-list', 'git\n> +\tlog', 'git check-attr', 'git check-ignore', and 'git\n> +\twhatchanged' will force a flush of the output stream after\n> +\teach commit-oriented record have been flushed. If this\n> +\tvariable is set to \"0\", the output of these commands will be\n> +\tdone using completely buffered I/O. If this environment\n> +\tvariable is not set, Git will choose buffered or\n> +\trecord-oriented flushing based on whether stdout appears to be\n> +\tredirected to a file or not.\n\nReflowing of the text is very much unappreciated X-<. \n\nIt took me five minutes to spot that you only added check-attr and\ncheck-ignore and forgot to adjust that \"commit-oriented record\" to\nan updated reality, where you now have commands that produce\nnon-commit-oriented record to the output.\n\nIt would have been far simpler to review if it were like this, don't\nyou think?\n\n> \tIf this environment variable is set to \"1\", then commands such\n> \tas 'git blame' (in incremental mode), 'git rev-list', 'git log',\n> -\tand 'git whatchanged' will force a flush of the output stream\n> -\tafter each commit-oriented record have been flushed. If this\n> +\t'git check-attr', 'git check-ignore', and 'git whatchanged' will\n> +\tforce a flush of the output stream\n> + after each record have been flushed. If this\n> \tvariable is set to \"0\", the output of these commands will be done\n> \tusing completely buffered I/O. If this environment variable is\n> \tnot set, Git will choose buffered or record-oriented flushing\n> \tbased on whether stdout appears to be redirected to a file or not.\n"},{"id":"213986","messageId":"20130411183344.GA3177@sigill.intra.peff.net","threadId":"33436","inReplyTo":"20130411112000.GC24296@pacific.linksys.moosehall","subject":"Re: [PATCH 2/5] check-ignore: allow incremental streaming of queries via --stdin","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-04-11T18:33:44Z","receivedAt":"2013-04-11T18:33:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 11, 2013 at 12:20:00PM +0100, Adam Spiers wrote:\n\n> On Thu, Apr 11, 2013 at 02:59:32AM +0100, Adam Spiers wrote:\n> > +test_expect_success STDBUF 'streaming support for --stdin' '\n> > +\t(\n> > +\t\techo one\n> > +\t\tsleep 2\n> > +\t\techo two\n> > +\t) | stdbuf -oL git check-ignore -v -n --stdin >out &\n> \n> I just noticed that this patch precedes the one in the same series\n> which adds -n support. I'll reorder them accordingly to avoid\n> breaking git bisect.\n\nThanks for noticing. FWIW, I often do this:\n\n GIT_EDITOR='sed -i \"/^pick /aexec make test\" \\\n git rebase -i origin/master\n\non my topics to make sure each commit compiles and passes the tests in\nisolation (it will stop on a failure, at which point you can fix up and\n\"git rebase --continue\").\n\nIt's somewhat annoying, because it runs the whole test suite, even for\ncommits that are obviously not going to have an impact (e.g., doc\nupdates, or change to a single test). But it has saved me from\nembarrassment many times, when I thought for sure that my commits were\nobviously correct. :)\n\n-Peff\n"},{"id":"213987","messageId":"20130411183518.GB3177@sigill.intra.peff.net","threadId":"33436","inReplyTo":"20130411110511.GB24296@pacific.linksys.moosehall","subject":"Re: [PATCH 1/5] check-ignore: move setup into cmd_check_ignore()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-04-11T18:35:18Z","receivedAt":"2013-04-11T18:35:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 11, 2013 at 12:05:11PM +0100, Adam Spiers wrote:\n\n> On Thu, Apr 11, 2013 at 01:25:53AM -0400, Jeff King wrote:\n> > On Thu, Apr 11, 2013 at 02:59:31AM +0100, Adam Spiers wrote:\n> > > -static int check_ignore(const char *prefix, const char **pathspec)\n> > > +static int check_ignore(struct path_exclude_check check,\n> > > +\t\t\tconst char *prefix, const char **pathspec)\n> > \n> > Did you mean to pass the struct by value here? If it is truly a per-path\n> > [...]\n>\n> It's not a per-path value; it's supposed to be reused across checks\n> for multiple paths, as explained in the comments above\n> last_exclude_matching_path():\n\nMakes sense (I didn't look into it very far, and was just guessing based\non the pass-by-value). Passing a pointer is definitely the right fix,\nthen.\n\nThanks.\n\n-Peff\n"},{"id":"213994","messageId":"20130411191132.GC3177@sigill.intra.peff.net","threadId":"33436","inReplyTo":"1365681913-7059-4-git-send-email-git@adamspiers.org","subject":"Re: [PATCH v2 4/5] check-ignore: allow incremental streaming of queries via --stdin","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-04-11T19:11:32Z","receivedAt":"2013-04-11T19:11:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 11, 2013 at 01:05:12PM +0100, Adam Spiers wrote:\n\n> +test_expect_success 'setup: have stdbuf?' '\n> +\tif which stdbuf >/dev/null 2>&1\n> +\tthen\n> +\t\ttest_set_prereq STDBUF\n> +\tfi\n> +'\n> +\n> +test_expect_success STDBUF 'streaming support for --stdin' '\n> +\t(\n> +\t\techo one\n> +\t\tsleep 2\n> +\t\techo two\n> +\t) | stdbuf -oL git check-ignore -v -n --stdin >out &\n> +\tpid=$! &&\n> +\tsleep 1 &&\n> +\tgrep \"^\\.gitignore:1:one\tone\" out &&\n> +\ttest $( wc -l +\tsleep 2 &&\n> +\tgrep \"^::\ttwo\" out &&\n> +\ttest $( wc -l +\t( wait $pid || kill $pid || : ) 2>/dev/null\n> +'\n\nI always get a little nervous with sleeps in the test suite, as they are\nindicative that we are trying to avoid some race condition, which means\nthat the test can fail when the system is under load, or when a tool\nlike valgrind is used which drastically alters the timing (e.g., if\ncheck-ignore takes longer than 1 second to produce its answer, we may\nfail here).\n\nIs there a simpler way to test this?\n\nLike:\n\n # Set up a long-running \"check-ignore\" connected by pipes.\n mkfifo in out &&\n (git check-ignore ... out &) &&\n\n # We cannot just \"echo >in\" because check-ignore\n # would get EOF after echo exited; instead we open\n # the descriptor in our shell, and then echo to the\n # fd. We make sure to close it at the end, so that\n # the subprocess does get EOF and dies properly.\n exec 9>in &&\n test_when_finished \"exec 9>&-\" &&\n\n # Now we can do interactive tests\n echo >&9 one &&\n read response &9 two &&\n read response Reflowing of the text is very much unappreciated X-<. \n\nI very much appreciate the excellent job you do as maintainer; your\nattention to detail results in an incredibly high quality project.\nHowever I do occasionally find your communication style unnecessarily\nabrasive. Maybe that's just me.\n\n> It took me five minutes to spot that you only added check-attr and\n> check-ignore and forgot to adjust that \"commit-oriented record\" to\n> an updated reality, where you now have commands that produce\n> non-commit-oriented record to the output.\n\nI am sorry for wasting five minutes of your time. A non-reflowed\nversion is included inline below, and also at:\n\n https://github.com/aspiers/git/compare/master...git-annex-streaming\n\n> It would have been far simpler to review if it were like this, don't\n> you think?\n\nIt would have been slightly simpler, yes. It did occur to me not to\nre-flow it, but then one of the lines ended up noticeably shorter, and\nas it was a short paragraph, I estimated that you would prefer it\nre-flowed. Clearly I was wrong - not the first time, and it won't be\nthe last either, since I'm just a flawed human being trying to do my\nbest in the time available. The question then arises: how uneven does\na paragraph's right margin have to be in order to justify re-flowing?\nI could not find any guidelines in SubmittingPatches or\nCodingGuidelines regarding re-flowing of documentation. With\nhindsight, I can now see that it would have been better to skip it on\nthis occasion, or at least keep the re-flow as a separate commit.\n\nSo I apologise again for the mistake, but don't you think it would\nhave been far more pleasant if instead you'd worded your email\nsomething like this?\n\n Thanks for the patches. I notice that you unnecessarily re-flowed\n the latter half of the GIT_FLUSH paragraph; unfortunately this\n meant I had to spend a few extra minutes on the review and almost\n missed that \"commit-oriented\" is no longer applicable. In future,\n please avoid re-flowing text where possible.\n\nFortunately I'm not the sensitive sort, but I imagine that there are\nothers in this community who might be discouraged from contributing\nfor fear of being on the receiving end of sentences which end with\nphrases such as \"[...] is very much unappreciated X-<.\" Please don't\nunderestimate the human factor; common courtesy can make a big\ndifference.\n\nThanks,\nAdam\n\n-- >8 --\nSubject: [PATCH v3 5/5] Documentation: add caveats about I/O buffering for\n check-{attr,ignore}\n\ncheck-attr and check-ignore have the potential to deadlock callers\nwhich do not read back the output in real-time. For example, if a\ncaller writes N paths out and then reads N lines back in, it risks\nbecoming blocked on write() to check-*, and check-* is blocked on\nwrite back to the caller. Somebody has to buffer; the pipe buffers\nprovide some leeway, but they are limited.\n\nThanks to Peff for pointing this out:\n\n http://article.gmane.org/gmane.comp.version-control.git/220534\n\nSigned-off-by: Adam Spiers \n---\n Documentation/git-check-attr.txt | 5 +++++\n Documentation/git-check-ignore.txt | 5 +++++\n Documentation/git.txt | 7 ++++---\n 3 files changed, 14 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-check-attr.txt b/Documentation/git-check-attr.txt\nindex 5abdbaa..a7be80d 100644\n--- a/Documentation/git-check-attr.txt\n+++ b/Documentation/git-check-attr.txt\n@@ -56,6 +56,11 @@ being queried and can be either:\n 'set';;\t\twhen the attribute is defined as true.\n ;;\twhen a value has been assigned to the attribute.\n \n+Buffering happens as documented under the `GIT_FLUSH` option in\n+linkgit:git[1]. The caller is responsible for avoiding deadlocks\n+caused by overfilling an input buffer or reading from an empty output\n+buffer.\n+\n EXAMPLES\n --------\n \ndiff --git a/Documentation/git-check-ignore.txt b/Documentation/git-check-ignore.txt\nindex 7e3cabc..8e1f7ab 100644\n--- a/Documentation/git-check-ignore.txt\n+++ b/Documentation/git-check-ignore.txt\n@@ -81,6 +81,11 @@ not. (Without this option, it would be impossible to tell whether the\n absence of output for a given file meant that it didn't match any\n pattern, or that the output hadn't been generated yet.)\n \n+Buffering happens as documented under the `GIT_FLUSH` option in\n+linkgit:git[1]. The caller is responsible for avoiding deadlocks\n+caused by overfilling an input buffer or reading from an empty output\n+buffer.\n+\n EXIT STATUS\n -----------\n \ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 6a875f2..3258f2c 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -808,9 +808,10 @@ for further details.\n \n 'GIT_FLUSH'::\n \tIf this environment variable is set to \"1\", then commands such\n-\tas 'git blame' (in incremental mode), 'git rev-list', 'git log',\n-\tand 'git whatchanged' will force a flush of the output stream\n-\tafter each commit-oriented record have been flushed. If this\n+\tas 'git blame' (in incremental mode), 'git rev-list', 'git\n+\tlog', 'git check-attr', 'git check-ignore', and 'git\n+\twhatchanged' will force a flush of the output stream\n+\tafter each record has been flushed. If this\n \tvariable is set to \"0\", the output of these commands will be done\n \tusing completely buffered I/O. If this environment variable is\n \tnot set, Git will choose buffered or record-oriented flushing\n-- \n1.8.2.1.347.gbef22ca\n"},{"id":"214003","messageId":"20130411203141.GB21091@pacific.linksys.moosehall","threadId":"33436","inReplyTo":"20130411191132.GC3177@sigill.intra.peff.net","subject":"Re: [PATCH v2 4/5] check-ignore: allow incremental streaming of queries via --stdin","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2013-04-11T20:31:41Z","receivedAt":"2013-04-11T20:31:41Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Thu, Apr 11, 2013 at 03:11:32PM -0400, Jeff King wrote:\n> I always get a little nervous with sleeps in the test suite, as they are\n> indicative that we are trying to avoid some race condition, which means\n> that the test can fail when the system is under load, or when a tool\n> like valgrind is used which drastically alters the timing (e.g., if\n> check-ignore takes longer than 1 second to produce its answer, we may\n> fail here).\n\nAgreed, especially here where my btrfs filesystems see fit to kindly\nfreeze my system for a few seconds many times each day :-/\n\n> Is there a simpler way to test this?\n> \n> Like:\n> \n> # Set up a long-running \"check-ignore\" connected by pipes.\n> mkfifo in out &&\n> (git check-ignore ... out &) &&\n> \n> # We cannot just \"echo >in\" because check-ignore\n> # would get EOF after echo exited; instead we open\n> # the descriptor in our shell, and then echo to the\n> # fd. We make sure to close it at the end, so that\n> # the subprocess does get EOF and dies properly.\n> exec 9>in &&\n> test_when_finished \"exec 9>&-\" &&\n> \n> # Now we can do interactive tests\n> echo >&9 one &&\n> read response test \"$response\" = ... &&\n> echo >&9 two &&\n> read response test \"$response\" = ...\n> \n> Hmm. Maybe simpler wasn't the right word. :) But it avoids any sleeps or\n> race conditions.\n\nThe shell source is strong with this one ;-)\n\nCongrats - I first tried with FIFOs (hence my other patch which moves\nthe PIPE test prerequisite definition into the core framework - the\noriginal intention was to reuse it here) but failed to get it working.\nI'll re-roll using your approach.\n"},{"id":"214004","messageId":"20130411204019.GA7588@sigill.intra.peff.net","threadId":"33436","inReplyTo":"20130411203141.GB21091@pacific.linksys.moosehall","subject":"Re: [PATCH v2 4/5] check-ignore: allow incremental streaming of queries via --stdin","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-04-11T20:40:19Z","receivedAt":"2013-04-11T20:40:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 11, 2013 at 09:31:41PM +0100, Adam Spiers wrote:\n\n> The shell source is strong with this one ;-)\n> \n> Congrats - I first tried with FIFOs (hence my other patch which moves\n> the PIPE test prerequisite definition into the core framework - the\n> original intention was to reuse it here) but failed to get it working.\n> I'll re-roll using your approach.\n\nThanks. If it make you feel any better, it took about 20 minutes of\nexperimenting and at least three head-scratching \"wait, that _should_\nhave worked\" moments to get it right. :)\n\nThe rest of the series looked good to me, though I admit I did not think\ntoo hard about the \"--non-matching\" patch, as it looked like you and\nJunio had already given some thought to the output format, which is the\ntricky part.\n\n-Peff\n"},{"id":"214007","messageId":"20130411210430.GA6234@pug.qqx.org","threadId":"33436","inReplyTo":"1365681913-7059-4-git-send-email-git@adamspiers.org","subject":"Re: [PATCH v2 4/5] check-ignore: allow incremental streaming of queries via --stdin","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2013-04-11T21:04:30Z","receivedAt":"2013-04-11T21:04:30Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"At 13:05 +0100 11 Apr 2013, Adam Spiers wrote:\n>The above use case suggests that empty STDIN is actually a reasonable\n>scenario (e.g. when the caller doesn't know in advance whether any\n>queries need to be fed to the background process until after it's\n>already started), so we make the minor behavioural change that \"no\n>pathspec given.\" is no longer emitted in when STDIN is empty.\n\nThe last \"in\" there looks to be misplaced. Was that originally \nsomething like \"in the case\"? If so the removed words should be \nrestored or the lingering one removed as well.\n"},{"id":"214017","messageId":"20130411225533.GA26949@pacific.linksys.moosehall","threadId":"33436","inReplyTo":"20130411210430.GA6234@pug.qqx.org","subject":"Re: [PATCH v2 4/5] check-ignore: allow incremental streaming of queries via --stdin","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2013-04-11T22:55:34Z","receivedAt":"2013-04-11T22:55:34Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Thu, Apr 11, 2013 at 05:04:30PM -0400, Aaron Schrab wrote:\n> At 13:05 +0100 11 Apr 2013, Adam Spiers wrote:\n> >The above use case suggests that empty STDIN is actually a reasonable\n> >scenario (e.g. when the caller doesn't know in advance whether any\n> >queries need to be fed to the background process until after it's\n> >already started), so we make the minor behavioural change that \"no\n> >pathspec given.\" is no longer emitted in when STDIN is empty.\n> \n> The last \"in\" there looks to be misplaced. Was that originally\n> something like \"in the case\"? If so the removed words should be\n> restored or the lingering one removed as well.\n\nI'll remove it; thanks.\n"},{"id":"214022","messageId":"7vzjx4fqex.fsf@alter.siamese.dyndns.org","threadId":"33436","inReplyTo":"20130411201219.GA21091@pacific.linksys.moosehall","subject":"Re: [PATCH v3 5/5] Documentation: add caveats about I/O buffering for check-{attr,ignore}","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-12T02:12:22Z","receivedAt":"2013-04-12T02:12:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Spiers writes:\n\n> On Thu, Apr 11, 2013 at 11:09:28AM -0700, Junio C Hamano wrote:\n>> Reflowing of the text is very much unappreciated X-<. \n>\n> I very much appreciate the excellent job you do as maintainer; your\n> attention to detail results in an incredibly high quality project.\n> However I do occasionally find your communication style unnecessarily\n> abrasive. Maybe that's just me.\n\nSorry for being me X-<. Yeah, I agree that the above came out to be\nmore blunt than needed.\n\nIt is usually OK to re-flow the text in the paragraph you are\ntouching. After all, for the purpose of reviewing, people can just\nblindly apply and then ask \"diff --color-words\". In this case,\nhowever, there was some changes that conflict in the vicinity, and\nreflowing made the resolution unnecessarily more cumbersome.\n\nI have briefly looked at this series, but it severely conflicts with\na few topics in flight that touch the infrastructure you are using,\nso I haven't merged it to 'pu'. Perhaps after things calm down, we\nmay want to ask you to reroll on top of updated codebase.\n\nThanks.\n"},{"id":"214052","messageId":"20130412110045.GC26949@pacific.linksys.moosehall","threadId":"33436","inReplyTo":"7vzjx4fqex.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 5/5] Documentation: add caveats about I/O buffering for check-{attr,ignore}","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2013-04-12T11:00:45Z","receivedAt":"2013-04-12T11:00:45Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Thu, Apr 11, 2013 at 07:12:22PM -0700, Junio C Hamano wrote:\n> It is usually OK to re-flow the text in the paragraph you are\n> touching. After all, for the purpose of reviewing, people can just\n> blindly apply and then ask \"diff --color-words\". In this case,\n> however, there was some changes that conflict in the vicinity, and\n> reflowing made the resolution unnecessarily more cumbersome.\n\nI see. Thanks for the tip; I was only dimly aware of --color-words.\n\n> I have briefly looked at this series, but it severely conflicts with\n> a few topics in flight that touch the infrastructure you are using,\n> so I haven't merged it to 'pu'. Perhaps after things calm down, we\n> may want to ask you to reroll on top of updated codebase.\n\nSure, no problem. I'll try a quick rebase now to see how ugly it is.\n\nBy the way, I've replaced my test for streaming --stdin which was\nbased on stdbuf(1) and sleep(1) with Peff's clever hack based on\nmkfifo. I'll hold off from sending a reroll until pathspec activity\ncools down, but in the meantime it's available here:\n\n https://github.com/aspiers/git/compare/master...git-annex-streaming\n\nIt requires my \"t: make PIPE a standard test prerequisite\" patch, but\nI notice that's already in master which will make things easier later\non.\n\nThanks!\nAdam\n"},{"id":"215106","messageId":"7vbo96phmn.fsf@alter.siamese.dyndns.org","threadId":"33436","inReplyTo":"20130411203141.GB21091@pacific.linksys.moosehall","subject":"Re: [PATCH v2 4/5] check-ignore: allow incremental streaming of queries via --stdin","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-22T18:03:44Z","receivedAt":"2013-04-22T18:03:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Spiers writes:\n\n> On Thu, Apr 11, 2013 at 03:11:32PM -0400, Jeff King wrote:\n>> I always get a little nervous with sleeps in the test suite, as they are\n>> indicative that we are trying to avoid some race condition, which means\n>> that the test can fail when the system is under load, or when a tool\n>> like valgrind is used which drastically alters the timing (e.g., if\n>> check-ignore takes longer than 1 second to produce its answer, we may\n>> fail here).\n>\n> Agreed, especially here where my btrfs filesystems see fit to kindly\n> freeze my system for a few seconds many times each day :-/\n> \n>> Is there a simpler way to test this?\n>> \n>> Like:\n>> ...\n> I'll re-roll using your approach.\n\nI think I missed this one and it already is in 'next'.\n\nI'll hold it back so please make your re-roll into an incremental\nupdate.\n\nThanks.\n"},{"id":"215296","messageId":"20130424080235.GC17889@pacific.linksys.moosehall","threadId":"33436","inReplyTo":"7vbo96phmn.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 4/5] check-ignore: allow incremental streaming of queries via --stdin","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2013-04-24T08:02:35Z","receivedAt":"2013-04-24T08:02:35Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Mon, Apr 22, 2013 at 11:03:44AM -0700, Junio C Hamano wrote:\n> Adam Spiers writes:\n> \n> > On Thu, Apr 11, 2013 at 03:11:32PM -0400, Jeff King wrote:\n> >> I always get a little nervous with sleeps in the test suite, as they are\n> >> indicative that we are trying to avoid some race condition, which means\n> >> that the test can fail when the system is under load, or when a tool\n> >> like valgrind is used which drastically alters the timing (e.g., if\n> >> check-ignore takes longer than 1 second to produce its answer, we may\n> >> fail here).\n> >\n> > Agreed, especially here where my btrfs filesystems see fit to kindly\n> > freeze my system for a few seconds many times each day :-/\n> > \n> >> Is there a simpler way to test this?\n> >> \n> >> Like:\n> >> ...\n> > I'll re-roll using your approach.\n> \n> I think I missed this one and it already is in 'next'.\n> \n> I'll hold it back so please make your re-roll into an incremental\n> update.\n\nWill do - will probably take a few days more though, since I'm\ncurrently catching up on a post-travel work backlog.\n"},{"id":"215930","messageId":"1367276125-15239-1-git-send-email-git@adamspiers.org","threadId":"33436","inReplyTo":"20130424080235.GC17889@pacific.linksys.moosehall","subject":"[PATCH] t0008: use named pipe (FIFO) to test check-ignore streaming","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2013-04-29T22:55:25Z","receivedAt":"2013-04-29T22:55:25Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"sleeps in the check-ignore test suite are not ideal since they can\nfail when the system is under load, or when a tool like valgrind is\nused which drastically alters the timing. Therefore we replace them\nwith a more robust solution using a named pipe (FIFO).\n\nThanks to Jeff King for coming up with the redirection wizardry\nrequired to make this work.\n\nhttp://article.gmane.org/gmane.comp.version-control.git/220916\n\nSigned-off-by: Adam Spiers \n---\n t/t0008-ignores.sh | 38 +++++++++++++++++---------------------\n 1 file changed, 17 insertions(+), 21 deletions(-)\n\ndiff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh\nindex fbf12ae..a56db80 100755\n--- a/t/t0008-ignores.sh\n+++ b/t/t0008-ignores.sh\n@@ -688,27 +688,23 @@ do\n \t'\n done\n \n-test_expect_success 'setup: have stdbuf?' '\n-\tif which stdbuf >/dev/null 2>&1\n-\tthen\n-\t\ttest_set_prereq STDBUF\n-\tfi\n-'\n-\n-test_expect_success STDBUF 'streaming support for --stdin' '\n-\t(\n-\t\techo one\n-\t\tsleep 2\n-\t\techo two\n-\t) | stdbuf -oL git check-ignore -v -n --stdin >out &\n-\tpid=$! &&\n-\tsleep 1 &&\n-\tgrep \"^\\.gitignore:1:one\tone\" out &&\n-\ttest $( wc -l /dev/null\n+test_expect_success PIPE 'streaming support for --stdin' '\n+\tmkfifo in out &&\n+\t(git check-ignore -n -v --stdin out &) &&\n+\n+\t# We cannot just \"echo >in\" because check-ignore would get EOF\n+\t# after echo exited; instead we open the descriptor in our\n+\t# shell, and then echo to the fd. We make sure to close it at\n+\t# the end, so that the subprocess does get EOF and dies\n+\t# properly.\n+\texec 9>in &&\n+\ttest_when_finished \"exec 9>&-\" &&\n+\techo >&9 one &&\n+\tread response &9 two &&\n+\tread response