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

[FUJIWARA Katsunori]

At Thu, 7 Jul 2016 18:00:16 +0100,
Martijn Pieters wrote:
> 
> On 5 July 2016 at 19:35, FUJIWARA Katsunori <foozy at lares.dti.ne.jp> wrote:
> >> 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.
> 
> Okay, I take back what I said earlier. setparents is too noisy by half.
> 
> The goal is to give users a clear record of where their working copy
> has been as they execute commands; we point users to journal when they
> need to recover from an accidental hg amend, hg strip or a histedit
> gone wrong, for example.

I had supposed finer (capturing setparents() and restorebackup()) or
larger (capturing before/after each commands) grain recording than you
thinking. I see.

Some additional help document (maybe ".. container:: verbose" ?) or so
about intermediate changes might help users in the future (e.g. fully
transactional action like "hg import" doesn't record any intermediate
one if no external hook is invoked, but other like "hg histedit"
does).


> > 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 :-)
> 
> If a transaction fails, the final state the user ends up in should be
> the same as before the transaction was started, so this would be too
> noisy too. And it sounds as if shelve is going to make this more
> confusing still if we were to record everything.
> 
> Wrapping dirstate._writedirstate gives us exactly the right level of
> detail (where we use dirstate._opendirstatefile() now). I'll send a V3
> of the patch with just the change from reading .hg/dirstate to
> .hg/dirstate or .hg/dirstate.pending. Note that there is no point
> anywhere in the mercurial testsuite where the pending dirstate differs
> from the final dirstate. I'd love to see a test case where hg journal
> would record a distinct pending dirstate, followed by a different
> final dirstate. I wasn't able to create a testcase for this at this
> stage.

Is this test scenario in test-import.t suited to your requirement ?

    https://selenic.com/repo/hg/file/tip/tests/test-import.t#l454

In this case, intermediate changes in transaction for "hg import" are
written into .hg/dirstate.pending file at each external hook (or
editor) invocation, and some of them differs from final one at closing
transaction.

> I feel at this point we can always *add* more recording later on if
> important states are missed, including complex hook configurations.
> 
> -- 
> Martijn Pieters
> 

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