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

Re: [PATCH 2/3] refs: introduce new API, wrap old API shallowly around new API

From
Stefan Beller <sbeller@google.com>
Date
Jul 27, 2018, 17:30 UTC
Message-ID
<CAGZ79kZfhSwtNgNk-GRDb6f4Uq7y6fi21HVO7xHv1YiuQoaSvA@mail.gmail.com>
In-Reply-To
<20180727171941.GA109508@google.com>
On Fri, Jul 27, 2018 at 10:19 AM Brandon Williams <bmwill@google.com> wrote:
Show 31 quoted lines
>
> On 07/27, Duy Nguyen wrote:
> > On Fri, Jul 27, 2018 at 2:40 AM Stefan Beller <sbeller@google.com> wrote:
> > >
> > > Currently the refs API takes a 'ref_store' as an argument to specify
> > > which ref store to iterate over; however it is more useful to specify
> > > the repository instead (or later a specific worktree of a repository).
> >
> > There is no 'later'. worktrees.c already passes a worktree specific
> > ref store. If you make this move you have to also design a way to give
> > a specific ref store now.
> >
> > Frankly I still dislike the decision to pass repo everywhere,
> > especially when refs code already has a nice ref-store abstraction.
> > Some people frown upon back pointers. But I think adding a back
> > pointer in ref-store, pointing back to the repository is the right
> > move.
>
> I don't quite understand why the refs code would need a whole repository
> and not just the ref-store it self.  I thought the refs code was self
> contained enough that all its state was based on the passed in
> ref-store.  If its not, then we've done a terrible job at avoiding
> layering violations (well actually we're really really bad at this in
> general, and I *think* we're trying to make this better though the
> object store/index refactoring).
>
> If anything I would expect that the actual ref-store code would remain
> untouched by any refactoring and that instead the higher-level API that
> hasn't already been converted to explicitly use a ref-store (and instead
> just calls the underlying impl with get_main_ref_store()).  Am I missing
> something here?

Then I think we might want to go with the original in Stolees proposal https://github.com/gitgitgadget/git/pull/11/commits/300db80140dacc927db0d46c804ca0ef4dcc1be1 but there the call to for_each_replace_ref just looks ugly, as it takes the repository as both the repository where to obtain the ref store from as well as the back pointer.

I anticipate that we need to have a lot of back pointers to the repository in question, hence I think we should have the repository pointer promoted to not just a back pointer.

Previous: Brandon WilliamsNext: Duy Nguyen
Message 6 of 14 in “Migrate the refs API to take the repository argument”
  1. 0/3 Migrate the refs API to take the repository argumentStefan Beller, Jul 27, 2018
  2. 1/3 refs.c: migrate internal ref iteration to pass thru repository argumentStefan Beller, Jul 27, 2018
  3. 2/3 refs: introduce new API, wrap old API shallowly around new APIStefan Beller, Jul 27, 2018
  4. Duy NguyenJul 27, 2018
  5. Brandon WilliamsJul 27, 2018
  6. Stefan BellerJul 27, 2018
  7. Duy NguyenJul 27, 2018
  8. 0/2 Cleanup refs API [WAS: Re: [PATCH 2/3] refs: introduce new API, wrap old API shallowly around new API]Stefan Beller, Jul 30, 2018
  9. 1/2 replace-objects: use arbitrary repositoriesStefan Beller, Jul 30, 2018
  10. 2/2 refs: switch for_each_replace_ref back to use a ref_storeStefan Beller, Jul 30, 2018
  11. Jonathan TanJul 31, 2018
  12. Stefan BellerJul 31, 2018
  13. Duy NguyenJul 31, 2018
  14. 3/3 replace: migrate to for_each_replace_repo_refStefan Beller, Jul 27, 2018

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.