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

quoted

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