Re: [PATCH] git-cat-file: fix output when format string contains no variables

Sven Brauch <svenbrauch@googlemail.com> writes:

Thanks; first some procedural issues:

 - Omit the first line "From 2e7b..."; that does not belong to any
   patch submission (it is a separator used between messages in the
   mailbox formatted file "format-patch --stdout" produces).

 - The second line "From: Sven..." records exactly the same address
   as the e-mail you are sending out, so it should be omitted as
   well.

 - The third line "Date: ..." is not the time we the general public
   sees your fix for the first time, which is what "Date: " of your
   e-mail header already records, so we do not need it either.

 - And the last one "Subject: ..." is redundant; we can see it in
   your e-mail header.

In general, the latter three lines are produced by format-patch to help you fill header fields in the MUA of your choice by cutting (not copying) and pasting. Unless there is a valid reason to have values different from what recipients would see in the e-mail header (and there often isn't, unless you are forwarding somebody else's patch, in which case you may want to use "From: ", or you are responding to an ongoing discussion with a patch, in which case you may want to use "Subject: "), please remove them after copying them out to your e-mail header.

An excellent log message. It would have been even better to add a new test to t1006 based on this reproduction recipe.

I think the real problem is that sha1_loose_object_info() is called by sha1_object_info_extended(), when it does not find a cached or a packed object, and the callee assumes that it is asked to fill in only the requested pieces of information while the caller does not even bother to check if such an object actually exists.

How about doing it like the attached instead?

-- >8 --
Subject: sha1_loose_object_info(): do not return success on missing object

Since 052fe5ea (sha1_loose_object_info: make type lookup optional, 2013-07-12), sha1_loose_object_info() returns happily without checking if the object in question exists, which is not what the the caller sha1_object_info_extended() expects; the caller does not even bother checking the existence of the object itself.

Noticed-by: Sven Brauch <svenbrauch@googlemail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

  Oh, by the way, there is this one iffy bit in batch_one_object():

          if (get_sha1(obj_name, data->sha1)) {
                  printf("%s missing\n", obj_name);
                  fflush(stdout);
                  return 0;
          }

  At this point, the object _may_ be missing, but the obj_name may be
  malformed, so saying "missing" is not strictly correct.  If, for
  example, you misspelled the name of the master branch, you would get
  this:

          $ echo mastre | git cat-file --batch-check=foo
          mastre missing

  I however doubt that it is a good idea to reword this message by
  adding a logic to tell misspelled object name and missing object
  name apart.  The users of "cat-file --batch-check" are not
  expecting to be able to distinguish these two classes of errors
  anyway.

 sha1_file.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)