Re: [PATCH 1/2] ls-files: optionally recurse into submodules

[Brandon Williams <bmwill@google.com>]

On Wed, Sep 21, 2016 at 11:20 PM, Jeff King <peff@peff.net> wrote:
>> +/**
>> + * Recursively call ls-files on a submodule
>> + */
>> +static void show_gitlink(const struct cache_entry *ce)
>> +{
>> +     struct child_process cp = CHILD_PROCESS_INIT;
>> +     int status;
>> +
>> +     argv_array_push(&cp.args, "ls-files");
>> +     argv_array_push(&cp.args, "--recurse-submodules");
>> +     argv_array_pushf(&cp.args, "--submodule-prefix=%s%s/",
>> +                      submodule_prefix ? submodule_prefix : "",
>> +                      ce->name);
>> +     cp.git_cmd = 1;
>> +     cp.dir = ce->name;
>> +     status = run_command(&cp);
>> +     if (status)
>> +             exit(status);
>> +}
>
> This doesn't propagate the parent argv at all. So if I run:
>
>   git ls-files -z --recurse-submodules
>
> then the paths are all NUL-terminated in the parent, but
> newline-terminated in the submodules. Oops.

Yep definitely missed that.  I can fix that.  I think the main reason
for not blindly
copying the argv array is that there may be some things we don't want to pass
to the child. While not in the context of ls-files, I was working on
recursive grep
earlier and with that you can pass a rev to grep.  You can't blindly
copy that because
 the rev is meaningless to the the child and may produce broken
output.  Instead we
would need to pass the actual rev of what the parent has checked out
in that particular
rev.  I haven't thought it completely through yet but it did
discourage me from blindly
copying the args across.

-Brandon