[Request] [+- ] D11624: dirstate-v2: Change the representation of negative directory mtime

SimonSapin phabricator at mercurial-scm.org
Mon Oct 11 20:32:01 UTC 2021


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

REVISION SUMMARY
  Change it from how I previously thought C’s `timespec` works
  to how it actually works. See code comments.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  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
@@ -185,8 +185,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,
 }
 
@@ -503,15 +502,33 @@
     }
 }
 
+const NSEC_PER_SEC: u32 = 1_000_000_000;
+
 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)`:
+                    (-1 - negative_secs, NSEC_PER_SEC - negative_nanos)
+                }
             }
         };
         Timestamp {
@@ -525,10 +542,17 @@
     fn from(timestamp: &'_ Timestamp) -> Self {
         let secs = timestamp.seconds.get();
         let nanos = timestamp.nanoseconds.get();
-        if secs >= 0 {
+        if secs < 0 {
+            let (s_to_subtract, ns_to_subtract) = if nanos == 0 {
+                (-secs, 0)
+            } else {
+                // Example: `(-5, +0.7)` → `(-4, -0.3)`
+                // See `impl From<SystemTime> for Timestamp` above
+                (1 - secs, NSEC_PER_SEC - nanos)
+            };
+            UNIX_EPOCH - Duration::new(s_to_subtract as u64, ns_to_subtract)
+        } else {
             UNIX_EPOCH + Duration::new(secs as u64, nanos)
-        } else {
-            UNIX_EPOCH - Duration::new((-secs) as u64, nanos)
         }
     }
 }



To: SimonSapin, #hg-reviewers
Cc: mercurial-patches, mercurial-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mercurial-scm.org/pipermail/mercurial-patches/attachments/20211011/53aca363/attachment-0001.html>


More information about the Mercurial-patches mailing list