[PATCH 1 of 2] journal: add dirstate tracking

[Yuya Nishihara]

On Mon, 4 Jul 2016 11:58:37 +0100, Martijn Pieters wrote:
> On 3 July 2016 at 04:42, Yuya Nishihara <yuya at tcha.org> wrote:
> > On Thu, 30 Jun 2016 13:41:25 +0100, Martijn Pieters wrote:  
> >> # HG changeset patch
> >> # User Martijn Pieters <mjpieters at fb.com>
> >> # Date 1467290363 -3600
> >> #      Thu Jun 30 13:39:23 2016 +0100
> >> # Node ID 09c7b17ab01c202460543f02d9d71ac643f85a20
> >> # Parent  cf092a3d202a11a670f2bbfd514bc07d6cc4cee7
> >> journal: add dirstate tracking  
> >  
> >> +# hooks to record dirstate changes
> >> +def wrapdirstate(orig, repo):
> >> +    dirstate = orig(repo)
> >> +    if util.safehasattr(repo, 'journal'):
> >> +        dirstate.journalrepo = repo
> >> +    return dirstate  
> >
> > IIRC, it's considered layering violation to pass repo to dirstate. Perhaps,
> > it could be dirstate.journal = repo.journal, or carry journal by a transaction
> > object.  
> 
> Good point; all that is needed really is the journal storage, so
> `dirstate.journalstorage = repo.journal` should suffice here. I'll
> make the change and send a V2.

[snip]

> > I understand why this is necessary, but it seems wrong to try to take a lock
> > in lock.releasefn(), which can't guarantee consistency. We could delay
> > "lock.held = 0" after releasefn() as the lock wasn't released yet, but it
> > would result in two releasefn() calls.
> >
> > Instead, I think it will be way simpler if the journal has a dedicated lock,
> > and we can get rid of the cycle between journalstrage and localrepo.  
> 
> I still need access to the repo object to support shared repositories,
> I think. I'll look into a dedicated lock, that'd make this a lot
> simpler probably.

I guess shared path can be resolved when attaching the journalstorage to the
repo, but I just looked over the code quickly, I might be wrong. Strictly
speaking, it would be layering violation to pass an object holding repo to
dirstate.