[PATCH v2 0/3] remote: replace static variables with struct remote_state

[Glen Choo <chooglen@google.com>]

This series aims to make the remotes subsystem work with
non-the_repository, which will allow submodule remotes to be accessed
in-process, rather than through child processes. This is accomplished by
creating a struct remote_state and adding it to struct repository.

One motivation for this is that it allows future submodule commands to run
in-process. An example is an RFC series of mine [1], where I tried to implement
"git branch --recurse-submodules" in-process but couldn't figure out how to
read the remotes of a submodule.

v2 aims to catch all of the instances of struct remote_state that I had
missed in v1, particularly with the struct branch functions. Because
more repo_* functions were added, the preparatory patches have been
reorganized.

In this version, my biggest uncertainty is in the interfaces of the
repo_* functions. Some functions expect a "struct repository" and one
of its contained members (e.g. "struct branch"). This interface allows
for incorrect behavior if the caller supplies a "struct repository"
instance that is unrelated from the "struct branch". This concern was
raised by Junio in [2].

While this concern is valid, I decided to keep this interface for a few
reasons:

1. The correct way of calling the function is 'obvious'.
2. It is relatively easy to get the contained object (struct branch/remote)
   and its containing struct repository/remote_state (e.g. we don't pass
   struct branch or remote through long call chains). For "struct
   branch", callers usually get the branch from the repo and use it
   immediately. For "struct remote", we don't use container objects
   outside of static functions. If you are interested in seeing all of
   the call sites, you can see a sample commit in [3].
3. The separation between container/contained objects allows us to
   reason better about what the correct interface is. e.g. we might be
   tempted to include a backpointer from struct branch to struct
   remote_state so that we can pass around struct branch and be
   confident that struct branch has all of the information it needs.

   However, I believe that some questions *shouldn't* be answered by
   just struct branch. The prime example in this series is
   branch_get_push() - it conceptually answers 'What is the pushremote
   of this branch', but the behavior we want is closer to 'If
   configured, give me the pushremote for the branch. Otherwise, give me
   the default pushremote of the repo.'. This is arguably a relevant
   detail that should be exposed to callers.

If we want the interface to prevent misuse, we can check that the
correct repository is passed at runtime. Alternatively, if we are
convinced that some questions can only be answered in the context of a
repository, we can take things one step further by using (struct
repository, branch_name) instead of (struct repository, struct branch).

A different concern is that struct repository is added to some callback
signatures even though the function body doesn't use it e.g.
repo_remote_for_branch(). However, this might be more of an accident of
history than anything else - the difference between remote and
pushremote is that remote always defaults to 'origin', whereas
the default value of pushremote is configurable.

Changes since v1:

* In v1, we moved static variables into the_repository->remote_state in
  two steps: static variables > static remote_state >
  the_repository->remote_state. In v2, make this change in one step:
  static variables > the_repository->remote_state.
* Add more instances of repo_* that were missed.

[1] https://lore.kernel.org/git/20210921232529.81811-1-chooglen@google.com/
[2] https://lore.kernel.org/git/xmqq4k9so15i.fsf@gitster.g/
[3] https://github.com/chooglen/git/commit/173e0268fd076044dd6b3cae893eedfa33965942

Glen Choo (3):
  remote: move static variables into per-repository struct
  remote: use remote_state parameter internally
  remote: add struct repository parameter to external functions

 remote.c     | 305 +++++++++++++++++++++++++++++++--------------------
 remote.h     | 130 +++++++++++++++++++---
 repository.c |   8 ++
 repository.h |   4 +
 4 files changed, 312 insertions(+), 135 deletions(-)

Range-diff against v1:
1:  6972ba4dcb = 1:  6972ba4dcb remote: move static variables into per-repository struct
2:  71b1da4389 = 2:  71b1da4389 remote: use remote_state parameter internally
3:  ff12771f06 = 3:  ff12771f06 remote: add struct repository parameter to external functions
-- 
2.33.0.882.g93a45727a2-goog