ds/omit-trailing-hash-in-index (was: What's cooking in git.git (Dec 2022, #06; Sun, 18))

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Dec 19, 2022, 10:49 UTC
Message-ID: <221219.86y1r3zmrv.gmgdl@evledraar.gmail.com>
In-Reply-To: <xmqqh6xtw4uw.fsf@gitster.g>

On Sun, Dec 18 2022, Junio C Hamano wrote:

Show 11 quoted lines

> * ds/omit-trailing-hash-in-index (2022-12-17) 4 commits
>  - features: feature.manyFiles implies fast index writes
>  - test-lib-functions: add helper for trailing hash
>  - read-cache: add index.skipHash config option
>  - hashfile: allow skipping the hash function
>
>  Introduce an optional configuration to allow the trailing hash that
>  protects the index file from bit flipping.
>
>  Will merge to 'next'?
>  source: <pull.1439.v4.git.1671204678.gitgitgadget@gmail.com>

I've been following this closely & reviewing it. I think the end-state is probably good, but noted in [1] that the intermediate progression equates bad config with "true", so:

	git -c index.skipHash=blahblah status

Enables it, fixing that is trivial, and probably worth a re-roll.

The "probably" above is then because the patches seemingly try to make this compatible with different config for submodules, but there's no tests for submodule interaction, so that may or may not work.

Normally we could just trust the "struct repository *" parameter we get, but in this case it's "istate->repo", which (as I showed in the v3 feedback[2]) is sometimes NULL.

Perhaps that NULL is benign, and perhaps it's a symptom that would result in a bug exposed by this topic bug. I don't know, but think that before this lands we really should have tests to tease out those interactions.

1. https://lore.kernel.org/git/221216.86sfhf1gbc.gmgdl@evledraar.gmail.com/
2. https://lore.kernel.org/git/221215.865yec3b1j.gmgdl@evledraar.gmail.com/