Re: [PATCH v5 3/3] push: teach --recurse-submodules the on-demand option

From: Jens Lehmann <jens.lehmann@web.de>
Date: Feb 15, 2012, 22:28 UTC
Message-ID: <4F3C3172.8030505@web.de>
In-Reply-To: <7v7gzq9jg2.fsf@alter.siamese.dyndns.org>

Am 14.02.2012 04:34, schrieb Junio C Hamano:

Show 37 quoted lines

> Heiko Voigt <hvoigt@hvoigt.net> writes:
> 
>> diff --git a/submodule.c b/submodule.c
>> index 3c714c2..ff0cfd8 100644
>> --- a/submodule.c
>> +++ b/submodule.c
>> @@ -411,6 +411,54 @@ int check_submodule_needs_pushing(unsigned char new_sha1[20],
>>  	return needs_pushing->nr;
>>  }
>>  
>> +static int push_submodule(const char *path)
>> +{
>> +	if (add_submodule_odb(path))
>> +		return 1;
>> +
>> +	if (for_each_remote_ref_submodule(path, has_remote, NULL) > 0) {
>> +		struct child_process cp;
>> +		const char *argv[] = {"push", NULL};
>> +
>> +		memset(&cp, 0, sizeof(cp));
>> +		cp.argv = argv;
>> +		cp.env = local_repo_env;
>> +		cp.git_cmd = 1;
>> +		cp.no_stdin = 1;
>> +		cp.dir = path;
>> +		if (run_command(&cp))
>> +			return 0;
>> +		close(cp.out);
>> +	}
>> +
>> +	return 1;
>> +}
> 
> Hmm, this makes me wonder if we fire subprocesses and have them run in
> parallel (to a reasonably limited parallelism), it might make the overall
> user experience more pleasant, and if we did the same on the fetching
> side, it would be even nicer.

Yeah, I had the same idea and did some experiments when working on fetch some time ago.

Show 6 quoted lines

> We would need to keep track of children and after firing a handful of them
> we would need to start waiting for some to finish and collect their exit
> status before firing more, and at the end we would need to wait for the
> remaining ones and find how each one of them did before returning from
> push_unpushed_submodules().  If we were to do so, what are the missing
> support we would need from the run_command() subsystem?

We would not only have to collect the exit status but also the output lines. You don't want to see the output of multiple fetches or pushes mixed together, so it makes sense to just defer that until the command exited and then print everything at once. The interesting part I couldn't come up with an easy solution for is to preserve the output order between the stdout and stdin lines, as they contain different parts of the progress which would look strange when shuffled around.

And I saw that sometimes parallel fetches took way longer than doing them sequentially (in my case because of strange DNS behavior of my DSL router), so we would definitely want a config option for that (maybe setting the maximum number of simultaneous threads to be used).

But don't get me wrong, I'm all for having that feature! :-)