Re: [PATCH v2 1/2] ls-files: fix recurse-submodules with nested submodules

[Jacob Keller <jacob.keller@gmail.com>]

On Thu, Apr 13, 2017 at 11:03 AM, Brandon Williams <bmwill@google.com> wrote:
> On 04/13, Jacob Keller wrote:
>> From: Jacob Keller <jacob.keller@gmail.com>
>>
>> Since commit e77aa336f116 ("ls-files: optionally recurse into
>> submodules", 2016-10-07) ls-files has known how to recurse into
>> submodules when displaying files.
>>
>> Unfortunately this fails for certain cases, including when nesting more
>> than one submodule, called from within a submodule that itself has
>> submodules, or when the GIT_DIR environemnt variable is set.
>>
>> Prior to commit b58a68c1c187 ("setup: allow for prefix to be passed to
>> git commands", 2017-03-17) this resulted in an error indicating that
>> --prefix and --super-prefix were incompatible.
>>
>> After this commit, instead, the process loops forever with a GIT_DIR set
>> to the parent and continuously reads the parent submodule files and
>> recursing forever.
>>
>> Fix this by preparing the environment properly for submodules when
>> setting up the child process. This is similar to how other commands such
>> as grep behave.
>>
>> This was not caught by the original tests because the scenario is
>> avoided if the submodules are created separately and not stored as the
>> standard method of putting the submodule git directory under
>> .git/modules/<name>. We can update the test to show the failure by the
>> addition of "git submodule absorbgitdirs" to the test case. However,
>> note that this new test would run forever without the necessary fix in
>> this patch.
>>
>> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
>
> This looks good to me.  Thanks again for catching this.  Dealing with
> submodules definitely isn't easy (I seem to have made a lot of mistakes
> that have been cropping up recently)...it would be easier if we didn't
> have to spin out a process for each submodule but that's not the world
> we live in today :)
>

Spinning out a process is one of the big downsides of working with
submodules in our code. Unfortunately, spinning out a process is also
one of the biggest ways we isolate submodules, and if we wanted to do
this "in-process" we would need to add an abstraction layer that lets
us handle submodules in-process in some clean way.