On 12/19/22 5:49 AM, Ævar Arnfjörð Bjarmason wrote: > > On Sun, Dec 18 2022, Junio C Hamano wrote: > >> * 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@xxxxxxxxx> > > 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. Fixing it is trivial, but as you say it is correct in the final version so I don't think this triviality is 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. This, and other feedback you've given around 'struct repository *' values makes me think you are not aware of the current state of the submodule work. I'm also a bit out-of-date, but my understanding is that the conversion to stop using the_repository everywhere is only halfway complete, which makes it difficult to properly test things with multiple 'struct repository *' pointers. However, the best thing we can do is to use whatever local repository we have, so we don't contribute further to the problem. The fact that istate->repo is sometimes NULL is a separate issue, but is generally unrelated to the subject at hand and should be fixed by another topic, not used to block this one. Thanks, -Stolee