[PATCH 1 of 3 V2] journal: add dirstate tracking

FUJIWARA Katsunori foozy at lares.dti.ne.jp
Tue Jul 5 14:35:37 EDT 2016


At Tue, 5 Jul 2016 09:48:37 +0100,
Martijn Pieters wrote:
> 
> On 4 July 2016 at 22:28, FUJIWARA Katsunori <foozy at lares.dti.ne.jp> wrote:
> > At Mon, 04 Jul 2016 15:35:56 +0100,
> > Martijn Pieters wrote:
> >>  # Journal recording, register hooks and storage object
> >>  def extsetup(ui):
> >>      extensions.wrapfunction(dispatch, 'runcommand', runcommand)
> >>      extensions.wrapfunction(bookmarks.bmstore, '_write', recordbookmarks)
> >> +    extensions.wrapfunction(
> >> +        dirstate.dirstate, '_writedirstate', recorddirstateparents)
> >> +    extensions.wrapfunction(
> >> +        localrepo.localrepository.dirstate, 'func', wrapdirstate)
> >>
> >
> > Wrapping "dirstate._writedirstate()" might track intermediate changing
> > parents while automated commit like backout, graft, import, rebase and
> > so on.
> >
> > Recording such intermediate status depends on:
> >
> >   - whether changing parents while automated commit occurs inside
> >     transaction or not
> >
> >     - if so (e.g. import):
> >
> >       - whether external hook is invoked or not inside transaction scope
> >
> >         - if so, intermediate commit writes changes (= changing
> >           parents) out at external hook invocation
> >
> >         - otherwise, writing changes out is delayed until transaction
> >           closing
> >
> >     - otherwise (e.g. graft):
> >
> >       actions below immediately write intermediate changes out.
> >
> >       - intermediate commit
> >       - external hook invocation after internal updating
> >
> > (see also https://www.mercurial-scm.org/wiki/DirstateTransactionPlan
> > for detail about dirstate and transaction)
> >
> > What would you think about appearance of such intermediate change in
> > journal records ?
> >
> > IMHO, from the point of view of end user, comparison between "parents"
> > before and after each command invocation seems enough resolution for
> > recording changes.
> 
> I'd rather record a dirstate change too many than too few here; they
> may want to recover to an earlier state *within* a set of steps driven
> by automation.

I see.


> >> +def recorddirstateparents(orig, dirstate, dirstatefp):
> >> +    """Records all dirstate parent changes in the journal."""
> >> +    if util.safehasattr(dirstate, 'journalstorage'):
> >> +        old = [node.nullid, node.nullid]
> >> +        nodesize = len(node.nullid)
> >> +        try:
> >> +            # The only source for the old state is in the dirstate file still
> >> +            # on disk; the in-memory dirstate object only contains the new
> >> +            # state. dirstate._filename is always the actual dirstate file;
> >> +            # backups for a transactions, shelving or the repo journal use a
> >> +            # suffix or prefix, _filename never changes.
> >> +            with dirstate._opener(dirstate._filename) as fp:
> >> +                state = fp.read(2 * nodesize)
> >> +            if len(state) == 2 * nodesize:
> >> +                old = [state[:nodesize], state[nodesize:]]
> >> +        except IOError:
> >> +            pass
> >> +
> >> +        new = dirstate.parents()
> >> +        if old != new:
> >
> > Comparison between "parents" in memory and in ".hg/dirstate" might
> > imply unexpected record for automated commit inside transaction,
> > because changes inside transaction are written into ".hg/dirstate.pending".
> >
> >   - this comparison always concludes that "parents" is changed, even
> >     if it isn't changed after previous recording
> >
> >   - "parents" stored in ".hg/dirstate" is always used as the source of
> >     changing parents
> >
> >     e.g. if automated commit implies A -> I1 -> I2 -> I3 -> B commits,
> >     this records (A -> I1), (A -> I2), (A -> I3), (A -> B).
> >
> >     (if external hook is invoked)
> 
> Right; so would reading `.hg/dirstate.pending` if present be a better
> idea here then? We can use `with dirstate._opendirstatefile() as fp:`
> in that case, which will select between `.hg/dirstate` and
> `.hg/dirstate.pending` as needed.

As described in my previous reply, writing dirstate changes out
depends on transaction and external hook configuration.

For consistent (and detailed) recording, "change of parents" should be
recorded regardless of writing dirstate out, IMHO.

Therefore, how about recording "change of parents" immediately at
dirstate.setparents() by wrapping it ? This can record all "logical"
(and internal) change of parents.


In addition to it, restoring dirstate from backup in cases below
should be recorded, too.

  - failure of transaction
  - rollback of previous transaction (via "hg rollback")
  - failure in dirstateguard scope

Now, wrapping dirstate.restorebackup() can record restoring dirstate
in all cases.


BTW, at the end of shelve/unshelve, "(explicit) abortion of
transaction" and "restoring from backup" are combined in non-standard
style, and strict recording this sequence might cause confusing result :-)


> -- 
> Martijn Pieters
> 

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy at lares.dti.ne.jp


More information about the Mercurial-devel mailing list