changeset 48203:a5a673ec8f6f

dirstate-v2: Change the representation of negative directory mtime Change it from how I previously thought C’s `timespec` works to how it actually works. The previous behavior was also buggy for timestamps strictly before the epoch but less than one second away from it, because two’s complement does not distinguish negative zero from positive zero. Differential Revision: https://phab.mercurial-scm.org/D11629
author Simon Sapin <simon.sapin@octobus.net>
date Mon, 11 Oct 2021 22:19:42 +0200
parents 0cc0c0972164
children d2f760c2c91c
files mercurial/helptext/internals/dirstate-v2.txt rust/hg-core/src/dirstate_tree/on_disk.rs
diffstat 2 files changed, 26 insertions(+), 11 deletions(-) [+]
line wrap: on
line diff
--- a/mercurial/helptext/internals/dirstate-v2.txt	Tue Oct 12 15:29:05 2021 +0200
+++ b/mercurial/helptext/internals/dirstate-v2.txt	Mon Oct 11 22:19:42 2021 +0200
@@ -443,20 +443,19 @@
 
   If an untracked node `HAS_MTIME` *set*,
   what follows is the modification time of a directory
-  represented with separated second and sub-second components
-  since the Unix epoch:
+  represented similarly to the C `timespec` struct:
 
   * Offset 31:
-    The number of seconds as a signed (two’s complement) 64-bit integer.
+    The number of seconds elapsed since the Unix epoch,
+    as a signed (two’s complement) 64-bit integer.
 
   * Offset 39:
-    The number of nanoseconds as 32-bit integer.
+    The number of nanoseconds elapsed since
+    the instant specified by the previous field alone,
+    as 32-bit integer.
     Always greater than or equal to zero, and strictly less than a billion.
     Increasing this component makes the modification time
-    go forward or backward in time dependening
-    on the sign of the integral seconds components.
-    (Note: this is buggy because there is no negative zero integer,
-    but will be changed soon.)
+    go forward in time regardless of the sign of the seconds component.
 
   The presence of a directory modification time means that at some point,
   this path in the working directory was observed:
--- a/rust/hg-core/src/dirstate_tree/on_disk.rs	Tue Oct 12 15:29:05 2021 +0200
+++ b/rust/hg-core/src/dirstate_tree/on_disk.rs	Mon Oct 11 22:19:42 2021 +0200
@@ -126,8 +126,7 @@
 
     /// In `0 .. 1_000_000_000`.
     ///
-    /// This timestamp is later or earlier than `(seconds, 0)` by this many
-    /// nanoseconds, if `seconds` is non-negative or negative, respectively.
+    /// This timestamp is after `(seconds, 0)` by this many nanoseconds.
     nanoseconds: U32Be,
 }
 
@@ -446,13 +445,30 @@
 
 impl From<SystemTime> for Timestamp {
     fn from(system_time: SystemTime) -> Self {
+        // On Unix, `SystemTime` is a wrapper for the `timespec` C struct:
+        // https://www.gnu.org/software/libc/manual/html_node/Time-Types.html#index-struct-timespec
+        // We want to effectively access its fields, but the Rust standard
+        // library does not expose them. The best we can do is:
         let (secs, nanos) = match system_time.duration_since(UNIX_EPOCH) {
             Ok(duration) => {
                 (duration.as_secs() as i64, duration.subsec_nanos())
             }
             Err(error) => {
+                // `system_time` is before `UNIX_EPOCH`.
+                // We need to undo this algorithm:
+                // https://github.com/rust-lang/rust/blob/6bed1f0bc3cc50c10aab26d5f94b16a00776b8a5/library/std/src/sys/unix/time.rs#L40-L41
                 let negative = error.duration();
-                (-(negative.as_secs() as i64), negative.subsec_nanos())
+                let negative_secs = negative.as_secs() as i64;
+                let negative_nanos = negative.subsec_nanos();
+                if negative_nanos == 0 {
+                    (-negative_secs, 0)
+                } else {
+                    // For example if `system_time` was 4.3 seconds before
+                    // the Unix epoch we get a Duration that represents
+                    // `(-4, -0.3)` but we want `(-5, +0.7)`:
+                    const NSEC_PER_SEC: u32 = 1_000_000_000;
+                    (-1 - negative_secs, NSEC_PER_SEC - negative_nanos)
+                }
             }
         };
         Timestamp {