Re: [PATCH v5 25/27] refs: add LMDB refs storage backend

[David Turner <dturner@twopensource.com>]

On Fri, 2016-02-19 at 09:54 +0700, Duy Nguyen wrote:
> On Fri, Feb 19, 2016 at 3:23 AM, David Turner <
> dturner@twopensource.com> wrote:
> > > > +static int read_per_worktree_ref(const char *submodule, const
> > > > char
> > > > *refname,
> > > > +                            struct MDB_val *val, int
> > > > *needs_free)
> > > 
> > > From what I read, I suspect these _per_worktree functions will be
> > > identical for the next backend. Should we just hand over the job
> > > for
> > > files backend? For all entry points that may deal with per
> > > -worktree
> > > refs, e.g. lmdb_resolve_ref_unsafe, can we check ref_type() first
> > > thing, if it's per-worktree we call
> > > refs_be_files.resolve_ref_unsafe()
> > > instead?  It could even be done at frontend level,
> > > e.g. refs.c:resolve_ref_unsafe().
> > > 
> > > Though I may be talking rubbish here because I don't know how
> > > whether
> > > it has anything to do with transactions.
> > 
> > The reason I did it this way is that some ref chains cross backend
> > boundaries (e.g. HEAD -> refs/heads/master).  But if we have other
> > backends later, we could generalize.
> 
> Crossing backends should go through frontend again, imo. But I don't
> really know if it's efficient.

It's pretty tricky to maintain state (e.g. count of symref redirects)
across that barrier.  So I'm not sure how to do it cleanly.

> > > > +static int lmdb_create_symref(const char *ref_target,
> > > > +                         const char *refs_heads_master,
> > > > +                         const char *logmsg)
> > > > +{
> > > > +
> > > ...
> > > > +   mdb_put_or_die(&transaction, &key, &val, 0);
> > > > +
> > > > +   /* TODO: Don't create ref d/f conflicts */
> > > 
> > > I'm not sure I get this comment. D/F conflicts are no longer a
> > > thing
> > > for lmdb backend, right?
> > 
> > I'm trying to avoid the lmdb backend creating a set of refs that
> > the
> > files backend can't handle.  This would make collaboration with
> > other
> > versions of git more difficult.
> 
> It already is. If you create refs "foo" and "FOO" on case sensitive
> file system and clone it on a case-insensitive one, you face the same
> problem. We may have an optional configuration knob to prevent
> incompatibilities with files backend, but I think that should be done
> (and enforced if necessary) outside backends.

Sure, the current state isn't perfect, but why make it worse?