git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Taylor Blau <me@ttaylorr.com>
Cc: git@vger.kernel.org, "René Scharfe" <l.s.r@web.de>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: Re: [PATCH 4/6] t: use hash-object --literally when created malformed objects
Date: Wed, 18 Jan 2023 21:06:40 -0500	[thread overview]
Message-ID: <Y8ilsLkR7R2uu542@coredump.intra.peff.net> (raw)
In-Reply-To: <Y8hiWGOArYqzen58@nand.local>

On Wed, Jan 18, 2023 at 04:19:20PM -0500, Taylor Blau wrote:

> > This patch is worth looking at because it shows the kinds of things the
> > new hash-object from patch 6 will reject.
> 
> Obviously we could avoid this patch entirely by making the new behavior
> of fscking the incoming objects hidden behind a `--fsck` flag or
> something. But I think the decision not to is a good one.
> 
> We already have `--literally`, and it makes sense that passing that
> should let us write anything, and that not passing it should perform
> some validity checks. But I think exactly *what* those checks are is
> ambiguous enough that the absence of `--literally` implying fsck checks
> isn't out of the question.
> 
> You address this in the last patch more thoroughly, but I figure that it
> is worth stating some of this here during review to indicate that I
> think the direction you pursued here is a good one.

Thanks for raising this, I think it's a good thing to consider. I didn't
even really think about making it a new option, since we already do
quality checks (and loosen them via --literally). This just seemed like
more of the same.

So yeah, if there were people who really wanted to distinguish between
the severity of the old checks and the new ones, we can add --fsck (or
even default to having it on, and disable it with --no-fsck to get the
old checks). But I just see little point in that.

One thing we _could_ support that my patch doesn't (I think; I didn't
test very deeply here) is respecting individual fsck.msgType config
variables. Again, I don't really see much point there. If you know you
are producing garbage, then just say --literally. The type-specific ones
are useful when you have to hold your nose and accept somebody else's
historical garbage, and you want to limit the damage as much as
possible.

-Peff

  reply	other threads:[~2023-01-19  2:06 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-18 20:35 [RFC/PATCH 0/6] hash-object: use fsck to check objects Jeff King
2023-01-18 20:35 ` [PATCH 1/6] t1007: modernize malformed object tests Jeff King
2023-01-18 21:13   ` Taylor Blau
2023-01-18 20:35 ` [PATCH 2/6] t1006: stop using 0-padded timestamps Jeff King
2023-01-18 20:36 ` [PATCH 3/6] t7030: stop using invalid tag name Jeff King
2023-01-18 20:41 ` [PATCH 4/6] t: use hash-object --literally when created malformed objects Jeff King
2023-01-18 21:19   ` Taylor Blau
2023-01-19  2:06     ` Jeff King [this message]
2023-01-18 20:43 ` [PATCH 5/6] fsck: provide a function to fsck buffer without object struct Jeff King
2023-01-18 21:24   ` Taylor Blau
2023-01-19  2:07     ` Jeff King
2023-01-18 20:44 ` [PATCH 6/6] hash-object: use fsck for object checks Jeff King
2023-01-18 21:34   ` Taylor Blau
2023-01-19  2:31     ` Jeff King
2023-02-01 12:50   ` Jeff King
2023-02-01 13:08     ` Ævar Arnfjörð Bjarmason
2023-02-01 20:41     ` Junio C Hamano
2023-01-18 20:46 ` [RFC/PATCH 0/6] hash-object: use fsck to check objects Jeff King
2023-01-18 20:59 ` Junio C Hamano
2023-01-18 21:38   ` Taylor Blau
2023-01-19  2:03     ` Jeff King
2023-01-19  1:39 ` Jeff King
2023-01-19 23:13   ` [PATCH 7/6] fsck: do not assume NUL-termination of buffers Jeff King
2023-01-19 23:58     ` Junio C Hamano
2023-01-21  9:36   ` [RFC/PATCH 0/6] hash-object: use fsck to check objects René Scharfe
2023-01-22  7:48     ` Jeff King
2023-01-22 11:39       ` René Scharfe
2023-02-01 14:06         ` Ævar Arnfjörð Bjarmason

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=Y8ilsLkR7R2uu542@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=l.s.r@web.de \
    --cc=me@ttaylorr.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).