Re: ds/omit-trailing-hash-in-index (was: What's cooking in git.git (Dec 2022, #06; Sun, 18))
- From
Derrick Stolee <derrickstolee@github.com>
- Date
- Dec 20, 2022, 13:39 UTC
- Message-ID
- <0113fed3-5dca-6a31-9b9d-becd009c2fca@github.com>
- In-Reply-To
- <221219.86y1r3zmrv.gmgdl@evledraar.gmail.com>
On 12/19/22 5:49 AM, Ævar Arnfjörð Bjarmason wrote:
Show 22 quoted lines
> > 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@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.
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.
Show 7 quoted lines
> 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