Re: [PATCHv5 0/5] submodule embedgitdirs

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Stefan Beller <sbeller@google.com> writes:
>>
>>> v5:
>>> * Add another layer of abstraction, i.e. the relocate_git_dir is only about
>>>   moving a git dir of one repository. The submodule specific stuff (e.g.
>>>   recursion into nested submodules) is in submodule.{c,h}
>>>
>>>   This was motivated by reviews on the series of checkout aware of submodules
>>>   building on top of this series, as we want to directly call the embed-git-dirs
>>>   function without the overhead of spawning a child process.
>>
>> OK.  Comparing the last steps between this round and the previous
>> one, I do think the separation of the responsibility among helpers
>> is much more reasonable in this version, where:
>>
>>  - submodule_embed_git_dir() is given a single path and is
>>    responsible for that submodule itself, which is done by calling
>>    submodule_embed_git_dir_for_path() on itself, and its
>>    sub-submodules, which is done by spawning the helper recursively
>>    with appropriate super-prefix;
>>
>>  - submodule_embed_git_dir_for_path() computes where the given path
>>    needs to be moved to using the knowledge specific to the
>>    submodule subsystem, and asks relocate_gitdir() to perform the
>>    actual relocation;
>>
>>  - relocate_gitdir() used to do quite a lot more, but now it is only
>>    about moving an existing .git directory elsewhere and pointing to
>>    the new location with .git file placed in the old location.
>>
>> I would have called the second helper submodule_embed_one_git_dir(),
>> but that is a minor detail.
>>
>> Very nicely done.
>
> One thing that is not so nice from the code organization point of
> view is that "dir.c" now needs to include "submodule.h" because it
> wants to call connect_work_tree_and_git_dir().
>
> I wonder if that function belongs to dir.c or worktree.c not
> submodule.c; I do not see anything that is submodule specific about
> what the function does.