Re: [PATCH 7/8] fetch: fetch unpopulated, changed submodules

From: Glen Choo <chooglen@google.com>
Date: Feb 11, 2022, 07:15 UTC
Message-ID: <kl6lbkzdyitt.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <xmqqtud6e3r8.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 19 quoted lines

>> +static struct fetch_task *
>> +get_fetch_task_from_changed(struct submodule_parallel_fetch *spf,
>> +			    const char **default_argv, struct strbuf *err)
>> +{
>
>> @@ -1553,7 +1628,10 @@ static int get_next_submodule(struct child_process *cp, struct strbuf *err,
>>  {
>>  	struct submodule_parallel_fetch *spf = data;
>>  	const char *default_argv = NULL;
>> -	struct fetch_task *task = get_fetch_task(spf, &default_argv, err);
>> +	struct fetch_task *task =
>> +		get_fetch_task_from_index(spf, &default_argv, err);
>> +	if (!task)
>> +		task = get_fetch_task_from_changed(spf, &default_argv, err);
>
> Hmph, intersting.  So if "from index" grabbed some submodules
> already, then the "from the changes in the superproject, we know
> these submodules need refreshing" is not happen at all?  I am afraid
> that I am still not following this...

Hm, perhaps the following will help:

- get_next_submodule() is an iterator, specifically, it is a
  get_next_task_fn passed to run_processes_parallel_tr2(). It gets
  called until it is exhausted.
- Since get_next_submodule() is an iterator, I've implemented
  get_fetch_task_from_index() and get_fetch_task_from_changed() as
  iterators (they return NULL when they are exhausted).

So in practice:

- We repeatedly call get_next_submodule(), which tries to get a fetch
  task by calling the get_fetch_task_* functions.
- If get_fetch_task_from_index() returns non-NULL, get_next_submodule()
  uses that fetch task.
- Eventually, we will have considered every submodule in the index. At
  that point, get_fetch_task_from_index() is exhausted and always
  returns NULL.
- Since get_fetch_task_from_index() returns NULL, get_next_submodule()
  now gets its fetch tasks from get_fetch_task_from_changed().
- Eventually, we will also have considered every changed submodule, and
  get_fetch_task_from_changed() is exhausted.
- get_next_submodule() has now been exhausted and we are done.

As for the other questions, I'll dig a bit deeper before getting back to you with answers.