[PATCH 1 of 3 STABLE] util: add utility function to skip avoiding file stat ambiguity if EPERM

[FUJIWARA Katsunori]

At Mon, 14 Nov 2016 22:43:04 +0900,
Yuya Nishihara wrote:
> 
> On Sun, 13 Nov 2016 06:16:08 +0900, FUJIWARA Katsunori wrote:
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
> > # Date 1478984783 -32400
> > #      Sun Nov 13 06:06:23 2016 +0900
> > # Branch stable
> > # Node ID b496a464399cb68628b09e52aa8cf379c98428e6
> > # Parent  4ed8bb8a153f91420777d98dea10ebbcd403a375
> > util: add utility function to skip avoiding file stat ambiguity if EPERM
> 
> The series looks good to me. I found a nit, but it isn't important. I'll
> push the patches tomorrow if there are no other comments.
> 
> > --- a/mercurial/util.py
> > +++ b/mercurial/util.py
> > @@ -1497,6 +1497,24 @@ class filestat(object):
> >          except AttributeError:
> >              return False
> >  
> > +    def avoidambig(self, path, old):
> > +        """Change file stat of specified path to avoid ambiguity
> > +
> > +        'old' should be previous filestat of 'path'.
> > +
> > +        This skips avoiding ambiguity, if a process doesn't have
> > +        appropriate privileges for 'path'.
> > +        """
> > +        advanced = (old.stat.st_mtime + 1) & 0x7fffffff
> > +        try:
> > +            os.utime(path, (advanced, advanced))
> > +        except OSError as inst:
> > +            if inst.errno == errno.EPERM:
> > +                # utime() on the file created by another user causes EPERM,
> > +                # if a process doesn't have appropriate privileges
> > +                return
> > +            raise
> 
> avoidambig() has no relation to self, which requires path and old filestat as
> arguments. So I don't think it should be a method of filestat class.

Oh, this useless "self" is a vestige of my wavering, sorry.

In fact, avoidambig() was implemented as a combination of examining
ambiguity (= using self.stat) and advancing st_mtime for convenience,
at first.

  before:
    if newstat.isambig(oldstat):
        advanced = (oldstat.stat.st_mtime + 1) & 0x7fffffff
        os.utime(dstpath, (advanced, advanced))

  after:
    newstat.avoidambig(oldstat, dstpath)

But finally, this patch separates advancing st_mtime from examining
ambiguity for review-ability of fixing urgent bug on stable
branch. With this simplified changes, reviewer can focus only on
centralizing code path around os.utime() into avoidambig().

Then, which fixing below would you like on stable ?

  1. define avoidambig() as a regular function in util.py

  2. mark avoidambig() by @static annotation

     this can centralize logic related to ExactCacheValidationPlan
     into filestat class

  3. integrate advancing st_mtime and examining ambiguity into
     avoidambig() for convenience

     this requires "self" for avoidambig()

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