On Wed, Dec 7, 2016 at 3:34 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
>> Stefan Beller <sbeller@xxxxxxxxxx> 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.
Right, it just happened historically for that function to live
in submodule area. I'll move the connect_work_tree_and_git_dir
to dir.c as well.