Re: [PATCH v2 1/2] wt-status: avoid passing NULL worktree

[Junio C Hamano <gitster@pobox.com>]

Phillip Wood <phillip.wood123@gmail.com> writes:

> In general the "struct worktree" returned may not correspond to
> the "current" worktree defined by is_current_worktree() as that
> function uses "the_repository" rather than "wt->repo" when
> deciding which worktree is "current".  In practice the "struct
> repository" we pass corresponds to "the_repository" as we only
> ever operate on a single repository at the moment.

This may technically be a correct description, but feels very
unsatisfactory, as it fails to answer this very simple question:

    what does it mean when is_current_worktree() says "no" to the
    worktree instance returned by this function?  In other words,
    what are the sample sequences that can lead to such a worktree?

We start a Git process in a directory which is part of a set of
worktrees governed by a single repository.  That repository becomes
the_repository and the worktree instance that represents our
directory would satisfy is_current_worktree().  Then we visit
another directory that is one of a set of worktrees goverend by a
separate and different repository.  We now have a repository
instance that is different from our the_repository.  Perhaps our
in-core submodule code may do that, and that different repository is
the submodule in question.  Running this function will yield the
worktree instance, whose path is a subdirectory of our current
directory where the submodule is checked out?  It may be the current
worktree if we asked is_current_worktree() about that worktree in
the context of the submodule, but it is not in the context of our
superproject repository.  In fact, none of the worktrees governed by
the submodule repository can be "current", as they are not our
checkout, from the viewpoint of our superproject repository.

Is that what is going on here?  

A related question that is much more relevant is this:

    What is the significance of the worktree, relative to our
    process, returned by this function for a given repo?  What is so
    special about this worktree, among others that are also linked
    to the same repository?

    What does it mean for a worktree to "corresponds to"
    repo->{gitdir,worktree}?  Why does the currently running Git
    process want to grab such a worktree?

If the answer were "it is the current worktree", then we would have
a nice and very understandable name "get-current-worktree-for-repo"
for the function, but because I do not think of a good answer to the
question (and you already explained why it is not the "current"
worktree), I cannot improve on "get _A_ worktree from repository",
the name given by the patch, which leaves the "which one of the
worktrees are we talking about?  Why did we pick that particular one
instead of other ones" unanswered.

Or perhaps "the current worktree" is not a per-Git-process concept,
but is a per-process-per-repo concept?

In other words, the function is_current_worktree(wt) may not take a
repository and always compute things relative to the_repository, but
once we wean ourselves off of the_repository, we would/should have
repo_is_current_worktree(repo, wt), making is_current_worktree(wt) a
thin wrapper for repo_is_current_worktree(the_repository, wt)?

> diff --git a/worktree.h b/worktree.h
> index e4bcccdc0ae..06efe26b835 100644
> --- a/worktree.h
> +++ b/worktree.h
> @@ -38,6 +38,12 @@ struct worktree **get_worktrees(void);
>   */
>  struct worktree **get_worktrees_without_reading_head(void);
>  
> +/*
> + * Construct a struct worktree corresponding to repo->gitdir and
> + * repo->worktree.
> + */
> +struct worktree *get_worktree_from_repository(struct repository *repo);
> +