git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Han-Wen Nienhuys <hanwen@google.com>, git <git@vger.kernel.org>,
	Jeff King <peff@peff.net>, Han-Wen Nienhuys <hanwenn@gmail.com>,
	karthik nayak <karthik.188@gmail.com>,
	Eric Sunshine <sunshine@sunshineco.com>
Subject: Re: Options like hash-object --literally and cat-file --allow-unknown-type
Date: Wed, 19 May 2021 00:16:01 +0900	[thread overview]
Message-ID: <xmqqtun0139q.fsf@gitster.g> (raw)
In-Reply-To: <87r1i4qf4h.fsf@evledraar.gmail.com> ("Ævar Arnfjörð Bjarmason"'s message of "Tue, 18 May 2021 16:08:18 +0200")

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

> So you might expect it to accept an --allow-unknown-type, but it
> doesn't. I could add support for that, but I think it would be stupid.
>
> Why should you need to restart a "cat-file --batch" just because you
> encounter a bad object? Just .. print it, we can do that safely. I
> really don't see the point of having --allow-unknown-type at all. Ditto
> for the --batch-check mode.
>
> I mean, having read all the code I think I know why it's there. I think
> It's because there was no way to ferry the information up from
> object-file.c before my yet-to-be-submitted patches, so the solution was
> to pass down a flag saying "please don't die()".
>
> But is it something that anyone thinks is a good idea in the abstract? I
> don't see why I shouldn't just document something like:
>
>     Older versions of "cat-file" used to require an --allow-unknown-type
>     flag to emit information about objects of unknown types. This is no
>     longer required or the default. If you'd like to die on anything
>     except known types (e.g. to die instead of bothering with parsing a
>     "bad" type that possibly has spaces in it in the --batch output)
>     supply --no-allow-unknown-type.
>
> What do you think?

Thanks for thinking things through.

My knee-jerk reaction is

 - As long as "cat-file -t $thing" exits with non-zero status for an
   invalid thing, which was crafted using hash-object --literally,
   reporting the typename it read from the object header to its
   standard output would be fine without "--allow-unknown-type".
   But scripts would be upset if it suddenly started to return with
   zero status when asked to check what type of object the $thing
   is.

 - For "cat-file --batch[-check]", I am on the fence.  A script may
   break because it is not prepared to see anything but four
   existing types (so it might even say "do X if it is a blob, do Y
   if it is a tree, do Z if it is a tag, and do W for everything
   else" and expect/assume that W will see only commits), so failing
   without --allow may still be a prudent thing to do.  We could
   declare that such a script is already broken, even though it
   would not change the fact that the user has been successfully
   using it reliably.

With your new "we can now bubble necessary information up from the
object-file.c layer without dying" change, it might make it easier
to arrange so that object-file.c layer would never die and I do not
think I have any objection against such a plan.  The implementation
of "--[no-]allow-unknown-type" would have to be migrated to the
caller at the higher level to decide what to do when it learns that
the object it asked the object_info() about is of an invalid type.
And the choice of the default would become easier to change later
with such a change to the lower layer.  But I am not sure if that
warrants switching of the default.

  reply	other threads:[~2021-05-18 15:16 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-18 14:08 Options like hash-object --literally and cat-file --allow-unknown-type Ævar Arnfjörð Bjarmason
2021-05-18 15:16 ` Junio C Hamano [this message]
2021-05-18 18:42   ` Ævar Arnfjörð Bjarmason
2021-05-19  0:25     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqtun0139q.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=hanwen@google.com \
    --cc=hanwenn@gmail.com \
    --cc=karthik.188@gmail.com \
    --cc=peff@peff.net \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).