git/list[1] front-page[2] threads[3] people[4] search[5] about
wed 2026-10-07 18:02 UTC

Re: [PATCH v1] repository: move fetch_if_missing into struct repository

From
Tian Yuchen <cat@malon.dev>
Date
Jul 16, 2026, 07:06 UTC
Message-ID
<ceb95555-d2fa-4877-9e46-eccac5db61d7@malon.dev>
In-Reply-To
<alcqQp0lkwRIIE1t@pks.im>
On 7/15/26 14:35, Patrick Steinhardt wrote:
Show 57 quoted lines
> On Wed, Jul 15, 2026 at 09:18:50AM +0800, Tian Yuchen wrote:
>> The global variable 'fetch_if_missing' controls whether a missing
>> object check should prompt a lazy fetch from a promisor remote.
>> In order to continue the libification effort, move it into
>> 'struct repository' and initialize it to 1 by default to keep the
>> previous behavior.
> 
> Right. I was also thinking about moving this into a non-global scope
> multiple times. I was approaching this a bit differently though: it's
> ultimately a property of the object database whether or not we want to
> accept missing objects, so I moved it in there instead.
> 
> I don't really think there's a downside with your version, though. Quite
> on the contrary: we can really only perform the backfill fetches with a
> whole repository at hand anyway. So conceptually your version might even
> be more sensible.
> 
>> Subsystems that already pass around a repository pointer, are
>> updated to read this flag directly from their respective 'repo'
>> instances. For the rest, we access 'the_repository'.
>>
>> Note that in builtin/fsck.c and builtin/index-pack.c, when running
>> related commands with the '-h' parameter, the 'repo' pointer is not
>> passed in. To prevent null pointer dereferences, we defer
>> operations on the repo in until after parameter parsing is complete.
> 
> s/on the repo in/on the repo/
> 
>> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
>> index 0793dc595c..721d576938 100644
>> --- a/builtin/index-pack.c
>> +++ b/builtin/index-pack.c
>> @@ -1898,15 +1898,16 @@ int cmd_index_pack(int argc,
>>   	int report_end_of_input = 0;
>>   	int hash_algo = 0;
>>   
>> +	show_usage_if_asked(argc, argv, index_pack_usage);
>> +
>>   	/*
>>   	 * index-pack never needs to fetch missing objects except when
>>   	 * REF_DELTA bases are missing (which are explicitly handled). It only
>>   	 * accesses the repo to do hash collision checks and to check which
>>   	 * REF_DELTA bases need to be fetched.
>>   	 */
>> -	fetch_if_missing = 0;
>> -
>> -	show_usage_if_asked(argc, argv, index_pack_usage);
>> +	if (repo)
>> +		repo->fetch_if_missing = 0;
>>   
>>   	disable_replace_refs();
>>   
> 
> Okay. This command can run without a repository, in which case we'll end
> up just indexing the pack. My assumption is that we'll probably end up
> using `the_repository` if so, as we still use `the_repository` in this
> file. So could this here cause a change in behaviour?
Hummm...
Show 23 quoted lines
> 
> If the answer is "maybe" I'd propose that we simply continue to use
> `the_repository` here.
> 
>> diff --git a/revision.c b/revision.c
>> index e91d7e1f11..bb645654c3 100644
>> --- a/revision.c
>> +++ b/revision.c
>> @@ -2714,7 +2714,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
>>   		revs->ignore_missing = 1;
>>   	} else if (opt && opt->allow_exclude_promisor_objects &&
>>   		   !strcmp(arg, "--exclude-promisor-objects")) {
>> -		if (fetch_if_missing)
>> +		if (revs->repo->fetch_if_missing)
>>   			BUG("exclude_promisor_objects can only be used when fetch_if_missing is 0");
>>   		revs->exclude_promisor_objects = 1;
>>   	} else {
> 
> This one here also makes me wonder whether it could cause weird
> interactions in case a caller passes a repository other than
> `the_repository`. It ideally _shouldn't_, but it's hard to tell because
> we still use `the_repository` in lots of places here.
> 
This makes sense to me. Let's use the_repository then.
> Thanks!
> 
> Patrick
Regards, yuchen
Previous: Patrick SteinhardtNext: Tian Yuchen
Message 5 of 26 in “repository: move fetch_if_missing into struct repository”
  1. repository: move fetch_if_missing into struct repositoryTian Yuchen, Jul 15, 2026
  2. Junio C HamanoJul 15, 2026
  3. Tian YuchenJul 15, 2026
  4. Patrick SteinhardtJul 15, 2026
  5. Tian YuchenJul 16, 2026
  6. repository: move fetch_if_missing into struct repositoryTian Yuchen, Jul 16, 2026
  7. Junio C HamanoJul 16, 2026
  8. Tian YuchenAug 1, 2026
  9. Patrick SteinhardtAug 4, 2026
  10. Junio C HamanoAug 4, 2026
  11. Tian YuchenAug 5, 2026
  12. Tian YuchenAug 5, 2026
  13. repository: move fetch_if_missing into struct repositoryTian Yuchen, Aug 7, 2026
  14. Junio C HamanoAug 7, 2026
  15. Tian YuchenAug 9, 2026
  16. repository: move fetch_if_missing into struct repositoryTian Yuchen, Aug 13, 2026
  17. Junio C HamanoAug 13, 2026
  18. 0/2 repository: move fetch_if_missing into struct repositoryTian Yuchen, Aug 14, 2026
  19. 1/2 pack-objects: give fetch_if_missing call sites access to 'repo'Tian Yuchen, Aug 14, 2026
  20. 2/2 repository: move fetch_if_missing into struct repositoryTian Yuchen, Aug 14, 2026
  21. Junio C HamanoAug 14, 2026
  22. Junio C HamanoAug 14, 2026
  23. Tian YuchenAug 15, 2026
  24. repository: move fetch_if_missing into struct repositoryTian Yuchen, Aug 15, 2026
  25. Junio C HamanoAug 15, 2026
  26. Tian YuchenAug 16, 2026

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.