From: Junio C Hamano <gitster@pobox.com>
To: Derrick Stolee <stolee@gmail.com>
Cc: Chinmoy via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org, Chinmoy <chinmoy12c@gmail.com>,
Derrick Stolee <dstolee@microsoft.com>
Subject: Re: [PATCH v3] cache-tree.c: remove implicit dependency on the_repository
Date: Mon, 05 Apr 2021 10:48:04 -0700 [thread overview]
Message-ID: <xmqqv99062sr.fsf@gitster.g> (raw)
In-Reply-To: <be9793f0-f437-8e42-d463-21a48d2ee948@gmail.com> (Derrick Stolee's message of "Mon, 5 Apr 2021 09:08:44 -0400")
Derrick Stolee <stolee@gmail.com> writes:
> With your additional comments, I think it is clear that the "fourth
> option" I mentioned earlier [1] is the way to go:
>
> Finally, there is yet a fourth option: use istate->repo instead. In
> 1fd9ae51 (repository: add repo reference to index_state), I added a
> 'repo' member to struct index_state. This is intended for methods to
> access a repository directly from the index.
>
> [1] https://lore.kernel.org/git/f187df01-8e59-ac74-01e1-586a7a63fd4e@gmail.com/
Thanks. I wasn't following the earlier discussion closely, as the
topic seemed to be in the hands of good reviewers ;-)
> So in this sense, we should always use istate->repo, but we might
> still need the following guard in some places:
>
> if (!istate->repo)
> istate->repo = the_repository;
>
> in case there are situations where the index is loaded before
> the_repository is loaded. I have hit this in testing, but don't fully
> understand the cases where this can happen.
As a longer term goal, it may not be a bad idea to make sure that
anybody who passes an istate should not have to pass a repo. I do
not think of a reason why, other than historical inertia, to do so
in post 1fd9ae51 world.
> The way it would change this patch is to drop the 'struct repository *r'
> pointers and changes to the method signatures. Instead, keep the
> methods only taking a 'struct index_state *istate' and use istate->repo
> everywhere.
Yes, and that would result in a patch that touches very limited
small parts of cache-tree.c without needing to touch any caller, I
would presume.
Thanks.
next prev parent reply other threads:[~2021-04-05 17:48 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-25 19:48 [PATCH] cache-tree.c: remove implicit dependency on the_repository Chinmoy via GitGitGadget
2021-03-25 20:31 ` Derrick Stolee
2021-03-26 6:49 ` Chinmoy Chakraborty
2021-03-26 7:54 ` Chinmoy Chakraborty
2021-03-26 15:35 ` [PATCH v2] " Chinmoy via GitGitGadget
2021-04-03 15:57 ` [PATCH v3] " Chinmoy via GitGitGadget
2021-04-04 1:49 ` Junio C Hamano
2021-04-04 5:11 ` Chinmoy Chakraborty
2021-04-04 5:36 ` Junio C Hamano
2021-04-04 5:18 ` Chinmoy Chakraborty
2021-04-04 6:09 ` Junio C Hamano
2021-04-05 13:08 ` Derrick Stolee
2021-04-05 17:48 ` Junio C Hamano [this message]
2021-04-07 6:54 ` [PATCH v4] " Chinmoy via GitGitGadget
2021-04-07 23:03 ` Junio C Hamano
2021-04-08 3:56 ` Chinmoy Chakraborty
2021-04-08 13:23 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqv99062sr.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=chinmoy12c@gmail.com \
--cc=dstolee@microsoft.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=stolee@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).