Re: [PATCH] fetch: Get submodule paths from index and not from .gitmodules
- From
Johannes Sixt <j.sixt@viscovery.net>
- Date
- Sep 17, 2010, 12:32 UTC
- Message-ID
- <4C935FF2.7080502@viscovery.net>
- In-Reply-To
- <4C935D77.3080008@web.de>
Am 9/17/2010 14:22, schrieb Jens Lehmann:
Show 51 quoted lines
> Am 17.09.2010 14:06, schrieb Johannes Sixt:
>> 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;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.
-- Hannes