git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [RFC PATCH 1/2] refs: make _advance() check struct repo, not flag

From
Han-Wen Nienhuys <hanwen@google.com>
Date
Aug 26, 2021, 16:39 UTC
Message-ID
<CAFQ2z_PKKZJY1kC1QJo8Zwq_yNh5QNGc3S5bq1jBfSfK3vQwRQ@mail.gmail.com>
In-Reply-To
<9187eab52552630863285ef5743a107ccc555495.1629933380.git.jonathantanmy@google.com>
On Thu, Aug 26, 2021 at 1:23 AM Jonathan Tan <jonathantanmy@google.com> wrote:
Show 21 quoted lines
>
> 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)

from a design perspective, it would be nice if the ref backend wouldn't need to know about the object store. Can't this be hidden in the layer in refs.c that calls into the backends?

If they have to know about the object store, have you considered passing the repository pointer in xxx_ref_store_create() ? Then there is no possibliity to mismatch the repository pointers and with the ref store.

Show 8 quoted lines
> - Making all ref stores not access the object store during their
>   _advance() callbacks, and making ref_iterator_advance() be responsible
>   for checking the object store - thus, simplifying the code in that the
>   logic of checking for the flag (current) or the pointer (after the
>   equivalent of this commit) is only in one place instead of in every
>   ref store's callback. However, the ref stores already make use of this
>   flag for another reason - for determining if refs are resolvable when
>   writing (search for "REF_STORE_ODB"). Thus, I decided to retain each
I looked, but I couldn't figure out how this flag is used.
-- 
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--
Han-Wen Nienhuys
Google Munich
hanwen@google.com
Previous: Jonathan TanNext: Jonathan Tan
Message 3 of 11 in “First steps towards iterating over submodule refs”
  1. 0/2 First steps towards iterating over submodule refsJonathan Tan, Aug 25, 2021
  2. 1/2 refs: make _advance() check struct repo, not flagJonathan Tan, Aug 25, 2021
  3. Han-Wen NienhuysAug 26, 2021
  4. Jonathan TanAug 26, 2021
  5. Glen ChooSep 14, 2021
  6. Han-Wen NienhuysSep 15, 2021
  7. Jonathan TanSep 16, 2021
  8. Junio C HamanoSep 16, 2021
  9. Jonathan TanSep 16, 2021
  10. Jonathan TanSep 16, 2021
  11. 2/2 refs: add repo paramater to _iterator_peel()Jonathan Tan, Aug 25, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.