git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
* [PATCH 0/2] the cat-file -e option doesn't work as documented
@ 2018-01-10 12:55 Ævar Arnfjörð Bjarmason
  2018-01-10 12:55 ` [PATCH 1/2] cat-file doc: document that -e will return some output Ævar Arnfjörð Bjarmason
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2018-01-10 12:55 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, H . Peter Anvin, Ævar Arnfjörð Bjarmason

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] cat-file doc: document that -e will return some output
  2018-01-10 12:55 [PATCH 0/2] the cat-file -e option doesn't work as documented Ævar Arnfjörð Bjarmason
@ 2018-01-10 12:55 ` Ævar Arnfjörð Bjarmason
  2018-01-10 21:38   ` Junio C Hamano
  2018-01-10 12:55 ` [PATCH 2/2] cat-file: -e should not emit output on stderr Ævar Arnfjörð Bjarmason
  2018-01-10 20:43 ` [PATCH 0/2] the cat-file -e option doesn't work as documented Junio C Hamano
  2 siblings, 1 reply; 7+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2018-01-10 12:55 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, H . Peter Anvin, Ævar Arnfjörð Bjarmason

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.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-cat-file.txt | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index fb09cd69d6..f90f09b03f 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/2] cat-file: -e should not emit output on stderr
  2018-01-10 12:55 [PATCH 0/2] the cat-file -e option doesn't work as documented Ævar Arnfjörð Bjarmason
  2018-01-10 12:55 ` [PATCH 1/2] cat-file doc: document that -e will return some output Ævar Arnfjörð Bjarmason
@ 2018-01-10 12:55 ` Ævar Arnfjörð Bjarmason
  2018-01-10 20:43 ` [PATCH 0/2] the cat-file -e option doesn't work as documented Junio C Hamano
  2 siblings, 0 replies; 7+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2018-01-10 12:55 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, H . Peter Anvin, Ævar Arnfjörð Bjarmason

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.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/cat-file.c  | 8 ++++++--
 t/t1006-cat-file.sh | 7 +++++++
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index f5fa4fd75a..75991788af 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -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;
diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index b19f332694..c05a899bc4 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] the cat-file -e option doesn't work as documented
  2018-01-10 12:55 [PATCH 0/2] the cat-file -e option doesn't work as documented Ævar Arnfjörð Bjarmason
  2018-01-10 12:55 ` [PATCH 1/2] cat-file doc: document that -e will return some output Ævar Arnfjörð Bjarmason
  2018-01-10 12:55 ` [PATCH 2/2] cat-file: -e should not emit output on stderr Ævar Arnfjörð Bjarmason
@ 2018-01-10 20:43 ` Junio C Hamano
  2018-01-10 20:50   ` Junio C Hamano
  2 siblings, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2018-01-10 20:43 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: git, H . Peter Anvin

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> 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(-)

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).


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] the cat-file -e option doesn't work as documented
  2018-01-10 20:43 ` [PATCH 0/2] the cat-file -e option doesn't work as documented Junio C Hamano
@ 2018-01-10 20:50   ` Junio C Hamano
  0 siblings, 0 replies; 7+ messages in thread
From: Junio C Hamano @ 2018-01-10 20:50 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: git, H . Peter Anvin

Junio C Hamano <gitster@pobox.com> writes:

> 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).


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] cat-file doc: document that -e will return some output
  2018-01-10 12:55 ` [PATCH 1/2] cat-file doc: document that -e will return some output Ævar Arnfjörð Bjarmason
@ 2018-01-10 21:38   ` Junio C Hamano
  2018-01-10 22:09     ` Junio C Hamano
  0 siblings, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2018-01-10 21:38 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: git, H . Peter Anvin

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> 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" [...]

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

> 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.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  Documentation/git-cat-file.txt | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] cat-file doc: document that -e will return some output
  2018-01-10 21:38   ` Junio C Hamano
@ 2018-01-10 22:09     ` Junio C Hamano
  0 siblings, 0 replies; 7+ messages in thread
From: Junio C Hamano @ 2018-01-10 22:09 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: git, H . Peter Anvin

Junio C Hamano <gitster@pobox.com> writes:

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> 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" [...]
>
> 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.
    
    Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-10 12:55 [PATCH 0/2] the cat-file -e option doesn't work as documented Ævar Arnfjörð Bjarmason
2018-01-10 12:55 ` [PATCH 1/2] cat-file doc: document that -e will return some output Ævar Arnfjörð Bjarmason
2018-01-10 21:38   ` Junio C Hamano
2018-01-10 22:09     ` Junio C Hamano
2018-01-10 12:55 ` [PATCH 2/2] cat-file: -e should not emit output on stderr Ævar Arnfjörð Bjarmason
2018-01-10 20:43 ` [PATCH 0/2] the cat-file -e option doesn't work as documented Junio C Hamano
2018-01-10 20:50   ` Junio C Hamano

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox