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

[Martijn Pieters]

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.

> 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.

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

-- 
Martijn Pieters