From: Jens Lehmann <jens.lehmann@web.de>
Date: Fri, 17 Sep 2010 14:01:25 GMT
Subject: Re: [PATCH] fetch: Get submodule paths from index and not from .gitmodules
Message-ID: <4C9374B5.9020907@web.de>
In-Reply-To: <4C935FF2.7080502@viscovery.net>

Am 17.09.2010 14:32, schrieb Johannes Sixt:
> Am 9/17/2010 14:22, schrieb Jens Lehmann:
>> So the correct fix should look like this:
>>
>>
>> diff --git a/submodule.c b/submodule.c
>> index e2c3bae..209efa4 100644
>> --- a/submodule.c
>> +++ b/submodule.c
>> @@ -260,7 +260,6 @@ int fetch_populated_submodules(int forced)
>>         cp.env = local_repo_env;
>>         cp.git_cmd = 1;
>>         cp.no_stdin = 1;
>> -       cp.out = -1;
>>
>>         for (i = 0; i < active_nr; i++) {
>>                 struct strbuf submodule_path = STRBUF_INIT;
> 
> That cannot be correct, either. Look further down. There you have:
> 
> 	len = strbuf_read(&buf, cp.out, 1024);
> 
> and later
> 
> 	close(cp.out);
> 
> You can do neither when you do not request a pipe for the child's stdout.
> You must set cp.out = -1 if you want to keep these two lines.

Aah, you must be looking at submodule.c from current master. Then you'll
find is_submodule_modified() at that location, where of course the -1 is
needed for cp.out because later read() and close() are used on it.

But I was trying to fix a problem introduced by 496b35e7 in current next,
where the new function fetch_populated_submodules() is located there. And
in that function the output is not read and so cp.out = 0 is the right
thing to do here.

But thanks for double checking!