parsers: make pack_dirstate take now in integer for consistency
authorFUJIWARA Katsunori <foozy@lares.dti.ne.jp>
Wed, 14 Oct 2015 02:40:04 +0900
changeset 26630 3111b45a2bbf
parent 26629 ae5f7be2b4ab
child 26631 e077ce385609
parsers: make pack_dirstate take now in integer for consistency On recent OS, 'stat.st_mtime' has a double precision floating point value to represent nano seconds, but it is not wide enough for actual file timestamp: nowadays, only 52 - 32 = 20 bit width is available for decimal places in sec. Therefore, casting it to 'int' may cause unexpected result. See also changeset 13272104bb07 fixing issue4836 for detail. For example, changed file A may be treated as "clean" unexpectedly in steps below. "rounded now" is the value gotten by rounding via 'int(st.st_mtime)' or so. ---------------------+--------------------+------------------------ "now" | | timestamp of A (time_t) float rounded time_t| action | FS dirstate ------ ------- ------+--------------------+-------- --------------- N+.nnn N N | | --- --- | update file A | N | dirstate.normal(A) | N N+.999 N+1 N | | | dirstate.write() | N (*1) | : | | change file A | N | : | N+1.00 N+1 N+1 | | | "hg status" (*2) | N N ------ ------- ------+--------------------+-------- --------------- Timestamp N of A in dirstate isn't dropped at (*1), because "rounded now" is N+1 at that time, even if 'st_mtime' in 'time_t' is still N. Then, file A is unexpectedly treated as "clean" at (*2) in this case. For consistent handling of 'stat.st_mtime', this patch makes 'pack_dirstate()' take 'now' argument not in floating point but in integer. This patch makes 'PyArg_ParseTuple()' in 'pack_dirstate()' use format 'i' (= checking type mismatch or overflow), even though it is ensured that 'now' is in the range of 32bit signed integer by masking with '_rangemask' (= 0x7fffffff) on caller side. It should be cheaper enough than packing itself, and useful to detect that legacy code invokes 'pack_dirstate()' with 'now' in floating point value.
mercurial/dirstate.py
mercurial/parsers.c
tests/fakedirstatewritetime.py
--- a/mercurial/dirstate.py	Tue Sep 29 00:18:49 2015 -0700
+++ b/mercurial/dirstate.py	Wed Oct 14 02:40:04 2015 +0900
@@ -627,7 +627,7 @@
     def _writedirstate(self, st):
         # use the modification time of the newly created temporary file as the
         # filesystem's notion of 'now'
-        now = util.fstat(st).st_mtime
+        now = util.statmtimesec(util.fstat(st)) & _rangemask
         st.write(parsers.pack_dirstate(self._map, self._copymap, self._pl, now))
         st.close()
         self._lastnormaltime = 0
--- a/mercurial/parsers.c	Tue Sep 29 00:18:49 2015 -0700
+++ b/mercurial/parsers.c	Wed Oct 14 02:40:04 2015 +0900
@@ -551,9 +551,9 @@
 	Py_ssize_t nbytes, pos, l;
 	PyObject *k, *v = NULL, *pn;
 	char *p, *s;
-	double now;
+	int now;
 
-	if (!PyArg_ParseTuple(args, "O!O!Od:pack_dirstate",
+	if (!PyArg_ParseTuple(args, "O!O!Oi:pack_dirstate",
 			      &PyDict_Type, &map, &PyDict_Type, &copymap,
 			      &pl, &now))
 		return NULL;
@@ -622,7 +622,7 @@
 		mode = tuple->mode;
 		size = tuple->size;
 		mtime = tuple->mtime;
-		if (state == 'n' && mtime == (uint32_t)now) {
+		if (state == 'n' && mtime == now) {
 			/* See pure/parsers.py:pack_dirstate for why we do
 			 * this. */
 			mtime = -1;
--- a/tests/fakedirstatewritetime.py	Tue Sep 29 00:18:49 2015 -0700
+++ b/tests/fakedirstatewritetime.py	Wed Oct 14 02:40:04 2015 +0900
@@ -31,8 +31,7 @@
 
     # parsing 'fakenow' in YYYYmmddHHMM format makes comparison between
     # 'fakenow' value and 'touch -t YYYYmmddHHMM' argument easy
-    timestamp = util.parsedate(fakenow, ['%Y%m%d%H%M'])[0]
-    fakenow = float(timestamp)
+    fakenow = util.parsedate(fakenow, ['%Y%m%d%H%M'])[0]
 
     orig_pack_dirstate = parsers.pack_dirstate
     wrapper = lambda *args: pack_dirstate(fakenow, orig_pack_dirstate, *args)