Re: [PATCH 4/6] t: use hash-object --literally when created malformed objects

From: Jeff King <peff@peff.net>
Date: Jan 19, 2023, 02:06 UTC
Message-ID: <Y8ilsLkR7R2uu542@coredump.intra.peff.net>
In-Reply-To: <Y8hiWGOArYqzen58@nand.local>

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

Show 16 quoted lines

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