Patchwork D2696: cleanup: use stat_result[stat.ST_MTIME] instead of stat_result.st_mtime

login
register
mail settings
Submitter phabricator
Date March 5, 2018, 9:09 p.m.
Message ID <differential-rev-PHID-DREV-o2axgwqybqu6ilynhtnh-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/29049/
State Superseded
Headers show

Comments

phabricator - March 5, 2018, 9:09 p.m.
durin42 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  The latter is floating point by default, and we've been doing
  os.stat_float_times(False). Unfortunately, os.stat_float_times was
  removed between Python 3.7.0a1 and 3.7.0b2, so we have to stop using
  it.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2696

AFFECTED FILES
  hgext/extdiff.py
  hgext/shelve.py
  mercurial/cext/osutil.c
  mercurial/chgserver.py
  mercurial/context.py
  mercurial/debugcommands.py
  mercurial/dirstate.py
  mercurial/hg.py
  mercurial/hgweb/common.py
  mercurial/posix.py
  mercurial/util.py
  tests/svn-safe-append.py
  tests/test-atomictempfile.py
  tests/test-context.py
  tests/test-filecache.py

CHANGE DETAILS




To: durin42, #hg-reviewers
Cc: mercurial-devel
phabricator - March 6, 2018, 12:05 a.m.
indygreg added inline comments.

INLINE COMMENTS

> osutil.c:125
>  static PyObject *listdir_stat_getitem(PyObject *self, PyObject *key) {
> -	long index = PyInt_AsLong(key);
> +	long index = PyLong_AsLong(key);
>  	if (index == -1 && PyErr_Occurred()) {

Is this valid for Python 2? The passed argument will likely be a PyInt on Python 2. I thought `PyLong_AsLong` only operates on int types?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2696

To: durin42, #hg-reviewers
Cc: indygreg, mercurial-devel
phabricator - March 6, 2018, 12:58 a.m.
durin42 added inline comments.

INLINE COMMENTS

> indygreg wrote in osutil.c:125
> Is this valid for Python 2? The passed argument will likely be a PyInt on Python 2. I thought `PyLong_AsLong` only operates on int types?

Seems to (I also could have sworn int and long became the same thing in 2.6), as it passes the entire testsuite.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2696

To: durin42, #hg-reviewers
Cc: indygreg, mercurial-devel
phabricator - March 8, 2018, 1 p.m.
yuja added a comment.


  Queued, thanks.
  
  Maybe we'll need to update cffi code as a follow-up.

INLINE COMMENTS

> chgserver.py:552
>              return (stat.st_ino == self._socketstat.st_ino and
> -                    stat.st_mtime == self._socketstat.st_mtime)
> +                    stat.st_mtime == self._socketstat[stat.ST_MTIME])
>          except OSError:

Fixed name conflicts and st_mtime oversight.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2696

To: durin42, #hg-reviewers, yuja
Cc: indygreg, mercurial-devel

Patch

diff --git a/tests/test-filecache.py b/tests/test-filecache.py
--- a/tests/test-filecache.py
+++ b/tests/test-filecache.py
@@ -1,5 +1,6 @@ 
 from __future__ import absolute_import, print_function
 import os
+import stat
 import subprocess
 import sys
 
@@ -200,7 +201,7 @@ 
         fp.close()
 
         oldstat = os.stat(filename)
-        if oldstat.st_ctime != oldstat.st_mtime:
+        if oldstat[stat.ST_CTIME] != oldstat[stat.ST_MTIME]:
             # subsequent changing never causes ambiguity
             continue
 
@@ -219,16 +220,17 @@ 
                 fp.write('BAR')
 
         newstat = os.stat(filename)
-        if oldstat.st_ctime != newstat.st_ctime:
+        if oldstat[stat.ST_CTIME] != newstat[stat.ST_CTIME]:
             # timestamp ambiguity was naturally avoided while repetition
             continue
 
         # st_mtime should be advanced "repetition * 2" times, because
         # all changes occurred at same time (in sec)
-        expected = (oldstat.st_mtime + repetition * 2) & 0x7fffffff
-        if newstat.st_mtime != expected:
-            print("'newstat.st_mtime %s is not %s (as %s + %s * 2)" %
-                  (newstat.st_mtime, expected, oldstat.st_mtime, repetition))
+        expected = (oldstat[stat.ST_MTIME] + repetition * 2) & 0x7fffffff
+        if newstat[stat.ST_MTIME] != expected:
+            print("'newstat[stat.ST_MTIME] %s is not %s (as %s + %s * 2)" %
+                  (newstat[stat.ST_MTIME], expected,
+                   oldstat[stat.ST_MTIME], repetition))
 
         # no more examination is needed regardless of result
         break
diff --git a/tests/test-context.py b/tests/test-context.py
--- a/tests/test-context.py
+++ b/tests/test-context.py
@@ -1,5 +1,6 @@ 
 from __future__ import absolute_import, print_function
 import os
+import stat
 from mercurial.node import hex
 from mercurial import (
     context,
@@ -170,7 +171,8 @@ 
     # touch 00manifest.i mtime so storecache could expire.
     # repo.__dict__['manifestlog'] is deleted by transaction releasefn.
     st = repo.svfs.stat('00manifest.i')
-    repo.svfs.utime('00manifest.i', (st.st_mtime + 1, st.st_mtime + 1))
+    repo.svfs.utime('00manifest.i',
+                    (st[stat.ST_MTIME] + 1, st[stat.ST_MTIME] + 1))
 
     # read the file just committed
     try:
diff --git a/tests/test-atomictempfile.py b/tests/test-atomictempfile.py
--- a/tests/test-atomictempfile.py
+++ b/tests/test-atomictempfile.py
@@ -3,6 +3,7 @@ 
 import glob
 import os
 import shutil
+import stat
 import tempfile
 import unittest
 
@@ -66,7 +67,7 @@ 
         for i in xrange(5):
             atomicwrite(False)
             oldstat = os.stat(self._filename)
-            if oldstat.st_ctime != oldstat.st_mtime:
+            if oldstat[stat.ST_CTIME] != oldstat[stat.ST_MTIME]:
                 # subsequent changing never causes ambiguity
                 continue
 
@@ -77,14 +78,14 @@ 
             for j in xrange(repetition):
                 atomicwrite(True)
             newstat = os.stat(self._filename)
-            if oldstat.st_ctime != newstat.st_ctime:
+            if oldstat[stat.ST_CTIME] != newstat[stat.ST_CTIME]:
                 # timestamp ambiguity was naturally avoided while repetition
                 continue
 
             # st_mtime should be advanced "repetition" times, because
             # all atomicwrite() occurred at same time (in sec)
-            self.assertTrue(newstat.st_mtime ==
-                            ((oldstat.st_mtime + repetition) & 0x7fffffff))
+            oldtime = (oldstat[stat.ST_MTIME] + repetition) & 0x7fffffff
+            self.assertTrue(newstat[stat.ST_MTIME] == oldtime)
             # no more examination is needed, if assumption above is true
             break
         else:
diff --git a/tests/svn-safe-append.py b/tests/svn-safe-append.py
--- a/tests/svn-safe-append.py
+++ b/tests/svn-safe-append.py
@@ -6,23 +6,23 @@ 
 Without this svn will not detect workspace changes."""
 
 import os
+import stat
 import sys
 
 text = sys.argv[1]
 fname = sys.argv[2]
 
 f = open(fname, "ab")
 try:
-    before = os.fstat(f.fileno()).st_mtime
+    before = os.fstat(f.fileno())[stat.ST_MTIME]
     f.write(text)
     f.write("\n")
 finally:
     f.close()
 inc = 1
-now = os.stat(fname).st_mtime
+now = os.stat(fname)[stat.ST_MTIME]
 while now == before:
     t = now + inc
     inc += 1
     os.utime(fname, (t, t))
-    now = os.stat(fname).st_mtime
-
+    now = os.stat(fname)[stat.ST_MTIME]
diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -1567,7 +1567,8 @@ 
                     newstat = filestat.frompath(dest)
                     if newstat.isambig(oldstat):
                         # stat of copied file is ambiguous to original one
-                        advanced = (oldstat.stat.st_mtime + 1) & 0x7fffffff
+                        advanced = (
+                            oldstat.stat[stat.ST_MTIME] + 1) & 0x7fffffff
                         os.utime(dest, (advanced, advanced))
         except shutil.Error as inst:
             raise Abort(str(inst))
@@ -1963,8 +1964,8 @@ 
             # avoided, comparison of size, ctime and mtime is enough
             # to exactly detect change of a file regardless of platform
             return (self.stat.st_size == old.stat.st_size and
-                    self.stat.st_ctime == old.stat.st_ctime and
-                    self.stat.st_mtime == old.stat.st_mtime)
+                    self.stat[stat.ST_CTIME] == old.stat[stat.ST_CTIME] and
+                    self.stat[stat.ST_MTIME] == old.stat[stat.ST_MTIME])
         except AttributeError:
             pass
         try:
@@ -2003,7 +2004,7 @@ 
         S[n].mtime", even if size of a file isn't changed.
         """
         try:
-            return (self.stat.st_ctime == old.stat.st_ctime)
+            return (self.stat[stat.ST_CTIME] == old.stat[stat.ST_CTIME])
         except AttributeError:
             return False
 
@@ -2018,7 +2019,7 @@ 
 
         Otherwise, this returns True, as "ambiguity is avoided".
         """
-        advanced = (old.stat.st_mtime + 1) & 0x7fffffff
+        advanced = (old.stat[stat.ST_MTIME] + 1) & 0x7fffffff
         try:
             os.utime(path, (advanced, advanced))
         except OSError as inst:
@@ -2069,7 +2070,7 @@ 
                 newstat = filestat.frompath(filename)
                 if newstat.isambig(oldstat):
                     # stat of changed file is ambiguous to original one
-                    advanced = (oldstat.stat.st_mtime + 1) & 0x7fffffff
+                    advanced = (oldstat.stat[stat.ST_MTIME] + 1) & 0x7fffffff
                     os.utime(filename, (advanced, advanced))
             else:
                 rename(self._tempname, filename)
diff --git a/mercurial/posix.py b/mercurial/posix.py
--- a/mercurial/posix.py
+++ b/mercurial/posix.py
@@ -617,8 +617,8 @@ 
                     self.stat.st_uid == other.stat.st_uid and
                     self.stat.st_gid == other.stat.st_gid and
                     self.stat.st_size == other.stat.st_size and
-                    self.stat.st_mtime == other.stat.st_mtime and
-                    self.stat.st_ctime == other.stat.st_ctime)
+                    self.stat[stat.ST_MTIME] == other.stat[stat.ST_MTIME] and
+                    self.stat[stat.ST_CTIME] == other.stat[stat.ST_CTIME])
         except AttributeError:
             return False
 
diff --git a/mercurial/hgweb/common.py b/mercurial/hgweb/common.py
--- a/mercurial/hgweb/common.py
+++ b/mercurial/hgweb/common.py
@@ -12,6 +12,7 @@ 
 import errno
 import mimetypes
 import os
+import stat
 
 from .. import (
     encoding,
@@ -132,7 +133,7 @@ 
         return os.stat(spath)
 
 def get_mtime(spath):
-    return get_stat(spath, "00changelog.i").st_mtime
+    return get_stat(spath, "00changelog.i")[stat.ST_MTIME]
 
 def ispathsafe(path):
     """Determine if a path is safe to use for filesystem access."""
diff --git a/mercurial/hg.py b/mercurial/hg.py
--- a/mercurial/hg.py
+++ b/mercurial/hg.py
@@ -12,6 +12,7 @@ 
 import hashlib
 import os
 import shutil
+import stat
 
 from .i18n import _
 from .node import (
@@ -1113,8 +1114,8 @@ 
                 st = os.stat(p)
             except OSError:
                 st = os.stat(prefix)
-            state.append((st.st_mtime, st.st_size))
-            maxmtime = max(maxmtime, st.st_mtime)
+            state.append((st[stat.ST_MTIME], st.st_size))
+            maxmtime = max(maxmtime, st[stat.ST_MTIME])
 
         return tuple(state), maxmtime
 
diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -49,7 +49,7 @@ 
     '''Get "now" timestamp on filesystem'''
     tmpfd, tmpname = vfs.mkstemp()
     try:
-        return os.fstat(tmpfd).st_mtime
+        return os.fstat(tmpfd)[stat.ST_MTIME]
     finally:
         os.close(tmpfd)
         vfs.unlink(tmpname)
@@ -387,7 +387,7 @@ 
     def normal(self, f):
         '''Mark a file normal and clean.'''
         s = os.lstat(self._join(f))
-        mtime = s.st_mtime
+        mtime = s[stat.ST_MTIME]
         self._addpath(f, 'n', s.st_mode,
                       s.st_size & _rangemask, mtime & _rangemask)
         self._map.copymap.pop(f, None)
@@ -626,7 +626,7 @@ 
             self._origpl = None
         # use the modification time of the newly created temporary file as the
         # filesystem's notion of 'now'
-        now = util.fstat(st).st_mtime & _rangemask
+        now = util.fstat(st)[stat.ST_MTIME] & _rangemask
 
         # enough 'delaywrite' prevents 'pack_dirstate' from dropping
         # timestamp of each entries in dirstate, because of 'now > mtime'
@@ -1068,9 +1068,10 @@ 
                     or size == -2 # other parent
                     or fn in copymap):
                     madd(fn)
-                elif time != st.st_mtime and time != st.st_mtime & _rangemask:
+                elif (time != st[stat.ST_MTIME]
+                      and time != st[stat.ST_MTIME] & _rangemask):
                     ladd(fn)
-                elif st.st_mtime == lastnormaltime:
+                elif st[stat.ST_MTIME] == lastnormaltime:
                     # fn may have just been marked as normal and it may have
                     # changed in the same second without changing its size.
                     # This can happen if we quickly do multiple commits.
diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py
--- a/mercurial/debugcommands.py
+++ b/mercurial/debugcommands.py
@@ -16,6 +16,7 @@ 
 import random
 import socket
 import ssl
+import stat
 import string
 import subprocess
 import sys
@@ -1373,9 +1374,9 @@ 
             l.release()
         else:
             try:
-                stat = vfs.lstat(name)
-                age = now - stat.st_mtime
-                user = util.username(stat.st_uid)
+                st = vfs.lstat(name)
+                age = now - st[stat.ST_MTIME]
+                user = util.username(st.st_uid)
                 locker = vfs.readlock(name)
                 if ":" in locker:
                     host, pid = locker.split(':')
diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -1893,7 +1893,7 @@ 
     def date(self):
         t, tz = self._changectx.date()
         try:
-            return (self._repo.wvfs.lstat(self._path).st_mtime, tz)
+            return (self._repo.wvfs.lstat(self._path)[stat.ST_MTIME], tz)
         except OSError as err:
             if err.errno != errno.ENOENT:
                 raise
diff --git a/mercurial/chgserver.py b/mercurial/chgserver.py
--- a/mercurial/chgserver.py
+++ b/mercurial/chgserver.py
@@ -45,6 +45,7 @@ 
 import os
 import re
 import socket
+import stat
 import struct
 import time
 
@@ -161,7 +162,7 @@ 
     def trystat(path):
         try:
             st = os.stat(path)
-            return (st.st_mtime, st.st_size)
+            return (st[stat.ST_MTIME], st.st_size)
         except OSError:
             # could be ENOENT, EPERM etc. not fatal in any case
             pass
@@ -548,7 +549,7 @@ 
         try:
             stat = os.stat(self._realaddress)
             return (stat.st_ino == self._socketstat.st_ino and
-                    stat.st_mtime == self._socketstat.st_mtime)
+                    stat.st_mtime == self._socketstat[stat.ST_MTIME])
         except OSError:
             return False
 
diff --git a/mercurial/cext/osutil.c b/mercurial/cext/osutil.c
--- a/mercurial/cext/osutil.c
+++ b/mercurial/cext/osutil.c
@@ -122,7 +122,7 @@ 
 }
 
 static PyObject *listdir_stat_getitem(PyObject *self, PyObject *key) {
-	long index = PyInt_AsLong(key);
+	long index = PyLong_AsLong(key);
 	if (index == -1 && PyErr_Occurred()) {
 		return NULL;
 	}
diff --git a/hgext/shelve.py b/hgext/shelve.py
--- a/hgext/shelve.py
+++ b/hgext/shelve.py
@@ -25,6 +25,7 @@ 
 import collections
 import errno
 import itertools
+import stat
 
 from mercurial.i18n import _
 from mercurial import (
@@ -283,7 +284,7 @@ 
     maxbackups = repo.ui.configint('shelve', 'maxbackups')
     hgfiles = [f for f in vfs.listdir()
                if f.endswith('.' + patchextension)]
-    hgfiles = sorted([(vfs.stat(f).st_mtime, f) for f in hgfiles])
+    hgfiles = sorted([(vfs.stat(f)[stat.ST_MTIME], f) for f in hgfiles])
     if 0 < maxbackups and maxbackups < len(hgfiles):
         bordermtime = hgfiles[-maxbackups][0]
     else:
@@ -542,7 +543,7 @@ 
         if not pfx or sfx != patchextension:
             continue
         st = shelvedfile(repo, name).stat()
-        info.append((st.st_mtime, shelvedfile(repo, pfx).filename()))
+        info.append((st[stat.ST_MTIME], shelvedfile(repo, pfx).filename()))
     return sorted(info, reverse=True)
 
 def listcmd(ui, repo, pats, opts):
diff --git a/hgext/extdiff.py b/hgext/extdiff.py
--- a/hgext/extdiff.py
+++ b/hgext/extdiff.py
@@ -65,6 +65,7 @@ 
 import os
 import re
 import shutil
+import stat
 import tempfile
 from mercurial.i18n import _
 from mercurial.node import (
@@ -297,7 +298,8 @@ 
             # copyfile() carries over the permission, so the mode check could
             # be in an 'elif' branch, but for the case where the file has
             # changed without affecting mtime or size.
-            if (cpstat.st_mtime != st.st_mtime or cpstat.st_size != st.st_size
+            if (cpstat[stat.ST_MTIME] != st[stat.ST_MTIME]
+                or cpstat.st_size != st.st_size
                 or (cpstat.st_mode & 0o100) != (st.st_mode & 0o100)):
                 ui.debug('file changed while diffing. '
                          'Overwriting: %s (src: %s)\n' % (working_fn, copy_fn))