From patchwork Tue Oct 19 22:04:35 2021
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Subject: D11701: dirstate: ignore sub-second component when either is zero in
 mtime
From: phabricator <phabricator@mercurial-scm.org>
X-Patchwork-Id: 50024
Message-Id: <differential-rev-PHID-DREV-pownsqxvwtv4l3wv5fsn-req@mercurial-scm.org>
To: Phabricator <phabricator@mercurial-scm.org>
Cc: mercurial-devel@mercurial-scm.org
Date: Tue, 19 Oct 2021 22:04:35 +0000

marmoute created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  When comparing mtimes for equality.
  
  Some APIs simply return zero when more precision is not available.
  When comparing values from different sources, if only one is truncated in
  that way, doing a simple comparison would cause many false negatives.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/cext/parsers.c
  mercurial/dirstateutils/timestamp.py
  mercurial/pure/parsers.py
  rust/hg-core/src/dirstate/entry.rs

CHANGE DETAILS




To: marmoute, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

diff --git a/rust/hg-core/src/dirstate/entry.rs b/rust/hg-core/src/dirstate/entry.rs
--- a/rust/hg-core/src/dirstate/entry.rs
+++ b/rust/hg-core/src/dirstate/entry.rs
@@ -120,9 +120,17 @@
     /// If someone is manipulating the modification times of some files to
     /// intentionally make `hg status` return incorrect results, not truncating
     /// wouldn’t help much since they can set exactly the expected timestamp.
+    ///
+    /// Sub-second precision is ignored if it is zero in either value.
+    /// Some APIs simply return zero when more precision is not available.
+    /// When comparing values from different sources, if only one is truncated
+    /// in that way, doing a simple comparison would cause many false
+    /// negatives.
     pub fn likely_equal(self, other: Self) -> bool {
         self.truncated_seconds == other.truncated_seconds
-            && self.nanoseconds == other.nanoseconds
+            && (self.nanoseconds == other.nanoseconds
+                || self.nanoseconds == 0
+                || other.nanoseconds == 0)
     }
 
     pub fn likely_equal_to_mtime_of(
diff --git a/mercurial/pure/parsers.py b/mercurial/pure/parsers.py
--- a/mercurial/pure/parsers.py
+++ b/mercurial/pure/parsers.py
@@ -302,7 +302,9 @@
             return False
         self_ns = self._mtime_ns
         other_sec, other_ns = other_mtime
-        return self_sec == other_sec and self_ns == other_ns
+        return self_sec == other_sec and (
+            self_ns == other_ns or self_ns == 0 or other_ns == 0
+        )
 
     @property
     def state(self):
diff --git a/mercurial/dirstateutils/timestamp.py b/mercurial/dirstateutils/timestamp.py
--- a/mercurial/dirstateutils/timestamp.py
+++ b/mercurial/dirstateutils/timestamp.py
@@ -5,15 +5,17 @@
 
 from __future__ import absolute_import
 
+import functools
 import stat
 
 
 rangemask = 0x7FFFFFFF
 
 
+@functools.total_ordering
 class timestamp(tuple):
     """
-    A Unix timestamp with nanoseconds precision,
+    A Unix timestamp with optional nanoseconds precision,
     modulo 2**31 seconds.
 
     A 2-tuple containing:
@@ -22,6 +24,7 @@
     truncated to its lower 31 bits
 
     `subsecond_nanoseconds`: number of nanoseconds since `truncated_seconds`.
+    When this is zero, the sub-second precision is considered unknown.
     """
 
     def __new__(cls, value):
@@ -29,6 +32,27 @@
         value = (truncated_seconds & rangemask, subsec_nanos)
         return super(timestamp, cls).__new__(cls, value)
 
+    def __eq__(self, other):
+        self_secs, self_subsec_nanos = self
+        other_secs, other_subsec_nanos = other
+        return self_secs == other_secs and (
+            self_subsec_nanos == other_subsec_nanos
+            or self_subsec_nanos == 0
+            or other_subsec_nanos == 0
+        )
+
+    def __gt__(self, other):
+        self_secs, self_subsec_nanos = self
+        other_secs, other_subsec_nanos = other
+        if self_secs > other_secs:
+            return True
+        if self_secs < other_secs:
+            return False
+        if self_subsec_nanos == 0 or other_subsec_nanos == 0:
+            # they are considered equal, so not "greater than"
+            return False
+        return self_subsec_nanos > other_subsec_nanos
+
 
 def zero():
     """
diff --git a/mercurial/cext/parsers.c b/mercurial/cext/parsers.c
--- a/mercurial/cext/parsers.c
+++ b/mercurial/cext/parsers.c
@@ -319,7 +319,9 @@
 		return NULL;
 	}
 	if ((self->flags & dirstate_flag_has_file_mtime) &&
-	    self->mtime_s == other_s && self->mtime_ns == other_ns) {
+	    self->mtime_s == other_s &&
+	    (self->mtime_ns == other_ns || self->mtime_ns == 0 ||
+	     other_ns == 0)) {
 		Py_RETURN_TRUE;
 	} else {
 		Py_RETURN_FALSE;