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.