Re: [PATCHv5 0/5] submodule embedgitdirs

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 7, 2016, 22:35 UTC
Message-ID: <xmqqzik7v04b.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20161207210157.18932-1-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

Show 8 quoted lines

> 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.