[PATCH 2 of 5] localrepo: invoke dirstate.unsureifambig in wwrite for safety

[FUJIWARA Katsunori]

At Thu, 25 Jun 2015 15:35:40 +0000,
Martin von Zweigbergk wrote:
> 
> On Thu, Jun 25, 2015 at 2:54 AM FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
> wrote:
> 
> > At Mon, 22 Jun 2015 18:23:29 +0000,
> > Martin von Zweigbergk wrote:
> > >
> > > On Mon, Jun 22, 2015 at 9:19 AM FUJIWARA Katsunori <
> > foozy at lares.dti.ne.jp>
> > > wrote:
> >
> > (snip)
> >
> > > > I summarized about "dirstate changes at failure" as a basis for
> > > > discussion. Please comment !
> > > >
> > > > BTW, it is assumed that `dirstate.normallookup` is invoked at
> > > > `repo.wwrite` or so, because, without these changes, problems like
> > > > issue4583 ("changed files are treated as clean") appear occasionally,
> > > > even if processing terminates successfully (e.g, at `hg revert`, `hg
> > > > import` and so on):
> > > >
> > >
> > > As I wrote before, I think the general problem is "that dirstate gets
> > > updated but not written to disk before working copy files get written to
> > > disk". I hope that is a correct assessment. I'm not convinced that
> > calling
> > > dirstate.normallookup() is the right solution (but I'm also not convinced
> > > it's not). It seems like an alternative would be to not write working
> > copy
> > > files while the dirstate is dirty. Hopefully someone else can have an
> > > opinion on this. It took me a long time to understand the issue even just
> > > somewhat well, so I understand that it's a lot to ask, but I think we
> > need
> > > more than just two people thinking about this.
> > >
> > > I haven't yet read the remainder of the email.
> >
> > The timetable below shows steps in typical case:
> >
> >   ---- ----------------------------------- ----------------
> >                                            timestamp of "f"
> >                                            ----------------
> >                                            dirstate    file
> >   time          action                     mem  file   system
> >   ---- ----------------------------------- ---- ----- -----
> >   N                                              -1    ***
> >        - change file "f"                                N
> >
> >        - execute `hg foobar`
> >          - get wlock
> >          - instanciate dirstate             -1   -1
> >          - `dirstate.normal("f")`           N    -1
> >            (e.g. via dirty check)
> >
> >          (*1)
> >
> >          - overwrite "f", but keep size                 N
> >   N+1
> >          - release wlock
> >            - `dirstate.write()`             N    N
> >
> >        - `hg status` shows "f" as "clean"   N    N      N
> >   ---- ----------------------------------- ---- ----- -----
> >
> > Your solution is "dirstate should be written out at (*1) above", isn't
> > it ?
> >
> 
> Yes, exactly. Thanks for the very detailed explanation. Before I comment on
> the details below, let's make sure we agree on what would happen during the
> write operation at (*1): Since the timestamp in the dirstate (N) is the
> same as the current time, the dirstate timestamp, in memory and on file,
> would be replaced by -1. Agree?

Oops, sorry I mis-understood that `parsers.pack_dirstate` replaces all
"N" timestamps at writing out without changing dirstate entries in
memory :-<

Yes, you are right. The dirstate timestamp, in memory and on file,
would be replaced by -1.

Then, corrected timetable is:

  ---- ----------------------------------- ----------------
                                           timestamp of "f"
                                           ----------------
                                           dirstate    file
  time          action                     mem  file   system
  ---- ----------------------------------- ---- ----- -----
  N                                              -1    ***
       - change file "f"                                N

       - execute `hg foobar`
         - get wlock
         - instanciate dirstate             -1   -1
         - `dirstate.normal("f")`           N    -1
           (e.g. via dirty check)

         - `dirstate.write()` as (*1)       -1   -1

         - overwrite "f", but keep size                 N
  N+1
         - release wlock
           - `dirstate.write()`             -1   -1

       - `hg status` shows "f" as "clean"   -1   -1     N
  ---- ----------------------------------- ---- ----- -----


> This is no different from running "hg status" right before "hg foobar"
> (i.e. still within time N), right?

Yes.


[snip]

> > On the other hand, in `hg revert --rev REV` case, it doesn't work as
> > expected, because:
> >
> >   - #2 and/or #4 break condition (1), and
> >   - #3 breaks condition (2)
> >
> >     ================
> >     commands.revert():
> >       cmdutil.revert():
> >         with repo.wlock() as wlock:
> >           repo.status('to get current status') #1
> >
> >           dirstate.write("as (*1) in timetable above")
> >
> >           repo.wwrite('files to be reverted')
> >           dirstate.remove('reverting to un-added') #2
> >           dirstate.normal('reverting to un-changed) if not opts['rev'] #3
> >           dirstate.add('reverting to un-removed ') #4
> >     ================

`dirstate.write()` at (*1) seems to work well also at `hg revert --rev
REV`.

OK, I'll try to fix problems by invoking `dirstate.write()` at (*1) !

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