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

[FUJIWARA Katsunori]

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 ?

Once I thought that it can resolve this kind of issue, too. But I
noticed that it requires one of conditions below:

  1. dirstate is never written out after (*1)
    
     In other words, "none of entries in dirstate is updated after (*1)".

     Updating ones for other files marks dirstate as "dirty" and
     causes writing timestamp N of "f" again at subsequent
     `dirstate.write`.

  2. dirstate entry of "f" is certainly updated by `normallookup`

     This drops timestamp N from in-memory dirstate changes, and
     subsequent `dirstate.write` doesn't cause issue.


In `merge.update` case, it works as expected:

  - at failure, (1) should be satisfied, because dirstate is updated
    only in `merge.recordupdates`

  - otherwise, (2) should be satisfied, because all dirstate entries
    of updated files are updated in `merge.recordsupdates`


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

`hg import` using `patch.patch` also causes similar situation, because
it updates dirstate entries only for files to be added or removed.


Then, we should always apply `dirstate.normallookup` on files being
changed (= removing `if not opts['rev']` at #3), for "`dirstate.write`
before working copy files get written to disk".

But this decreases performance of subsequent `hg status` or so,
because dropping timestamp/size forces examination of file content.


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