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

[Martin von Zweigbergk]

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?

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


>
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150625/f9086993/attachment.html>