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

[Martin von Zweigbergk]

On Tue, Jun 30, 2015 at 8:50 AM FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
wrote:

> At Mon, 29 Jun 2015 04:23:47 +0000,
> Martin von Zweigbergk wrote:
> >
> > [1  <text/plain; UTF-8 (7bit)>]
> > On Sun, Jun 28, 2015 at 9:07 AM FUJIWARA Katsunori <
> foozy at lares.dti.ne.jp>
> > wrote:
> >
> > >
> > > OK, I'll try to fix problems by invoking `dirstate.write()` at (*1) !
> >
> >
> > Great! I wasn't sure if I was missing something and wasting everyone's
> > time. I'm glad it now seems it was worthwhile. And thanks for working on
> > this! I'm pretty sure I've been bitten at least twice by this while
> > rebasing (dirty file appeared clean).
>
> Thanks for your many suggestions, too !
>
>
> > Just a reminder that adding a check for dirty dirstate in
> > localrepo.wwrite() seems like a good way to find the places to fix (and
> > perhaps for printing a warning after we've fixed the known buggy
> callers).
>
> You mean steps like below, or others ?
>

I meant that something as simple as adding the following two lines to the
top of localrepo.wwrite() seems helpful for finding bugs:

  if self.dirstate._dirty:
      util.dst("BUG: writing file to disk while dirstate is dirty")



>
>   - `localrepo.wwrite()` tells dirstate that the file is "updated"
>
>   - dirstate clears "updated" mark of files, on which `normal`,
>     `normallookup` and so on are applied
>
>   - `dirstate.write()` warns (with `ui.develwarn`) if there are files
>     still marked as "updated"
>
>
> BTW, as Pierre-Yves pointed out before, `merge.update` may invoke
> `localrepo.wwrite` on forked worker processes, and marking in
> `localrepo.wwrite` doesn't work well in such case :-<
>
>
> > Does it make sense for workingctx._dirstatestatus() to do the writing? Or
> > maybe it needs to be done at a higher level that's aware of any running
> > transaction?
>
> I'm putting `dirstate.write()` into `workingctx._checklookup()` at
> first, because occasional test failure for "unexpected status"
> prevents any subsequent development :-)
>
> Then, explicit `dirstate.write()` invocations will be replaced by
> another one if needed, while making in-memory dirstate changes visible
> to external process.
>
>
> ----------------------------------------------------------------------
> [FUJIWARA Katsunori]                             foozy at lares.dti.ne.jp
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150630/ce6199f2/attachment.html>