threads / discuss / 26714

[Buglet] "git show <random-40-hexdigit>" gives an unclear error message

Subject: [Buglet] "git show <random-40-hexdigit>" gives an unclear error message

## tl;dr

3 messages between Mar 11, 2011 and Mar 14, 2011.

replies: 2people: 2as markdown or json

Junio C Hamano· Mar 11, 2011, 18:28 UTC · lore

Here is one who wants to tackle potentially a low-hanging fruit.

  $ git cat-file -p 4cd96ce446ab653325a0e47fad73e94c99c57dd2
  error: unable to find 4cd96ce446ab653325a0e47fad73e94c99c57dd2
  fatal: Not a valid object name 4cd96ce446ab653325a0e47fad73e94c99c57dd2

This looks sensible and understandable, even though you could argue that we don't have to say the same thing twice. In the same repository:

  $ git show 4cd96ce446ab653325a0e47fad73e94c99c57dd3
  fatal: bad object 4cd96ce446ab653325a0e47fad73e94c99c57dd3

This is bad, as it sounds as if we found the named object but it is corrupt.

Jakob Pfender· Mar 14, 2011, 15:11 UTC · re: Junio C Hamano · lore

[PATCH] revision.c: Clarify error message for missing objects

[Sorry Junio, accidentally sent this to just you the first time around]

If "git show" is invoked on a hash for a file that doesn't exist, the produced error message was "fatal: bad object <hash>". This is misleading as the object isn't bad, but missing. As a matter of fact, if an object is bad, this error message is never produced, because parse_object() terminates with its own error message if the object is corrupt.

This patch introduces a check to see whether an object exists (with the appropriate error message if it doesn't) and removes the unnecessary and misleading original error message if parse_object() fails.

Signed-off-by: Jakob Pfender <jpfender@elegosoft.com>
---
  revision.c |    4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/revision.c b/revision.c
index 86d2470..085aac2 100644
--- a/revision.c
+++ b/revision.c
@@ -173,9 +173,9 @@ static struct object *get_reference(struct rev_info 
*revs, const char *name, con
  {
      struct object *object;

+    if (sha1_object_info(sha1, NULL) < 0)
+        die("Not a valid object name %s", name);
      object = parse_object(sha1);
-    if (!object)
-        die("bad object %s", name);
      object->flags |= flags;
      return object;
  }

-- 
1.7.0.4

Junio C Hamano· Mar 14, 2011, 17:40 UTC · re: Jakob Pfender · lore

Re: [PATCH] revision.c: Clarify error message for missing objects

Jakob Pfender <jpfender@elegosoft.com> writes:

> This patch introduces a check to see whether an object exists (with the
> appropriate error message if it doesn't) and removes the unnecessary and
> misleading original error message if parse_object() fails.

How does this interact with object replacement?

I suspect the real issue is in read_sha1_file_repl() that says:

    /*
     * This function dies on corrupt objects; the callers who want to
     * deal with them should arrange to call read_object() and give error
     * messages themselves.
     */

without giving sufficient support for the callers that wish to use read_object() themselves to implement the object replacement logic.

Show 22 quoted lines

> Signed-off-by: Jakob Pfender <jpfender@elegosoft.com>
> ---
>  revision.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/revision.c b/revision.c
> index 86d2470..085aac2 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -173,9 +173,9 @@ static struct object *get_reference(struct
> rev_info *revs, const char *name, con
>  {
>      struct object *object;
>
> +    if (sha1_object_info(sha1, NULL) < 0)
> +        die("Not a valid object name %s", name);
>      object = parse_object(sha1);
> -    if (!object)
> -        die("bad object %s", name);
>      object->flags |= flags;
>      return object;
>  }