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. 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. REPOSITORY rHG Mercurial BRANCH default 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, } @@ -444,15 +443,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 { @@ -466,10 +483,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) } } } 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, mercurial-devel _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
