the cat-file -e option doesn't work as documented

The -e option to cat-file will emit output, after promising not to.

We should take either 1/2 or 2/2, but not both. I'm partial to just documenting the existing behavior and dropping 2/2, it's useful to know if you passed in something that didn't look like a SHA-1.

But if others disagree we can drop 1/2 and take 2/2. Up to you.

Ævar Arnfjörð Bjarmason (2):
cat-file doc: document that -e will return some output
cat-file: -e should not emit output on stderr

 Documentation/git-cat-file.txt | 7 ++++---
 builtin/cat-file.c             | 8 ++++++--
 t/t1006-cat-file.sh            | 7 +++++++
 3 files changed, 17 insertions(+), 5 deletions(-)

2.15.1.424.g9478a66081

The -e option added in 7950571ad7 ("A few more options for git-cat-file", 2005-12-03) has always errored out with message on stderr saying that the provided object is malformed, currently:

    $ git cat-file -e malformed; echo $?
    fatal: Not a valid object name malformed
    128

A careful reader of this documentation would be mislead into thinking the could write:

    if ! git cat-file -e "$object" [...]

As opposed to:

    if ! git cat-file -e "$object" 2>/dev/null [...]

To check whether some arbitrary $object string was both valid, and pointed to an object that exists.

---
 Documentation/git-cat-file.txt | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

index fb09cd69d6..f90f09b03f 100644
@@ -42,8 +42,9 @@ OPTIONS
 	<object>.
 
 -e::
-	Suppress all output; instead exit with zero status if <object>
-	exists and is a valid object.
+	Exit with zero status if <object> exists and is a valid
+	object. If <object> is of an invalid format exit with non-zero and
+	emits an error on stderr.
 
 -p::
 	Pretty-print the contents of <object> based on its type.

@@ -168,7 +169,7 @@ If `-t` is specified, one of the <type>.
 
 If `-s` is specified, the size of the <object> in bytes.
 
-If `-e` is specified, no output.
+If `-e` is specified, no output, unless the <object> is malformed.
 
 If `-p` is specified, the contents of <object> are pretty-printed.
 

2.15.1.424.g9478a66081

It is arguable if such a reader is careful or careless. I'd rather drop s/careful // there ;-)

Actually the phrasing around here was a bit strange, and I ended up rewriting a bit more.

cat-file doc: document that -e will return some output

The -e option added in 7950571ad7 ("A few more options for git-cat-file", 2005-12-03) has always errored out with message on stderr saying that the provided object is malformed, like this:

$ git cat-file -e malformed; echo $?
fatal: Not a valid object name malformed
128

A reader of this documentation may be misled into thinking that

if ! git cat-file -e "$object" [...]

as opposed to:

if ! git cat-file -e "$object" 2>/dev/null [...]

is sufficient to implement a truly silent test that checks whether some arbitrary $object string was both valid, and pointed to an object that exists.

Change "cat-file -e some-garbage" to work as documented. Before it would emit:

    $ git cat-file -e some-garbage; echo $?
    fatal: Not a valid object name some-garbage
    128

Now:

    $ ./git-cat-file -e some-garbage; echo $?
    1

This is a change to longstanding behavior established in 7950571ad7 ("A few more options for git-cat-file", 2005-12-03) when the option was initially added, but we should go with the promise made in the documentation.

---
 builtin/cat-file.c  | 8 ++++++--
 t/t1006-cat-file.sh | 7 +++++++
 2 files changed, 13 insertions(+), 2 deletions(-)

index f5fa4fd75a..75991788af 100644
@@ -65,8 +65,12 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 		flags |= OBJECT_INFO_ALLOW_UNKNOWN_TYPE;
 
 	if (get_oid_with_context(obj_name, GET_OID_RECORD_PATH,
-				 &oid, &obj_context))
-		die("Not a valid object name %s", obj_name);
+				 &oid, &obj_context)) {
+		if (opt == 'e')
+			return 1;
+		else
+			die("Not hello a valid object name %s", obj_name);
+	}
 
 	if (!path)
 		path = obj_context.path;

index b19f332694..c05a899bc4 100755
@@ -201,6 +201,13 @@ do
     '
 done
 
+test_expect_success 'Providing -e should suppress all error output' '
+	! git cat-file -e some-garbage >stdout 2>stderr &&
+	>expect &&
+	test_cmp expect stdout &&
+	test_cmp expect stderr
+'
+
 for opt in t s e p
 do
     test_expect_success "Passing -$opt with --follow-symlinks fails" '

2.15.1.424.g9478a66081

I am kind of confused.

When the doc there says "no output", I read it as "no output", and no other restriction (like suppressing an error diagnosis, which is not even sent to the standard output stream).

Ah, OK, so you were saying with 1/2 that "output" could mean "something sent to standard error stream" as well and can be helped with a more explicit clarification. Makes sense to me (sort-of).