From: Jeff King <peff@peff.net>
To: Jonathan Tan <jonathantanmy@google.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 1/9] refs: make _advance() check struct repo, not flag
Date: Fri, 24 Sep 2021 14:13:36 -0400 [thread overview]
Message-ID: <YU4VUF17tyG2AZi3@coredump.intra.peff.net> (raw)
In-Reply-To: <493fff7f4716d889da751b5f8c6740cc1e3aa360.1632242495.git.jonathantanmy@google.com>
On Tue, Sep 21, 2021 at 09:51:03AM -0700, Jonathan Tan wrote:
> Currently, ref iterators access the object store each time they advance
> if and only if the boolean flag DO_FOR_EACH_INCLUDE_BROKEN is unset.
> (The iterators access the object store because, if
> DO_FOR_EACH_INCLUDE_BROKEN is unset, they need to attempt to resolve
> each ref to determine that it is not broken.)
>
> Also, the object store accessed is always that of the_repository, making
> it impossible to iterate over a submodule's refs without
> DO_FOR_EACH_INCLUDE_BROKEN (unless add_submodule_odb() is used).
>
> As a first step in resolving both these problems, replace the
> DO_FOR_EACH_INCLUDE_BROKEN flag with a struct repository pointer. This
> commit is a mechanical conversion - whenever DO_FOR_EACH_INCLUDE_BROKEN
> is set, a NULL repository (representing access to no object store) is
> used instead, and whenever DO_FOR_EACH_INCLUDE_BROKEN is unset, a
> non-NULL repository (representing access to that repository's object
> store) is used instead. Right now, the locations in which
> non-the_repository support needs to be added are marked with BUG()
> statements - in a future patch, these will be replaced. (NEEDSWORK: in
> this RFC patch set, this has not been done)
I think your goal here of passing around a repository object is good.
But rolling the meaning of DO_FOR_EACH_INCLUDE_BROKEN into an implicit
"do we have a non-NULL repository" makes things awkward, I think.
As you noticed, we can't get rid of the flags parameter entirely. We
still have DO_FOR_EACH_PER_WORKTREE_ONLY. But I also have a series which
adds another flag which pairs with INCLUDE_BROKEN. Having half of the
logic implicit in the repository pointer and half in a flag would be
weird.
I'll post that series in a moment, but what I'm wondering here is: would
it be that big a deal to just pass the repository object around, and it
is simply not used if INCLUDE_BROKEN is passed?
-Peff
next prev parent reply other threads:[~2021-09-24 18:13 UTC|newest]
Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-21 16:51 [PATCH 0/9] No more adding submodule ODB as alternate Jonathan Tan
2021-09-21 16:51 ` [PATCH 1/9] refs: make _advance() check struct repo, not flag Jonathan Tan
2021-09-23 1:00 ` Junio C Hamano
2021-09-24 17:56 ` Jonathan Tan
2021-09-24 19:55 ` Junio C Hamano
2021-09-24 18:13 ` Jeff King [this message]
2021-09-24 18:28 ` Jonathan Tan
2021-09-21 16:51 ` [PATCH 2/9] refs: add repo paramater to _iterator_peel() Jonathan Tan
2021-09-21 16:51 ` [PATCH 3/9] refs iterator: support non-the_repository advance Jonathan Tan
2021-09-21 16:51 ` [PATCH 4/9] refs: teach refs_for_each_ref() arbitrary repos Jonathan Tan
2021-09-21 16:51 ` [PATCH 5/9] merge-{ort,recursive}: remove add_submodule_odb() Jonathan Tan
2021-09-28 0:29 ` Elijah Newren
2021-09-21 16:51 ` [PATCH 6/9] object-file: only register submodule ODB if needed Jonathan Tan
2021-09-21 16:51 ` [PATCH 7/9] submodule: pass repo to check_has_commit() Jonathan Tan
2021-09-21 16:51 ` [PATCH 8/9] refs: change refs_for_each_ref_in() to take repo Jonathan Tan
2021-09-21 16:51 ` [PATCH 9/9] submodule: trace adding submodule ODB as alternate Jonathan Tan
2021-09-23 18:05 ` [PATCH 0/9] No more " Junio C Hamano
2021-09-28 20:10 ` [PATCH v2 " Jonathan Tan
2021-09-28 20:10 ` [PATCH v2 1/9] refs: plumb repo param in begin-iterator functions Jonathan Tan
2021-09-28 22:24 ` Junio C Hamano
2021-09-28 20:10 ` [PATCH v2 2/9] refs: teach arbitrary repo support to iterators Jonathan Tan
2021-09-28 22:35 ` Junio C Hamano
2021-09-29 17:04 ` Jonathan Tan
2021-09-28 20:10 ` [PATCH v2 3/9] refs: peeling non-the_repository iterators is BUG Jonathan Tan
2021-09-28 20:10 ` [PATCH v2 4/9] refs: teach refs_for_each_ref() arbitrary repos Jonathan Tan
2021-09-28 22:49 ` Junio C Hamano
2021-09-28 20:10 ` [PATCH v2 5/9] merge-{ort,recursive}: remove add_submodule_odb() Jonathan Tan
2021-09-28 20:10 ` [PATCH v2 6/9] object-file: only register submodule ODB if needed Jonathan Tan
2021-09-28 20:10 ` [PATCH v2 7/9] submodule: pass repo to check_has_commit() Jonathan Tan
2021-09-28 20:10 ` [PATCH v2 8/9] refs: change refs_for_each_ref_in() to take repo Jonathan Tan
2021-09-28 20:10 ` [PATCH v2 9/9] submodule: trace adding submodule ODB as alternate Jonathan Tan
2021-09-29 23:06 ` [PATCH v3 0/7] No more " Jonathan Tan
2021-09-29 23:06 ` [PATCH v3 1/7] refs: plumb repo into ref stores Jonathan Tan
2021-09-30 11:13 ` [PATCH] fixup! " Carlo Marcelo Arenas Belón
2021-10-06 17:42 ` Glen Choo
2021-10-08 20:05 ` Jonathan Tan
2021-10-08 20:07 ` Jonathan Tan
2021-10-07 18:33 ` [PATCH v3 1/7] " Josh Steadmon
2021-10-08 20:08 ` Jonathan Tan
2021-09-29 23:06 ` [PATCH v3 2/7] refs: teach arbitrary repo support to iterators Jonathan Tan
2021-10-07 19:31 ` Glen Choo
2021-10-08 20:12 ` Jonathan Tan
2021-09-29 23:06 ` [PATCH v3 3/7] refs: peeling non-the_repository iterators is BUG Jonathan Tan
2021-09-29 23:06 ` [PATCH v3 4/7] merge-{ort,recursive}: remove add_submodule_odb() Jonathan Tan
2021-10-07 18:34 ` Josh Steadmon
2021-10-08 20:19 ` Jonathan Tan
2021-09-29 23:06 ` [PATCH v3 5/7] object-file: only register submodule ODB if needed Jonathan Tan
2021-10-07 18:34 ` Josh Steadmon
2021-10-08 20:22 ` Jonathan Tan
2021-09-29 23:06 ` [PATCH v3 6/7] submodule: pass repo to check_has_commit() Jonathan Tan
2021-09-29 23:06 ` [PATCH v3 7/7] submodule: trace adding submodule ODB as alternate Jonathan Tan
2021-10-07 18:34 ` Josh Steadmon
2021-10-08 20:23 ` Jonathan Tan
2021-10-07 18:32 ` [PATCH v3 0/7] No more " Josh Steadmon
2021-10-08 21:08 ` [PATCH v4 " Jonathan Tan
2021-10-08 21:08 ` [PATCH v4 1/7] refs: plumb repo into ref stores Jonathan Tan
2021-10-08 21:08 ` [PATCH v4 2/7] refs: teach arbitrary repo support to iterators Jonathan Tan
2021-10-08 21:08 ` [PATCH v4 3/7] refs: peeling non-the_repository iterators is BUG Jonathan Tan
2021-10-08 21:08 ` [PATCH v4 4/7] merge-{ort,recursive}: remove add_submodule_odb() Jonathan Tan
2021-10-08 21:08 ` [PATCH v4 5/7] object-file: only register submodule ODB if needed Jonathan Tan
2021-10-08 21:08 ` [PATCH v4 6/7] submodule: pass repo to check_has_commit() Jonathan Tan
2021-10-08 21:08 ` [PATCH v4 7/7] submodule: trace adding submodule ODB as alternate Jonathan Tan
2021-10-12 22:10 ` [PATCH v4 0/7] No more " Glen Choo
2021-10-12 22:40 ` Josh Steadmon
2021-10-12 22:49 ` 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=YU4VUF17tyG2AZi3@coredump.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=jonathantanmy@google.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.