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

From: Taylor Blau <me@ttaylorr.com>
Date: Jan 18, 2023, 21:19 UTC
Message-ID: <Y8hiWGOArYqzen58@nand.local>
In-Reply-To: <Y8hZlN9rRg1msc0L@coredump.intra.peff.net>

On Wed, Jan 18, 2023 at 03:41:56PM -0500, Jeff King wrote:

Show 6 quoted lines

> Many test scripts use hash-object to create malformed objects to see how
> we handle the results in various commands. In some cases we already have
> to use "hash-object --literally", because it does some rudimentary
> quality checks. But let's use "--literally" more consistently to
> future-proof these tests against hash-object learning to be more
> careful.

Heh, I suppose this is a good illustration of how loose our checks our even without `--literally` ;-).

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

Show 14 quoted lines

>  t/t1450-fsck.sh                 | 28 ++++++++++++++--------------
>  t/t4054-diff-bogus-tree.sh      |  2 +-
>  t/t4058-diff-duplicates.sh      |  2 +-
>  t/t4212-log-corrupt.sh          |  4 ++--
>  t/t5302-pack-index.sh           |  2 +-
>  t/t5504-fetch-receive-strict.sh |  2 +-
>  t/t5702-protocol-v2.sh          |  2 +-
>  t/t6300-for-each-ref.sh         |  2 +-
>  t/t7509-commit-authorship.sh    |  2 +-
>  t/t7510-signed-commit.sh        |  2 +-
>  t/t7528-signed-commit-ssh.sh    |  2 +-
>  t/t8003-blame-corner-cases.sh   |  2 +-
>  t/t9350-fast-export.sh          |  2 +-
>  13 files changed, 27 insertions(+), 27 deletions(-)

And these all look good, too. Each of the spots you touch here is limited to replacing "git hash-object" with "git hash-object --literally".

Thanks, Taylor