{"thread":{"id":"47586","subject":"[PATCH 0/2] the cat-file -e option doesn't work as documented","startedAt":"2018-01-10T12:57:54Z","lastAt":"2018-01-10T22:09:20Z","messageCount":7,"participants":["Ævar Arnfjörð Bjarmason","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"336376","messageId":"20180110125554.12582-1-avarab@gmail.com","threadId":"47586","inReplyTo":null,"subject":"[PATCH 0/2] the cat-file -e option doesn't work as documented","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-01-10T12:55:52Z","receivedAt":"2018-01-10T12:57:54Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The -e option to cat-file will emit output, after promising not to.\n\nWe should take either 1/2 or 2/2, but not both. I'm partial to just\ndocumenting the existing behavior and dropping 2/2, it's useful to\nknow if you passed in something that didn't look like a SHA-1.\n\nBut if others disagree we can drop 1/2 and take 2/2. Up to you.\n\nÆvar Arnfjörð Bjarmason (2):\n cat-file doc: document that -e will return some output\n cat-file: -e should not emit output on stderr\n\n Documentation/git-cat-file.txt | 7 ++++---\n builtin/cat-file.c | 8 ++++++--\n t/t1006-cat-file.sh | 7 +++++++\n 3 files changed, 17 insertions(+), 5 deletions(-)\n\n-- \n2.15.1.424.g9478a66081\n\n"},{"id":"336377","messageId":"20180110125554.12582-3-avarab@gmail.com","threadId":"47586","inReplyTo":"20180110125554.12582-1-avarab@gmail.com","subject":"[PATCH 2/2] cat-file: -e should not emit output on stderr","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-01-10T12:55:54Z","receivedAt":"2018-01-10T12:58:00Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change \"cat-file -e some-garbage\" to work as documented. Before it\nwould emit:\n\n $ git cat-file -e some-garbage; echo $?\n fatal: Not a valid object name some-garbage\n 128\n\nNow:\n\n $ ./git-cat-file -e some-garbage; echo $?\n 1\n\nThis is a change to longstanding behavior established in\n7950571ad7 (\"A few more options for git-cat-file\", 2005-12-03) when\nthe option was initially added, but we should go with the promise made\nin the documentation.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/cat-file.c | 8 ++++++--\n t/t1006-cat-file.sh | 7 +++++++\n 2 files changed, 13 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex f5fa4fd75a..75991788af 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -65,8 +65,12 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \t\tflags |= OBJECT_INFO_ALLOW_UNKNOWN_TYPE;\n \n \tif (get_oid_with_context(obj_name, GET_OID_RECORD_PATH,\n-\t\t\t\t &oid, &obj_context))\n-\t\tdie(\"Not a valid object name %s\", obj_name);\n+\t\t\t\t &oid, &obj_context)) {\n+\t\tif (opt == 'e')\n+\t\t\treturn 1;\n+\t\telse\n+\t\t\tdie(\"Not hello a valid object name %s\", obj_name);\n+\t}\n \n \tif (!path)\n \t\tpath = obj_context.path;\ndiff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\nindex b19f332694..c05a899bc4 100755\n--- a/t/t1006-cat-file.sh\n+++ b/t/t1006-cat-file.sh\n@@ -201,6 +201,13 @@ do\n '\n done\n \n+test_expect_success 'Providing -e should suppress all error output' '\n+\t! git cat-file -e some-garbage >stdout 2>stderr &&\n+\t>expect &&\n+\ttest_cmp expect stdout &&\n+\ttest_cmp expect stderr\n+'\n+\n for opt in t s e p\n do\n test_expect_success \"Passing -$opt with --follow-symlinks fails\" '\n-- \n2.15.1.424.g9478a66081\n\n"},{"id":"336378","messageId":"20180110125554.12582-2-avarab@gmail.com","threadId":"47586","inReplyTo":"20180110125554.12582-1-avarab@gmail.com","subject":"[PATCH 1/2] cat-file doc: document that -e will return some output","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-01-10T12:55:53Z","receivedAt":"2018-01-10T12:58:02Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The -e option added in 7950571ad7 (\"A few more options for\ngit-cat-file\", 2005-12-03) has always errored out with message on\nstderr saying that the provided object is malformed, currently:\n\n $ git cat-file -e malformed; echo $?\n fatal: Not a valid object name malformed\n 128\n\nA careful reader of this documentation would be mislead into thinking\nthe could write:\n\n if ! git cat-file -e \"$object\" [...]\n\nAs opposed to:\n\n if ! git cat-file -e \"$object\" 2>/dev/null [...]\n\nTo check whether some arbitrary $object string was both valid, and\npointed to an object that exists.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/git-cat-file.txt | 7 ++++---\n 1 file changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\nindex fb09cd69d6..f90f09b03f 100644\n--- a/Documentation/git-cat-file.txt\n+++ b/Documentation/git-cat-file.txt\n@@ -42,8 +42,9 @@ OPTIONS\n \t.\n \n -e::\n-\tSuppress all output; instead exit with zero status if \n-\texists and is a valid object.\n+\tExit with zero status if exists and is a valid\n+\tobject. If is of an invalid format exit with non-zero and\n+\temits an error on stderr.\n \n -p::\n \tPretty-print the contents of based on its type.\n@@ -168,7 +169,7 @@ If `-t` is specified, one of the .\n \n If `-s` is specified, the size of the in bytes.\n \n-If `-e` is specified, no output.\n+If `-e` is specified, no output, unless the is malformed.\n \n If `-p` is specified, the contents of are pretty-printed.\n \n-- \n2.15.1.424.g9478a66081\n\n"},{"id":"336421","messageId":"xmqqbmi11m2l.fsf@gitster.mtv.corp.google.com","threadId":"47586","inReplyTo":"20180110125554.12582-1-avarab@gmail.com","subject":"Re: [PATCH 0/2] the cat-file -e option doesn't work as documented","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-10T20:43:46Z","receivedAt":"2018-01-10T20:43:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> The -e option to cat-file will emit output, after promising not to.\n>\n> We should take either 1/2 or 2/2, but not both. I'm partial to just\n> documenting the existing behavior and dropping 2/2, it's useful to\n> know if you passed in something that didn't look like a SHA-1.\n>\n> But if others disagree we can drop 1/2 and take 2/2. Up to you.\n>\n> Ævar Arnfjörð Bjarmason (2):\n> cat-file doc: document that -e will return some output\n> cat-file: -e should not emit output on stderr\n>\n> Documentation/git-cat-file.txt | 7 ++++---\n> builtin/cat-file.c | 8 ++++++--\n> t/t1006-cat-file.sh | 7 +++++++\n> 3 files changed, 17 insertions(+), 5 deletions(-)\n\nI am kind of confused. \n\nWhen the doc there says \"no output\", I read it as \"no output\", and\nno other restriction (like suppressing an error diagnosis, which is\nnot even sent to the standard output stream).\n\n"},{"id":"336423","messageId":"xmqq7esp1lrl.fsf@gitster.mtv.corp.google.com","threadId":"47586","inReplyTo":"xmqqbmi11m2l.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/2] the cat-file -e option doesn't work as documented","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-10T20:50:22Z","receivedAt":"2018-01-10T20:50:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> I am kind of confused. \n>\n> When the doc there says \"no output\", I read it as \"no output\", and\n> no other restriction (like suppressing an error diagnosis, which is\n> not even sent to the standard output stream).\n\nAh, OK, so you were saying with 1/2 that \"output\" could mean\n\"something sent to standard error stream\" as well and can be helped\nwith a more explicit clarification. Makes sense to me (sort-of).\n\n"},{"id":"336428","messageId":"xmqqtvvtz95g.fsf@gitster.mtv.corp.google.com","threadId":"47586","inReplyTo":"20180110125554.12582-2-avarab@gmail.com","subject":"Re: [PATCH 1/2] cat-file doc: document that -e will return some output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-10T21:38:51Z","receivedAt":"2018-01-10T21:38:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> The -e option added in 7950571ad7 (\"A few more options for\n> git-cat-file\", 2005-12-03) has always errored out with message on\n> stderr saying that the provided object is malformed, currently:\n>\n> $ git cat-file -e malformed; echo $?\n> fatal: Not a valid object name malformed\n> 128\n>\n> A careful reader of this documentation would be mislead into thinking\n> the could write:\n>\n> if ! git cat-file -e \"$object\" [...]\n\nIt is arguable if such a reader is careful or careless. I'd rather drop\ns/careful // there ;-)\n\n> As opposed to:\n>\n> if ! git cat-file -e \"$object\" 2>/dev/null [...]\n>\n> To check whether some arbitrary $object string was both valid, and\n> pointed to an object that exists.\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> Documentation/git-cat-file.txt | 7 ++++---\n> 1 file changed, 4 insertions(+), 3 deletions(-)\n"},{"id":"336431","messageId":"xmqqk1wpz7qu.fsf@gitster.mtv.corp.google.com","threadId":"47586","inReplyTo":"xmqqtvvtz95g.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/2] cat-file doc: document that -e will return some output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-10T22:09:13Z","receivedAt":"2018-01-10T22:09:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> The -e option added in 7950571ad7 (\"A few more options for\n>> git-cat-file\", 2005-12-03) has always errored out with message on\n>> stderr saying that the provided object is malformed, currently:\n>>\n>> $ git cat-file -e malformed; echo $?\n>> fatal: Not a valid object name malformed\n>> 128\n>>\n>> A careful reader of this documentation would be mislead into thinking\n>> the could write:\n>>\n>> if ! git cat-file -e \"$object\" [...]\n>\n> It is arguable if such a reader is careful or careless. I'd rather drop\n> s/careful // there ;-)\n\nActually the phrasing around here was a bit strange, and I ended up\nrewriting a bit more.\n\n cat-file doc: document that -e will return some output\n \n The -e option added in 7950571ad7 (\"A few more options for\n git-cat-file\", 2005-12-03) has always errored out with message on\n stderr saying that the provided object is malformed, like this:\n \n $ git cat-file -e malformed; echo $?\n fatal: Not a valid object name malformed\n 128\n \n A reader of this documentation may be misled into thinking that\n \n if ! git cat-file -e \"$object\" [...]\n \n as opposed to:\n \n if ! git cat-file -e \"$object\" 2>/dev/null [...]\n \n is sufficient to implement a truly silent test that checks whether\n some arbitrary $object string was both valid, and pointed to an\n object that exists.\n \n Signed-off-by: Ævar Arnfjörð Bjarmason \n Signed-off-by: Junio C Hamano \n"}]}