dirstate-v2: Apply SECOND_AMBIGUOUS to directory mtimes too

This would only be relevant in contrived scenarios such as a dirstate file
being written with a libc that supports sub-second precision in mtimes,
then transfered (at the filesystem level, not `hg clone`) to another
system where libc *doesn’t* have sub-second precision and truncates the stored
mtime of a directory to integer seconds.

Differential Revision: https://phab.mercurial-scm.org/D11939

rust/hg-core/src/dirstate/status.rs

@@ -9,8 +9,8 b''
 //! It is currently missing a lot of functionality compared to the Python one
 //! and will only be triggered in narrow cases.
 
+use crate::dirstate::entry::TruncatedTimestamp;
 use crate::dirstate_tree::on_disk::DirstateV2ParseError;
-
 use crate::{
     utils::hg_path::{HgPath, HgPathError},
     PatternError,
@@ -77,7 +77,7 b' pub struct StatusOptions {'
 pub struct DirstateStatus<'a> {
     /// The current time at the start of the `status()` algorithm, as measured
     /// and possibly truncated by the filesystem.
-    pub filesystem_time_at_status_start: Option<std::time::SystemTime>,
+    pub filesystem_time_at_status_start: Option<TruncatedTimestamp>,
 
     /// Tracked files whose contents have changed since the parent revision
     pub modified: Vec<StatusPath<'a>>,

rust/hg-core/src/dirstate_tree/on_disk.rs

@@ -382,7 +382,7 b' impl Node {'
             && self.flags().contains(Flags::HAS_MTIME)
             && self.flags().contains(Flags::ALL_UNKNOWN_RECORDED)
         {
-            Ok(Some(self.mtime.try_into()?))
+            Ok(Some(self.mtime()?))
         } else {
             Ok(None)
         }
@@ -402,6 +402,14 b' impl Node {'
         file_type | permisions
     }
 
+    fn mtime(&self) -> Result<TruncatedTimestamp, DirstateV2ParseError> {
+        let mut m: TruncatedTimestamp = self.mtime.try_into()?;
+        if self.flags().contains(Flags::MTIME_SECOND_AMBIGUOUS) {
+            m.second_ambiguous = true;
+        }
+        Ok(m)
+    }
+
     fn assume_entry(&self) -> Result<DirstateEntry, DirstateV2ParseError> {
         // TODO: convert through raw bits instead?
         let wdir_tracked = self.flags().contains(Flags::WDIR_TRACKED);
@@ -418,11 +426,7 b' impl Node {'
             && !self.flags().contains(Flags::DIRECTORY)
             && !self.flags().contains(Flags::EXPECTED_STATE_IS_MODIFIED)
         {
-            let mut m: TruncatedTimestamp = self.mtime.try_into()?;
-            if self.flags().contains(Flags::MTIME_SECOND_AMBIGUOUS) {
-                m.second_ambiguous = true;
-            }
-            Some(m)
+            Some(self.mtime()?)
         } else {
             None
         };
@@ -681,7 +685,7 b" impl Writer<'_, '_> {"
                         dirstate_map::NodeData::Entry(entry) => {
                             Node::from_dirstate_entry(entry)
                         }
-                        dirstate_map::NodeData::CachedDirectory { mtime } => (
+                        dirstate_map::NodeData::CachedDirectory { mtime } => {
                             // we currently never set a mtime if unknown file
                             // are present.
                             // So if we have a mtime for a directory, we know
@@ -692,12 +696,14 b" impl Writer<'_, '_> {"
                             // We never set ALL_IGNORED_RECORDED since we
                             // don't track that case
                             // currently.
-                            Flags::DIRECTORY
+                            let mut flags = Flags::DIRECTORY
                                 | Flags::HAS_MTIME
-                                | Flags::ALL_UNKNOWN_RECORDED,
-                            0.into(),
-                            (*mtime).into(),
-                        ),
+                                | Flags::ALL_UNKNOWN_RECORDED;
+                            if mtime.second_ambiguous {
+                                flags.insert(Flags::MTIME_SECOND_AMBIGUOUS)
+                            }
+                            (flags, 0.into(), (*mtime).into())
+                        }
                         dirstate_map::NodeData::None => (
                             Flags::DIRECTORY,
                             0.into(),

rust/hg-core/src/dirstate_tree/status.rs

@@ -63,7 +63,8 b" pub fn status<'tree, 'on_disk: 'tree>("
             (Box::new(|&_| true), vec![], None)
         };
 
-    let filesystem_time_at_status_start = filesystem_now(&root_dir).ok();
+    let filesystem_time_at_status_start =
+        filesystem_now(&root_dir).ok().map(TruncatedTimestamp::from);
     let outcome = DirstateStatus {
         filesystem_time_at_status_start,
         ..Default::default()
@@ -145,7 +146,7 b" struct StatusCommon<'a, 'tree, 'on_disk:"
 
     /// The current time at the start of the `status()` algorithm, as measured
     /// and possibly truncated by the filesystem.
-    filesystem_time_at_status_start: Option<SystemTime>,
+    filesystem_time_at_status_start: Option<TruncatedTimestamp>,
 }
 
 enum Outcome {
@@ -472,25 +473,39 b" impl<'a, 'tree, 'on_disk> StatusCommon<'"
         directory_metadata: &std::fs::Metadata,
         dirstate_node: NodeRef<'tree, 'on_disk>,
     ) -> Result<(), DirstateV2ParseError> {
-        if children_all_have_dirstate_node_or_are_ignored {
+        if !children_all_have_dirstate_node_or_are_ignored {
+            return Ok(());
+        }
             // All filesystem directory entries from `read_dir` have a
             // corresponding node in the dirstate, so we can reconstitute the
             // names of those entries without calling `read_dir` again.
-            if let (Some(status_start), Ok(directory_mtime)) = (
-                &self.filesystem_time_at_status_start,
-                directory_metadata.modified(),
-            ) {
+
+        // TODO: use let-else here and below when available:
+        // https://github.com/rust-lang/rust/issues/87335
+        let status_start = if let Some(status_start) =
+            &self.filesystem_time_at_status_start
+        {
+            status_start
+        } else {
+            return Ok(());
+        };
+
                 // Although the Rust standard library’s `SystemTime` type
                 // has nanosecond precision, the times reported for a
                 // directory’s (or file’s) modified time may have lower
                 // resolution based on the filesystem (for example ext3
                 // only stores integer seconds), kernel (see
                 // https://stackoverflow.com/a/14393315/1162888), etc.
-                if &directory_mtime >= status_start {
-                    // The directory was modified too recently, don’t cache its
-                    // `read_dir` results.
-                    //
-                    // A timeline like this is possible:
+        let directory_mtime = if let Ok(option) =
+            TruncatedTimestamp::for_reliable_mtime_of(
+                directory_metadata,
+                status_start,
+            ) {
+            if let Some(directory_mtime) = option {
+                directory_mtime
+            } else {
+                // The directory was modified too recently,
+                // don’t cache its `read_dir` results.
                     //
                     // 1. A change to this directory (direct child was
                     //    added or removed) cause its mtime to be set
@@ -504,7 +519,12 b" impl<'a, 'tree, 'on_disk> StatusCommon<'"
                     // On a system where the time resolution poor, this
                     // scenario is not unlikely if all three steps are caused
                     // by the same script.
+                return Ok(());
+            }
                 } else {
+            // OS/libc does not support mtime?
+            return Ok(());
+        };
                     // We’ve observed (through `status_start`) that time has
                     // “progressed” since `directory_mtime`, so any further
                     // change to this directory is extremely likely to cause a
@@ -518,11 +538,10 b" impl<'a, 'tree, 'on_disk> StatusCommon<'"
                     //
                     // We deem this scenario (unlike the previous one) to be
                     // unlikely enough in practice.
-                    let truncated = TruncatedTimestamp::from(directory_mtime);
-                    let is_up_to_date = if let Some(cached) =
-                        dirstate_node.cached_directory_mtime()?
-                    {
-                        cached.likely_equal(truncated)
+
+        let is_up_to_date =
+            if let Some(cached) = dirstate_node.cached_directory_mtime()? {
+                cached.likely_equal(directory_mtime)
                     } else {
                         false
                     };
@@ -533,10 +552,7 b" impl<'a, 'tree, 'on_disk> StatusCommon<'"
                         self.new_cachable_directories
                             .lock()
                             .unwrap()
-                            .push((hg_path, truncated))
-                    }
-                }
-            }
+                .push((hg_path, directory_mtime))
         }
         Ok(())
     }

rust/rhg/src/commands/status.rs

@@ -315,9 +315,8 b' pub fn run(invocation: &crate::CliInvoca'
     }
 
     let mut dirstate_write_needed = ds_status.dirty;
-    let filesystem_time_at_status_start = ds_status
-        .filesystem_time_at_status_start
-        .map(TruncatedTimestamp::from);
+    let filesystem_time_at_status_start =
+        ds_status.filesystem_time_at_status_start;
 
     if (fixup.is_empty() || filesystem_time_at_status_start.is_none())
         && !dirstate_write_needed