Re: [PATCH v4 3/4] submodule: support running in multiple worktree setup

[Stefan Beller <sbeller@google.com>]

On Wed, Jul 20, 2016 at 10:24 AM, Nguyễn Thái Ngọc Duy
<pclouds@gmail.com> wrote:
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  Documentation/git-worktree.txt | 8 ++++++++
>  git-submodule.sh               | 8 ++++----
>  2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> index 41350db..2a5661d 100644
> --- a/Documentation/git-worktree.txt
> +++ b/Documentation/git-worktree.txt
> @@ -142,6 +142,14 @@ to share to all working directories:
>     you are sure you always use sparse checkout for all working
>     directories.
>
> + - `submodule.*` in current state should not be shared because the
> +   information is tied to a particular version of .gitmodules in a
> +   working directory.

While the submodule.* settings are copied from the .gitmodules file initially,
they can be changed in the config later. (That was actually the whole
point of it,
so you can change the submodule remotes URL without having to change history.)

And I would think that most submodule related settings (such as remote URL,
name, path, even depth recommendation) should be the same for all worktrees,
and a different value for one worktree is a carefully crafted
exception by the user.

So while the .gitmodules file can diverge in the work trees I do not
think that the
actual remotes for the submodules in the different worktrees differ
though. The change
of the .gitmodule files may be because you checked out an old commit, that
has outdated information on where to get the submodule from.

> +
> + - `remote.*` added by submodules may be per working directory as
> +   well, unless you are sure remotes from all possible submodules in
> +   history are consistent.
> +
>  DETAILS
>  -------
>  Each linked working tree has a private sub-directory in the repository's
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 4ec7546..7b576f5 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -261,7 +261,7 @@ or you are unsure what this means choose another name with the '--name' option."
>                         esac
>                 ) || die "$(eval_gettext "Unable to checkout submodule '\$sm_path'")"
>         fi
> -       git config submodule."$sm_name".url "$realrepo"
> +       git config --worktree submodule."$sm_name".url "$realrepo"

This is in cmd_add. Actually I think this should be --not-worktree
(i.e. --local)
as when you add a submodule in one worktree, and then in another,
you may want to have the same URL. However if another worktree
already configured it you want to keep the option.
so rather:

  if git config  submodule."$sm_name".url then
      # it exists, do nothing
  else
    # it does not exist
    git config --local ...

>
>         git add $force "$sm_path" ||
>         die "$(eval_gettext "Failed to add submodule '\$sm_path'")"
> @@ -461,7 +461,7 @@ Submodule work tree '\$displaypath' contains a .git directory
>                         # Remove the whole section so we have a clean state when
>                         # the user later decides to init this submodule again
>                         url=$(git config submodule."$name".url)
> -                       git config --remove-section submodule."$name" 2>/dev/null &&
> +                       git config --worktree --remove-section submodule."$name" 2>/dev/null &&
>                         say "$(eval_gettext "Submodule '\$name' (\$url) unregistered for path '\$displaypath'")"

This is in cmd_deinit, which is documented as:
           Unregister the given submodules, i.e. remove the whole
           submodule.$name section from .git/config together with their work
           tree. Further calls to git submodule update, git submodule foreach
           and git submodule sync will skip any unregistered submodules until
           they are initialized again, so use this command if you don’t want
           to have a local checkout of the submodule in your work tree
           anymore. If you really want to remove a submodule from the
           repository and commit that use git-rm(1) instead.

So one might wonder if the unregister should be a global unregister
or a worktree specific unregister.

From a users POV there are:
* non existent submodules (no gitlink recorded, no config set,
  no repo in place)
* not initialized submodules (gitlink is recorded, no config set,
  and an empty repo is put in the working tree as a place holder).
* initialized submodules (gitlink is recorded, the config
  submodule .<name>.url is copied from the .gitmodules file to .git/config.
  an empty dir in the working tree as a place holder)
  A user may change the configuration before the next step as the url in
  the .gitmodules file may be wrong and the user doesn't want to
  rewrite history
* existing submodules (gitlink is recorded, the config option is set
  and instead of an empty placeholder dir, we actually have a git
  repo there.)
* matching submodules (the recorded git link matches
  the actual checked out state of the repo!, config option and repo exist)

I made up these terms for these 5 states and they don't appear in any
documentation, but I think that is the exhaustive list of what a submodule
should be, when using the git-submodule command properly.

There can be more things though. As we have three indicators (existence of
gitlink, config option and repo), we can have up to 8 states, I left some out in
the above listing.

* gitlink recorded, no config set, repo is there and maybe even matches the
  recorded git link.
   What a strange thing! git treats that as a "not initialized" from above.
* no gitlink exists, no config exists, but a repo exists:
  That's just an embedded repo, never touch it!
* no gitlink, no repo, but a config exists: just a stray old config
laying around,
  ignore it
* no gitlink, but a config and a repo exists: "A deleted submodule", use
  rm -rf to purge it.

--------
The above was a wall of text to make myself aware of the pitfalls of submodules.
---

A discussion with Jonathan Nieder in office ensued and we came to the following
conclusions:
* Anything except the "existence in the working tree" shall be shared,
i.e. is a repository
  specific, not working tree specific setting.

Currently we use the submodule."<name>".url config option to determine
the existence of
a submodule (init vs non init; if init -> git submodule update will
(maybe fetch) and checkout
accordingly).

As an intermediate step forward we could do:
* introduce a submodule."<name>".existsInWorktree = [true/false]
setting, that decouples
  the check for existence from the url being present. That is the only
option per worktree, all
  other submodule."<name>".* options are shared if not overwritten
manually with care

* another approach would be to have a
submodule.includeInWorktree=<pathspec> option
  which is similar, but has slightly different naming.

Looking back at origin/sb/submodule-default-path (which is in pu for a
long time now),
which allowed a configuration submodule.defaultUpdatePath <pathspec>,
that could be
used with `git submodule update --init-default-path`.

----
So maybe we want to drop that series and first talk about a migration plan from
the current state to a world where we have the existence depending not
on the url
parameter, but a boolean variable submodule.<name>.<good_name>.
Depending on <good_name> a submodule would be ignored or tried to checkout
in e.g. `submodule update`

---
If we want to move the current behavior of submodules forward, we
would want to have
anything but the url as shared variables and then use the url variable
as a per-worktree
existence flag.

Thanks,
Stefan