[Updated] D11629: dirstate-v2: Change the representation of negative directory mtime

[SimonSapin]

SimonSapin updated this revision to Diff 30719.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D11629?vs=30714&id=30719

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D11629/new/

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

AFFECTED FILES
  mercurial/helptext/internals/dirstate-v2.txt
  rust/hg-core/src/dirstate_tree/on_disk.rs

CHANGE DETAILS

diff --git a/rust/hg-core/src/dirstate_tree/on_disk.rs b/rust/hg-core/src/dirstate_tree/on_disk.rs
--- a/rust/hg-core/src/dirstate_tree/on_disk.rs
+++ b/rust/hg-core/src/dirstate_tree/on_disk.rs
@@ -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 {
diff --git a/mercurial/helptext/internals/dirstate-v2.txt b/mercurial/helptext/internals/dirstate-v2.txt
--- a/mercurial/helptext/internals/dirstate-v2.txt
+++ b/mercurial/helptext/internals/dirstate-v2.txt
@@ -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:



To: SimonSapin, #hg-reviewers
Cc: mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20211012/9c6d4a35/attachment-0002.html>