Re: [PATCH] fetch: Get submodule paths from index and not from .gitmodules

From: Jens Lehmann <jens.lehmann@web.de>
Date: Sep 17, 2010, 12:22 UTC
Message-ID: <4C935D77.3080008@web.de>
In-Reply-To: <4C9359D4.2030109@viscovery.net>

Am 17.09.2010 14:06, schrieb Johannes Sixt:

Show 21 quoted lines

> Am 9/17/2010 13:31, schrieb Jens Lehmann:
>> But I think I found the real issue, the stdout of the forked "git fetch"
>> was swallowed due to a copy & paste bug while the actual fetch commands
>> were executed nonetheless. Please try the following change:
>>
>>
>> diff --git a/submodule.c b/submodule.c
>> index e2c3bae..4fb1071 100644
>> --- a/submodule.c
>> +++ b/submodule.c
>> @@ -260,7 +260,8 @@ int fetch_populated_submodules(int forced)
>>         cp.env = local_repo_env;
>>         cp.git_cmd = 1;
>>         cp.no_stdin = 1;
>> -       cp.out = -1;
>> +       cp.out = 1;
>> +       cp.err = 1;
> 
> This cannot be correct. Subsequent code reads the stdout of the child
> process, i.e., you want a pipe; hence, cp.out = -1 is correct (this
> requests a pipe; later code correctly closes cp.out).

Thanks for catching this! I copied this code from a spot where stdout is read via a pipe (and is then closed afterwards), but that isn't the case here.

> As far as stderr of the child is concerned, if you only want to re-use the
> standard error of the parent, then not assigning anything to cp.err is
> correct (it was set to 0 in the memset before this hunk). But perhaps you
> want to achieve something else?

Nope. You are right, setting both to 0 (via the memset) to inherit the channel from the parent is just what is needed here.

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;