Re: [PATCH 1/2] remote: move static variables into struct

From: Junio C Hamano <gitster@pobox.com>
To: "Glen Choo via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Glen Choo <chooglen@google.com>
Subject: Re: [PATCH 1/2] remote: move static variables into struct
Date: Thu, 07 Oct 2021 16:36:05 -0700	[thread overview]
Message-ID: <xmqq8rz4o1ay.fsf@gitster.g> (raw)
In-Reply-To: <f7639a20fcd437d5f50665b5c6f5318c1245b9b5.1633633635.git.gitgitgadget@gmail.com> (Glen Choo via GitGitGadget's message of "Thu, 07 Oct 2021 19:07:14 +0000")

"Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Glen Choo <chooglen@google.com>
>
> remote.c only works with the_repository because it stores its state as
> static variables. To support non-the_repository, we can use a
> per-repository struct for the remotes subsystem.

OK, so eventually we can hook the struct this patch introduces as a
member in the repository structure, so that we can have one such
struct per in-core repository instances?

Will some of the component of this new structure have to be aware of
the containing structure, even the repository instance the
containing structure belongs to?

For example, the apply_push_cas() function (there is nothing special
about it---just a random example near the end of file) takes "struct
remote *" which will be passed to underlying apply_cas() and the
control flow eventually need to reach read_ref() via
remote_tracking().  In a future world where we keep multiple in-core
repositories and interact with multiple repositories in-process,
this read_ref() must know in which repository it is asked to read
the value of the ref.  Do we expect to plumb "struct repository *"
throughout the callchain, or will some of the struct types gain a
backpointer to "struct remote_state", and "struct remote_state" will
in turn have a backpointer to the containing repository, so that
remote_tracking() can learn the ref_store it needs to use when
making a call to refs_read_ref_full() to replace the current call it
makes to the read_ref() function?

As to the name, I do not think "remote state" is not too bad, but
the word "remote" there certainly does not mean "state of a remote
repository", and in that sense, it might be confusing.

But It is still a seemingly random "collection of data that we need
to interact with remote repositories", so in that sense, the word
"remote" fits its purpose.  There are pieces of information needed
for a branch to interact with a remote repository, and the branches
variable serves as a caching mechanism for that, for example.