[Request] [+- ] D11889: rhg: Set second_ambiguous as needed in post-status fixup

[SimonSapin]

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

REVISION SUMMARY
  This fixes an intermittent bug that manifested only in test-revert.t,
  and unfortunately not on CI. On a fast enough machine we could have:
  
  1. A file is modified
  2. `rhg status` writes an updated dirstate-v1
  3. The same file is modified again
  
  … all within the same integer second. Because the dirstate-v1 file format
  does not store sub-second precision, step 2 must write the file’s mtime
  as "unknown" because of the possibility of step 3.
  
  However, most of the code now handles timestamps with nanosecond precision
  in order to take advantage of it in dirstate-v2. `second_ambiguous` must
  be set for timestamps that become ambiguous if sub-second precision is dropped
  (such as through serialization in dirstate-v1Â format).

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  rust/hg-core/src/dirstate/entry.rs
  rust/rhg/src/commands/status.rs

CHANGE DETAILS

diff --git a/rust/rhg/src/commands/status.rs b/rust/rhg/src/commands/status.rs
--- a/rust/rhg/src/commands/status.rs
+++ b/rust/rhg/src/commands/status.rs
@@ -352,9 +352,13 @@
                     let fs_metadata = repo
                         .working_directory_vfs()
                         .symlink_metadata(&fs_path)?;
-                    let mtime = TruncatedTimestamp::for_mtime_of(&fs_metadata)
-                        .when_reading_file(&fs_path)?;
-                    if mtime.is_reliable_mtime(&mtime_boundary) {
+                    if let Some(mtime) =
+                        TruncatedTimestamp::for_reliable_mtime_of(
+                            &fs_metadata,
+                            &mtime_boundary,
+                        )
+                        .when_reading_file(&fs_path)?
+                    {
                         let mode = fs_metadata.mode();
                         let size = fs_metadata.len() as u32 & RANGE_MASK_31BIT;
                         let mut entry = dmap
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
@@ -87,6 +87,10 @@
         }
     }
 
+    /// Returns a `TruncatedTimestamp` for the modification time of `metadata`.
+    ///
+    /// Propagates errors from `std` on platforms where modification time
+    /// is not available at all.
     pub fn for_mtime_of(metadata: &fs::Metadata) -> io::Result<Self> {
         #[cfg(unix)]
         {
@@ -102,13 +106,18 @@
         }
     }
 
-    /// Returns whether this timestamp is reliable as the "mtime" of a file.
+    /// Like `for_mtime_of`, but may return `None` or a value with
+    /// `second_ambiguous` set if the mtime is not "reliable".
     ///
     /// A modification time is reliable if it is older than `boundary` (or
     /// sufficiently in the future).
     ///
     /// Otherwise a concurrent modification might happens with the same mtime.
-    pub fn is_reliable_mtime(&self, boundary: &Self) -> bool {
+    pub fn for_reliable_mtime_of(
+        metadata: &fs::Metadata,
+        boundary: &Self,
+    ) -> io::Result<Option<Self>> {
+        let mut mtime = Self::for_mtime_of(metadata)?;
         // If the mtime of the ambiguous file is younger (or equal) to the
         // starting point of the `status` walk, we cannot garantee that
         // another, racy, write will not happen right after with the same mtime
@@ -118,16 +127,23 @@
         // mismatch between the current clock and previous file system
         // operation. So mtime more than one days in the future are considered
         // fine.
-        if self.truncated_seconds == boundary.truncated_seconds {
-            self.nanoseconds != 0
+        let reliable = if mtime.truncated_seconds == boundary.truncated_seconds
+        {
+            mtime.second_ambiguous = true;
+            mtime.nanoseconds != 0
                 && boundary.nanoseconds != 0
-                && self.nanoseconds < boundary.nanoseconds
+                && mtime.nanoseconds < boundary.nanoseconds
         } else {
             // `truncated_seconds` is less than 2**31,
             // so this does not overflow `u32`:
             let one_day_later = boundary.truncated_seconds + 24 * 3600;
-            self.truncated_seconds < boundary.truncated_seconds
-                || self.truncated_seconds > one_day_later
+            mtime.truncated_seconds < boundary.truncated_seconds
+                || mtime.truncated_seconds > one_day_later
+        };
+        if reliable {
+            Ok(Some(mtime))
+        } else {
+            Ok(None)
         }
     }
 



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/20211209/a88dbee3/attachment-0001.html>