git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 4/4] features: feature.manyFiles implies fast index writes

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Dec 12, 2022, 18:27 UTC
Message-ID
<221212.86v8mg4gr2.gmgdl@evledraar.gmail.com>
In-Reply-To
<1b2aa60c-633f-0a7f-e8e3-5d0f01ff94b6@github.com>
On Mon, Dec 12 2022, Derrick Stolee wrote:
Show 46 quoted lines
> On 12/7/2022 5:30 PM, Ævar Arnfjörð Bjarmason wrote:
>> 
>> On Wed, Dec 07 2022, Derrick Stolee via GitGitGadget wrote:
>> 
>>> From: Derrick Stolee <derrickstolee@github.com>
>>> [...]
>>> diff --git a/read-cache.c b/read-cache.c
>>> index fb4d6fb6387..1844953fba7 100644
>>> --- a/read-cache.c
>>> +++ b/read-cache.c
>>> @@ -2923,12 +2923,13 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
>>>  	int ieot_entries = 1;
>>>  	struct index_entry_offset_table *ieot = NULL;
>>>  	int nr, nr_threads;
>>> -	int skip_hash;
>>>  
>>>  	f = hashfd(tempfile->fd, tempfile->filename.buf);
>>>  
>>> -	if (!git_config_get_maybe_bool("index.skiphash", &skip_hash))
>>> -		f->skip_hash = skip_hash;
>>> +	if (istate->repo) {
>>> +		prepare_repo_settings(istate->repo);
>>> +		f->skip_hash = istate->repo->settings.index_skip_hash;
>>> +	}
>> 
>> Urm, are we ever going to find ourselves in a situation where:
>> 
>>  * We have read the settings for the_repository
>>  * We have an index we're about to write out as our "main index", but
>>    the istate->repo *isn't* the_repository.
>>  * Even then, wouldn't the two copies of the repos have read the same
>>    repo settings?
>> 
>> But maybe there's a really obvious submodule / worktree / whatever edge
>> case I'm missing.
>> 
>> But if not, shouldn't we just always read/write this from
>> the_repository?
>
> I don't understand your concern. We call prepare_repo_settings(istate->repo)
> just before using these settings, so we are using whatever repository-local
> config we have available to us.
>
> If you're thinking that we could be writing an index but istate->repo is
> somehow the "wrong" repo, then that is a larger problem. This patch is
> doing the best thing it can with the information it is given.
It's not a concern, just confusion :)

In the preceding step (and this is still the case in your v2) we used git_config_get_maybe_bool(), if we meant to use istate->repo shouldn't we have used repo_config_get_maybe_bool() to begin with?

And will we ever get !istate->repo? If not should we BUG() here? Otherwise the 4/4 changes this to a state where we'll no longer read the index.skipHash setting if that "repo" is NULL, but our previous the_repository was non-NULL...

Previous: Derrick StoleeNext: Junio C Hamano
Message 20 of 59 in “Optionally skip hashing index on write”
  1. 0/4 Optionally skip hashing index on writeDerrick Stolee via GitGitGadget, Dec 7, 2022
  2. 2/4 read-cache: add index.skipHash config optionDerrick Stolee via GitGitGadget, Dec 7, 2022
  3. Eric SunshineDec 7, 2022
  4. Derrick StoleeDec 12, 2022
  5. Eric SunshineDec 12, 2022
  6. Ævar Arnfjörð BjarmasonDec 7, 2022
  7. Ævar Arnfjörð BjarmasonDec 7, 2022
  8. Junio C HamanoDec 8, 2022
  9. Derrick StoleeDec 12, 2022
  10. Ævar Arnfjörð BjarmasonDec 12, 2022
  11. 1/4 hashfile: allow skipping the hash functionDerrick Stolee via GitGitGadget, Dec 7, 2022
  12. Ævar Arnfjörð BjarmasonDec 7, 2022
  13. Jeff KingDec 8, 2022
  14. 3/4 test-lib-functions: add helper for trailing hashDerrick Stolee via GitGitGadget, Dec 7, 2022
  15. Ævar Arnfjörð BjarmasonDec 7, 2022
  16. Derrick StoleeDec 12, 2022
  17. 4/4 features: feature.manyFiles implies fast index writesDerrick Stolee via GitGitGadget, Dec 7, 2022
  18. Ævar Arnfjörð BjarmasonDec 7, 2022
  19. Derrick StoleeDec 12, 2022
  20. Ævar Arnfjörð BjarmasonDec 12, 2022
  21. Junio C HamanoDec 7, 2022
  22. Ævar Arnfjörð BjarmasonDec 7, 2022
  23. Derrick StoleeDec 8, 2022
  24. Jacob KellerDec 12, 2022
  25. 0/4 Optionally skip hashing index on writeDerrick Stolee via GitGitGadget, Dec 12, 2022
  26. 1/4 hashfile: allow skipping the hash functionDerrick Stolee via GitGitGadget, Dec 12, 2022
  27. 4/4 features: feature.manyFiles implies fast index writesDerrick Stolee via GitGitGadget, Dec 12, 2022
  28. 2/4 read-cache: add index.skipHash config optionDerrick Stolee via GitGitGadget, Dec 12, 2022
  29. 3/4 test-lib-functions: add helper for trailing hashDerrick Stolee via GitGitGadget, Dec 12, 2022
  30. SZEDER GáborDec 12, 2022
  31. Junio C HamanoDec 13, 2022
  32. SZEDER GáborDec 17, 2022
  33. 0/4 Optionally skip hashing index on writeDerrick Stolee via GitGitGadget, Dec 15, 2022
  34. 1/4 hashfile: allow skipping the hash functionDerrick Stolee via GitGitGadget, Dec 15, 2022
  35. 2/4 read-cache: add index.skipHash config optionDerrick Stolee via GitGitGadget, Dec 15, 2022
  36. Ævar Arnfjörð BjarmasonDec 15, 2022
  37. 3/4 test-lib-functions: add helper for trailing hashDerrick Stolee via GitGitGadget, Dec 15, 2022
  38. 4/4 features: feature.manyFiles implies fast index writesDerrick Stolee via GitGitGadget, Dec 15, 2022
  39. Ævar Arnfjörð BjarmasonDec 15, 2022
  40. Derrick StoleeDec 16, 2022
  41. 0/4 Optionally skip hashing index on writeDerrick Stolee via GitGitGadget, Dec 16, 2022
  42. 1/4 hashfile: allow skipping the hash functionDerrick Stolee via GitGitGadget, Dec 16, 2022
  43. 2/4 read-cache: add index.skipHash config optionDerrick Stolee via GitGitGadget, Dec 16, 2022
  44. 3/4 test-lib-functions: add helper for trailing hashDerrick Stolee via GitGitGadget, Dec 16, 2022
  45. 4/4 features: feature.manyFiles implies fast index writesDerrick Stolee via GitGitGadget, Dec 16, 2022
  46. Ævar Arnfjörð BjarmasonDec 16, 2022
  47. Derrick StoleeJan 6, 2023
  48. Junio C HamanoJan 6, 2023
  49. Derrick StoleeJan 6, 2023
  50. Ævar Arnfjörð BjarmasonJan 9, 2023
  51. Derrick StoleeJan 9, 2023
  52. Ævar Arnfjörð BjarmasonJan 9, 2023
  53. 0/4 Optionally skip hashing index on writeDerrick Stolee via GitGitGadget, Jan 6, 2023
  54. 1/4 hashfile: allow skipping the hash functionDerrick Stolee via GitGitGadget, Jan 6, 2023
  55. 2/4 read-cache: add index.skipHash config optionDerrick Stolee via GitGitGadget, Jan 6, 2023
  56. 3/4 test-lib-functions: add helper for trailing hashDerrick Stolee via GitGitGadget, Jan 6, 2023
  57. 4/4 features: feature.manyFiles implies fast index writesDerrick Stolee via GitGitGadget, Jan 6, 2023
  58. Junio C HamanoJan 15, 2023
  59. Derrick StoleeJan 17, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.