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

Re: [PATCH v2 00/21] refs backend reroll

From
David Turner <dturner@twopensource.com>
Date
Jan 14, 2016, 16:25 UTC
Message-ID
<1452788752.16226.17.camel@twopensource.com>
In-Reply-To
<20160114092614.GA8533@sigill.intra.peff.net>
On Thu, 2016-01-14 at 04:26 -0500, Jeff King wrote:
Show 35 quoted lines
> On Tue, Jan 12, 2016 at 04:22:09PM -0800, Junio C Hamano wrote:
> 
> > David Turner <dturner@twopensource.com> writes:
> > 
> > > This version incorporates many changes suggested by Michael
> > > Haggerty,
> > > Junio, Jonathan Nieder, Eric Sunshine, and Jeff King. I think I
> > > have
> > > addressed of the comments that were sent to me.  Those that I
> > > chose
> > > not to incorporate, I responded to on the mailing list.
> > > 
> > > Thanks for all of the feed back so far.
> > 
> > Unfortunately this did not compile for me X-< and with a trivial
> > fix-up, I found that this overlaps with Peff's recent fixes to the
> > locking of symbolic refs.  So for today's integration run, I
> > punted.
> > 
> > I still will push out this topic to the broken-out repository I
> > keep
> > here:
> > 
> >     https://github.com/gitster/git
> > 
> > It's just 'pu' will not have this latest incarnation, but has the
> > older one.
> 
> I took a look at David's changes. The conflicts come from "refs:
> resolve symbolic refs first". I'm not sure I fully understand all
> that
> is going on in that patch, but it looks like after it, we are less
> likely to handle ENOTDIR and d/f conflicts for symrefs, as we skip
> that
> whole code path for REF_ISSYMREF.

We only get into the symref part of that codepath if there's already a symref present, meaning that d/f conflicts can't happen.

Show 11 quoted lines
> The rest of the conflicts are related to the fact that all of the
> initial resolution is pulled out of lock_ref_sha1_basic(), and the
> caller is supposed to do it. So I think if create_symref() is going
> to
> call lock_ref_sha1_basic(), as in my series, when combined with
> David's
> it should also be calling dereference_symrefs(). That uses a
> ref_transaction, which we don't have in create_symref() right now,
> but
> it makes sense that we would ultimately want to push symref updates
> through the same transaction/backend system.

I don't think that's quite true. create_symref *always* creates symrefs, and never creates underlying refs. So it calls lock_ref_sha1_basic(), but since type_p is NULL, it doesn't go into the resolved-symlinks path; instead, we get into the original codepath.

> So sorry, I don't have a quick resolution to this. I'm hoping David
> can
> make more sense of it than I did.

I was totally convinced that we were doomed, but I think the stupid resolution basically works, with some minor tweaks. I'm going to re -review that patch and resend the series (then go out of town until Tuesday).

We will need to apply your new d/f conflict check to the LMDB backend's symref code (presently, it fails your new test), but I'm going to punt on that for now since d/f conflicts don't cause problems for the LMDB backend and this is a relatively minor case. I've added a TODO to the code.

Previous: Jeff KingNext: Jeff King
Message 34 of 35 in “refs backend reroll”
  1. 00/21 refs backend rerollDavid Turner, Jan 12, 2016
  2. 01/21 refs: add a backend method structure with transaction functionsDavid Turner, Jan 12, 2016
  3. 02/21 refs: add methods for misc ref operationsDavid Turner, Jan 12, 2016
  4. 03/21 refs: add methods for the ref iteratorsDavid Turner, Jan 12, 2016
  5. 04/21 refs: add do_for_each_per_worktree_refDavid Turner, Jan 12, 2016
  6. 05/21 refs: add methods for reflogDavid Turner, Jan 12, 2016
  7. 06/21 refs: add method for initial ref transaction commitDavid Turner, Jan 12, 2016
  8. 07/21 refs: add method for delete_refsDavid Turner, Jan 12, 2016
  9. 08/21 refs: add methods to init refs dbDavid Turner, Jan 12, 2016
  10. 09/21 refs: add method to rename refsDavid Turner, Jan 12, 2016
  11. 10/21 refs: make lock genericDavid Turner, Jan 12, 2016
  12. 11/21 refs: move duplicate check to common codeDavid Turner, Jan 12, 2016
  13. 12/21 refs: allow log-only updatesDavid Turner, Jan 12, 2016
  14. 13/21 refs: resolve symbolic refs firstDavid Turner, Jan 12, 2016
  15. Junio C HamanoJan 13, 2016
  16. David TurnerJan 13, 2016
  17. 14/21 refs: always handle non-normal refs in files backendDavid Turner, Jan 12, 2016
  18. Junio C HamanoJan 13, 2016
  19. David TurnerJan 13, 2016
  20. 15/21 init: allow alternate backends to be set for new reposDavid Turner, Jan 12, 2016
  21. 16/21 refs: check submodules ref storage configDavid Turner, Jan 12, 2016
  22. Junio C HamanoJan 13, 2016
  23. David TurnerJan 13, 2016
  24. Junio C HamanoJan 13, 2016
  25. 17/21 clone: use child_process for recursive checkoutsDavid Turner, Jan 12, 2016
  26. 18/21 refs: allow ref backend to be set for cloneDavid Turner, Jan 12, 2016
  27. 19/21 svn: learn ref-storage argumentDavid Turner, Jan 12, 2016
  28. 20/21 refs: add LMDB refs backendDavid Turner, Jan 12, 2016
  29. Jeff KingJan 14, 2016
  30. David TurnerJan 14, 2016
  31. 21/21 refs: tests for lmdb backendDavid Turner, Jan 12, 2016
  32. Junio C HamanoJan 13, 2016
  33. Jeff KingJan 14, 2016
  34. David TurnerJan 14, 2016
  35. Jeff KingJan 14, 2016

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.