[PATCH 2 of 5] util: make atomictempfile avoid ambiguity of file stat if needed

[FUJIWARA Katsunori]

At Tue, 24 May 2016 22:39:26 +0900,
Yuya Nishihara wrote:
> 
> On Thu, 19 May 2016 00:26:28 +0900, FUJIWARA Katsunori wrote:
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
> > # Date 1463584838 -32400
> > #      Thu May 19 00:20:38 2016 +0900
> > # Node ID 561ed91a420adfee1e1386e6dd20fea5ad8c1db5
> > # Parent  dc731ebd60613cf3f08799a8d8dc48435798665b
> > util: make atomictempfile avoid ambiguity of file stat if needed
> > 
> > Ambiguity check is executed at close(), only if:
> > 
> >   - atomictempfile is created with checkambig=True, and
> >   - target file exists before renaming
> > 
> > This restriction avoids performance decrement by needless examination
> > of file stat (for example, filelog doesn't need exact cache
> > validation, even though it uses atomictempfile to write changes out).
> > 
> > See description of filestat class for detail about why the logic in
> > this patch works as expected.
> > 
> > This patch is a part of preparation for "Exact Cache Validation Plan":
> > 
> >     https://www.mercurial-scm.org/wiki/ExactCacheValidationPlan
> > 
> > diff --git a/mercurial/util.py b/mercurial/util.py
> > --- a/mercurial/util.py
> > +++ b/mercurial/util.py
> > @@ -1453,11 +1453,12 @@ class atomictempfile(object):
> >      visible. If the object is destroyed without being closed, all your
> >      writes are discarded.
> >      '''
> > -    def __init__(self, name, mode='w+b', createmode=None):
> > +    def __init__(self, name, mode='w+b', createmode=None, checkambig=False):
> >          self.__name = name      # permanent name
> >          self._tempname = mktempcopy(name, emptyok=('w' in mode),
> >                                      createmode=createmode)
> >          self._fp = posixfile(self._tempname, mode)
> > +        self._checkambig = checkambig
> >  
> >          # delegated methods
> >          self.write = self._fp.write
> > @@ -1468,7 +1469,17 @@ class atomictempfile(object):
> >      def close(self):
> >          if not self._fp.closed:
> >              self._fp.close()
> > -            rename(self._tempname, localpath(self.__name))
> > +            filename = localpath(self.__name)
> > +            oldstat = self._checkambig and filestat(filename)
> > +            if oldstat and oldstat.stat:
> > +                rename(self._tempname, filename)
> > +                newstat = filestat(filename)
> > +                if newstat.isambig(oldstat):
> > +                    # stat of changed file is ambiguous to original one
> > +                    advanced = (oldstat.stat.st_mtime + 1) & 0x7fffffff
> > +                    os.utime(filename, (advanced, advanced))
> 
> There might be a race if two writers do stat-rename-utime, but we don't care
> because that would be unavoidable and wouldn't normally happen thanks to
> wlock/lock. Is that correct?

Yes, this code path is used only for files guarded by wlock/(s)lock,
and such kind of race should be avoided.

Should it be described in filestat (or clients of it) ?

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