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@xxxxxxxxxx/ [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
