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

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