{"thread":{"id":"34414","subject":"[PATCH 0/4] Make check-{attr,ignore} -z consistent","startedAt":"2013-07-12T06:18:43Z","lastAt":"2013-07-12T06:55:26Z","messageCount":6,"participants":["Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"223132","messageId":"1373609927-3244-1-git-send-email-gitster@pobox.com","threadId":"34414","inReplyTo":null,"subject":"[PATCH 0/4] Make check-{attr,ignore} -z consistent","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-12T06:18:43Z","receivedAt":"2013-07-12T06:18:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"A command that has to deal with input/output that may contain LF\nneeds to offer the \"-z\" (--nul-terminated-records) option, and if it\ndoes not support separate --nul-terminated-{input,output} options,\nthe \"-z\" option should govern both input and output. A caller that\nuses \"-z\" knows that the paths it feeds to these commands as input\nmay have LF that cannot be expressed in LF delimited input format,\nand the output from these commands do contain the same paths, so\nthere is no way for their output to be expressed unambiguously for\nan input that requires \"-z\".\n\nUnfortunately, \"git check-attr -z\" was broken and ignored the option\non the output side. This is a backward-incompatible fix, so we may\nneed to add a \"checkAttr.brokenZ\" configuration to allow people to\nkeep the existing breakage on top of these fixes, and then flip the\ndefault at Git 2.0 boundary (sometime early next year).\n\nCredit goes to Eric Sunshine for finding this discrepancy\n($gmane/230158).\n\nJunio C Hamano (4):\n check-ignore: the name of the character is NUL, not NULL\n check-attr: the name of the character is NUL, not NULL\n check-ignore -z: a single -z should apply to both input and output\n check-attr -z: a single -z should apply to both input and output\n\n Documentation/git-check-attr.txt | 9 +++++++--\n builtin/check-attr.c | 20 ++++++++++++++------\n builtin/check-ignore.c | 12 ++++++------\n 3 files changed, 27 insertions(+), 14 deletions(-)\n\n-- \n1.8.3.2-911-g2c4daa5\n"},{"id":"223133","messageId":"1373609927-3244-2-git-send-email-gitster@pobox.com","threadId":"34414","inReplyTo":"1373609927-3244-1-git-send-email-gitster@pobox.com","subject":"[PATCH 1/4] check-ignore: the name of the character is NUL, not NULL","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-12T06:18:44Z","receivedAt":"2013-07-12T06:18:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Signed-off-by: Junio C Hamano \n---\n builtin/check-ignore.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/check-ignore.c b/builtin/check-ignore.c\nindex 0240f99..be22bce 100644\n--- a/builtin/check-ignore.c\n+++ b/builtin/check-ignore.c\n@@ -12,7 +12,7 @@ static const char * const check_ignore_usage[] = {\n NULL\n };\n \n-static int null_term_line;\n+static int nul_term_line;\n \n static const struct option check_ignore_options[] = {\n \tOPT__QUIET(&quiet, N_(\"suppress progress reporting\")),\n@@ -20,8 +20,8 @@ static const struct option check_ignore_options[] = {\n \tOPT_GROUP(\"\"),\n \tOPT_BOOLEAN(0, \"stdin\", &stdin_paths,\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('z', NULL, &nul_term_line,\n+\t\t N_(\"input paths are terminated by a NUL character\")),\n \tOPT_END()\n };\n \n@@ -29,7 +29,7 @@ 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-\tif (!null_term_line) {\n+\tif (!nul_term_line) {\n \t\tif (!verbose) {\n \t\t\twrite_name_quoted(path, stdout, '\\n');\n \t\t} else {\n@@ -111,7 +111,7 @@ static int check_ignore_stdin_paths(const char *prefix)\n \tstruct strbuf buf, nbuf;\n \tchar **pathspec = NULL;\n \tsize_t nr = 0, alloc = 0;\n-\tint line_termination = null_term_line ? 0 : '\\n';\n+\tint line_termination = nul_term_line ? 0 : '\\n';\n \tint num_ignored;\n \n \tstrbuf_init(&buf, 0);\n@@ -150,7 +150,7 @@ int cmd_check_ignore(int argc, const char **argv, const char *prefix)\n \t\tif (argc > 0)\n \t\t\tdie(_(\"cannot specify pathnames with --stdin\"));\n \t} else {\n-\t\tif (null_term_line)\n+\t\tif (nul_term_line)\n \t\t\tdie(_(\"-z only makes sense with --stdin\"));\n \t\tif (argc == 0)\n \t\t\tdie(_(\"no path specified\"));\n-- \n1.8.3.2-911-g2c4daa5\n"},{"id":"223134","messageId":"1373609927-3244-3-git-send-email-gitster@pobox.com","threadId":"34414","inReplyTo":"1373609927-3244-1-git-send-email-gitster@pobox.com","subject":"[PATCH 2/4] check-attr: the name of the character is NUL, not NULL","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-12T06:18:45Z","receivedAt":"2013-07-12T06:18:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Signed-off-by: Junio C Hamano \n---\n builtin/check-attr.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/check-attr.c b/builtin/check-attr.c\nindex 075d01d..7cc9b5d 100644\n--- a/builtin/check-attr.c\n+++ b/builtin/check-attr.c\n@@ -13,14 +13,14 @@ N_(\"git check-attr --stdin [-z] [-a | --all | attr...] < \"),\n NULL\n };\n \n-static int null_term_line;\n+static int nul_term_line;\n \n static const struct option check_attr_options[] = {\n \tOPT_BOOLEAN('a', \"all\", &all_attrs, N_(\"report all attributes set on file\")),\n \tOPT_BOOLEAN(0, \"cached\", &cached_attrs, N_(\"use .gitattributes only from the index\")),\n \tOPT_BOOLEAN(0 , \"stdin\", &stdin_paths, N_(\"read file names from stdin\")),\n-\tOPT_BOOLEAN('z', NULL, &null_term_line,\n-\t\tN_(\"input paths are terminated by a null character\")),\n+\tOPT_BOOLEAN('z', NULL, &nul_term_line,\n+\t\t N_(\"input paths are terminated by a NUL character\")),\n \tOPT_END()\n };\n \n@@ -65,7 +65,7 @@ static void check_attr_stdin_paths(const char *prefix, int cnt,\n \tstruct git_attr_check *check)\n {\n \tstruct strbuf buf, nbuf;\n-\tint line_termination = null_term_line ? 0 : '\\n';\n+\tint line_termination = nul_term_line ? 0 : '\\n';\n \n \tstrbuf_init(&buf, 0);\n \tstrbuf_init(&nbuf, 0);\n-- \n1.8.3.2-911-g2c4daa5\n"},{"id":"223135","messageId":"1373609927-3244-4-git-send-email-gitster@pobox.com","threadId":"34414","inReplyTo":"1373609927-3244-1-git-send-email-gitster@pobox.com","subject":"[PATCH 3/4] check-ignore -z: a single -z should apply to both input and output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-12T06:18:46Z","receivedAt":"2013-07-12T06:18:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Unless a command has separate --nul-terminated-{input,output}\noptions, the --nul-terminated-records (-z) option should apply\nto both input and output for consistency. The caller knows that its\ninput paths may need to be protected for LF, and the program shows\nthese problematic paths to its output.\n\nThe code already did the right thing. Only the help text needs\nfixing.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/check-ignore.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/check-ignore.c b/builtin/check-ignore.c\nindex be22bce..03e509e 100644\n--- a/builtin/check-ignore.c\n+++ b/builtin/check-ignore.c\n@@ -21,7 +21,7 @@ static const struct option check_ignore_options[] = {\n \tOPT_BOOLEAN(0, \"stdin\", &stdin_paths,\n \t\t N_(\"read file names from stdin\")),\n \tOPT_BOOLEAN('z', NULL, &nul_term_line,\n-\t\t N_(\"input paths are terminated by a NUL character\")),\n+\t\t N_(\"terminate input and output records by a NUL character\")),\n \tOPT_END()\n };\n \n-- \n1.8.3.2-911-g2c4daa5\n"},{"id":"223136","messageId":"1373609927-3244-5-git-send-email-gitster@pobox.com","threadId":"34414","inReplyTo":"1373609927-3244-1-git-send-email-gitster@pobox.com","subject":"[PATCH 4/4] check-attr -z: a single -z should apply to both input and output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-12T06:18:47Z","receivedAt":"2013-07-12T06:18:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Unless a command has separate --nul-terminated-{input,output}\noptions, the --nul-terminated-records (-z) option should apply\nto both input and output for consistency. The caller knows that its\ninput paths may need to be protected for LF, and the program shows\nthese problematic paths to its output.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/git-check-attr.txt | 9 +++++++--\n builtin/check-attr.c | 14 +++++++++++---\n 2 files changed, 18 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-check-attr.txt b/Documentation/git-check-attr.txt\nindex 5abdbaa..760aca9 100644\n--- a/Documentation/git-check-attr.txt\n+++ b/Documentation/git-check-attr.txt\n@@ -31,8 +31,9 @@ OPTIONS\n \tRead file names from stdin instead of from the command-line.\n \n -z::\n-\tOnly meaningful with `--stdin`; paths are separated with a\n-\tNUL character instead of a linefeed character.\n+\tThe output format is modified to be machine-parseable.\n+\tIf `--stdin` is also given, input paths are separated\n+\twith a NUL character instead of a linefeed character.\n \n \\--::\n \tInterpret all preceding arguments as attributes and all following\n@@ -48,6 +49,10 @@ OUTPUT\n The output is of the form:\n COLON SP COLON SP LF\n \n+unless `-z` is in effect, in which case NUL is used as delimiter:\n+ NUL NUL NUL\n+\n+\n is the path of a file being queried, is an attribute\n being queried and can be either:\n \ndiff --git a/builtin/check-attr.c b/builtin/check-attr.c\nindex 7cc9b5d..cd46690 100644\n--- a/builtin/check-attr.c\n+++ b/builtin/check-attr.c\n@@ -20,7 +20,7 @@ static const struct option check_attr_options[] = {\n \tOPT_BOOLEAN(0, \"cached\", &cached_attrs, N_(\"use .gitattributes only from the index\")),\n \tOPT_BOOLEAN(0 , \"stdin\", &stdin_paths, N_(\"read file names from stdin\")),\n \tOPT_BOOLEAN('z', NULL, &nul_term_line,\n-\t\t N_(\"input paths are terminated by a NUL character\")),\n+\t\t N_(\"terminate input and output records by a NUL character\")),\n \tOPT_END()\n };\n \n@@ -38,8 +38,16 @@ static void output_attr(int cnt, struct git_attr_check *check,\n \t\telse if (ATTR_UNSET(value))\n \t\t\tvalue = \"unspecified\";\n \n-\t\tquote_c_style(file, NULL, stdout, 0);\n-\t\tprintf(\": %s: %s\\n\", git_attr_name(check[j].attr), value);\n+\t\tif (nul_term_line) {\n+\t\t\tprintf(\"%s%c\" /* path */\n+\t\t\t \"%s%c\" /* attrname */\n+\t\t\t \"%s%c\" /* attrvalue */,\n+\t\t\t file, 0, git_attr_name(check[j].attr), 0, value, 0);\n+\t\t} else {\n+\t\t\tquote_c_style(file, NULL, stdout, 0);\n+\t\t\tprintf(\": %s: %s\\n\", git_attr_name(check[j].attr), value);\n+\t\t}\n+\n \t}\n }\n \n-- \n1.8.3.2-911-g2c4daa5\n"},{"id":"223148","messageId":"CAPig+cRuLpNzYaQQBe2Cw1Y4OQbMjM6YYJtuGcDyXAcy4fnBdw@mail.gmail.com","threadId":"34414","inReplyTo":"1373609927-3244-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 0/4] Make check-{attr,ignore} -z consistent","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-07-12T06:55:26Z","receivedAt":"2013-07-12T06:55:26Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jul 12, 2013 at 2:18 AM, Junio C Hamano wrote:\n> A command that has to deal with input/output that may contain LF\n> needs to offer the \"-z\" (--nul-terminated-records) option, and if it\n> does not support separate --nul-terminated-{input,output} options,\n> the \"-z\" option should govern both input and output. A caller that\n> uses \"-z\" knows that the paths it feeds to these commands as input\n> may have LF that cannot be expressed in LF delimited input format,\n> and the output from these commands do contain the same paths, so\n> there is no way for their output to be expressed unambiguously for\n> an input that requires \"-z\".\n\nFWIW, applying to the entire series:\n\nReviewed-by: Eric Sunshine \n\n> Unfortunately, \"git check-attr -z\" was broken and ignored the option\n> on the output side. This is a backward-incompatible fix, so we may\n> need to add a \"checkAttr.brokenZ\" configuration to allow people to\n> keep the existing breakage on top of these fixes, and then flip the\n> default at Git 2.0 boundary (sometime early next year).\n>\n> Credit goes to Eric Sunshine for finding this discrepancy\n> ($gmane/230158).\n>\n> Junio C Hamano (4):\n> check-ignore: the name of the character is NUL, not NULL\n> check-attr: the name of the character is NUL, not NULL\n> check-ignore -z: a single -z should apply to both input and output\n> check-attr -z: a single -z should apply to both input and output\n>\n> Documentation/git-check-attr.txt | 9 +++++++--\n> builtin/check-attr.c | 20 ++++++++++++++------\n> builtin/check-ignore.c | 12 ++++++------\n> 3 files changed, 27 insertions(+), 14 deletions(-)\n>\n> --\n> 1.8.3.2-911-g2c4daa5\n"}]}