rhg: Update the dirstate on disk after status

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

mercurial/dirstateutils/timestamp.py

@@ -96,7 +96,7 b' def reliable_mtime_of(stat_result, prese'
     """same as `mtime_of`, but return None if the date might be ambiguous
 
     A modification time is reliable if it is older than "present_time" (or
-    sufficiently in the futur).
+    sufficiently in the future).
 
     Otherwise a concurrent modification might happens with the same mtime.
     """

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

@@ -102,6 +102,35 b' impl TruncatedTimestamp {'
         }
     }
 
+    /// Returns whether this timestamp is reliable as the "mtime" of a file.
+    ///
+    /// 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 {
+        // 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
+        // and we cannot cache the information.
+        //
+        // However if the mtime is far away in the future, this is likely some
+        // 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
+                && boundary.nanoseconds != 0
+                && self.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
+        }
+    }
+
     /// The lower 31 bits of the number of seconds since the epoch.
     pub fn truncated_seconds(&self) -> u32 {
         self.truncated_seconds
@@ -191,7 +220,7 b' impl From<SystemTime> for TruncatedTimes'
 }
 
 const NSEC_PER_SEC: u32 = 1_000_000_000;
-const RANGE_MASK_31BIT: u32 = 0x7FFF_FFFF;
+pub const RANGE_MASK_31BIT: u32 = 0x7FFF_FFFF;
 
 pub const MTIME_UNSET: i32 = -1;
 

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

@@ -73,6 +73,10 b' pub struct StatusOptions {'
 
 #[derive(Debug, Default)]
 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>,
+
     /// Tracked files whose contents have changed since the parent revision
     pub modified: Vec<HgPathCow<'a>>,
 

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

@@ -61,16 +61,21 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 outcome = DirstateStatus {
+        filesystem_time_at_status_start,
+        ..Default::default()
+    };
     let common = StatusCommon {
         dmap,
         options,
         matcher,
         ignore_fn,
-        outcome: Default::default(),
+        outcome: Mutex::new(outcome),
         ignore_patterns_have_changed: patterns_changed,
         new_cachable_directories: Default::default(),
         outated_cached_directories: Default::default(),
-        filesystem_time_at_status_start: filesystem_now(&root_dir).ok(),
+        filesystem_time_at_status_start,
     };
     let is_at_repo_root = true;
     let hg_path = &BorrowedPath::OnDisk(HgPath::new(""));

rust/rhg/src/commands/status.rs

@@ -13,14 +13,18 b' use format_bytes::format_bytes;'
 use hg;
 use hg::config::Config;
 use hg::dirstate::has_exec_bit;
-use hg::errors::HgError;
+use hg::dirstate::TruncatedTimestamp;
+use hg::dirstate::RANGE_MASK_31BIT;
+use hg::errors::{HgError, IoResultExt};
+use hg::lock::LockError;
 use hg::manifest::Manifest;
 use hg::matchers::AlwaysMatcher;
 use hg::repo::Repo;
 use hg::utils::files::get_bytes_from_os_string;
-use hg::utils::hg_path::{hg_path_to_os_string, HgPath};
+use hg::utils::hg_path::{hg_path_to_path_buf, HgPath};
 use hg::{HgPathCow, StatusOptions};
 use log::{info, warn};
+use std::io;
 
 pub const HELP_TEXT: &str = "
 Show changed files in the working directory
@@ -229,6 +233,7 b' pub fn run(invocation: &crate::CliInvoca'
             &ds_status.unsure
         );
     }
+    let mut fixup = Vec::new();
     if !ds_status.unsure.is_empty()
         && (display_states.modified || display_states.clean)
     {
@@ -243,8 +248,9 b' pub fn run(invocation: &crate::CliInvoca'
                 }
             } else {
                 if display_states.clean {
-                    ds_status.clean.push(to_check);
+                    ds_status.clean.push(to_check.clone());
                 }
+                fixup.push(to_check.into_owned())
             }
         }
     }
@@ -318,6 +324,71 b' pub fn run(invocation: &crate::CliInvoca'
             b"C",
         )?;
     }
+
+    let mut dirstate_write_needed = ds_status.dirty;
+    let filesystem_time_at_status_start = ds_status
+        .filesystem_time_at_status_start
+        .map(TruncatedTimestamp::from);
+
+    if (fixup.is_empty() || filesystem_time_at_status_start.is_none())
+        && !dirstate_write_needed
+    {
+        // Nothing to update
+        return Ok(());
+    }
+
+    // Update the dirstate on disk if we can
+    let with_lock_result =
+        repo.try_with_wlock_no_wait(|| -> Result<(), CommandError> {
+            if let Some(mtime_boundary) = filesystem_time_at_status_start {
+                for hg_path in fixup {
+                    use std::os::unix::fs::MetadataExt;
+                    let fs_path = hg_path_to_path_buf(&hg_path)
+                        .expect("HgPath conversion");
+                    // Specifically do not reuse `fs_metadata` from
+                    // `unsure_is_clean` which was needed before reading
+                    // contents. Here we access metadata again after reading
+                    // content, in case it changed in the meantime.
+                    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) {
+                        let mode = fs_metadata.mode();
+                        let size = fs_metadata.len() as u32 & RANGE_MASK_31BIT;
+                        let mut entry = dmap
+                            .get(&hg_path)?
+                            .expect("ambiguous file not in dirstate");
+                        entry.set_clean(mode, size, mtime);
+                        dmap.add_file(&hg_path, entry)?;
+                        dirstate_write_needed = true
+                    }
+                }
+            }
+            drop(dmap); // Avoid "already mutably borrowed" RefCell panics
+            if dirstate_write_needed {
+                repo.write_dirstate()?
+            }
+            Ok(())
+        });
+    match with_lock_result {
+        Ok(closure_result) => closure_result?,
+        Err(LockError::AlreadyHeld) => {
+            // Not updating the dirstate is not ideal but not critical:
+            // don’t keep our caller waiting until some other Mercurial
+            // process releases the lock.
+        }
+        Err(LockError::Other(HgError::IoError { error, .. }))
+            if error.kind() == io::ErrorKind::PermissionDenied =>
+        {
+            // `hg status` on a read-only repository is fine
+        }
+        Err(LockError::Other(error)) => {
+            // Report other I/O errors
+            Err(error)?
+        }
+    }
     Ok(())
 }
 
@@ -368,7 +439,7 b' fn unsure_is_modified('
     hg_path: &HgPath,
 ) -> Result<bool, HgError> {
     let vfs = repo.working_directory_vfs();
-    let fs_path = hg_path_to_os_string(hg_path).expect("HgPath conversion");
+    let fs_path = hg_path_to_path_buf(hg_path).expect("HgPath conversion");
     let fs_metadata = vfs.symlink_metadata(&fs_path)?;
     let is_symlink = fs_metadata.file_type().is_symlink();
     // TODO: Also account for `FALLBACK_SYMLINK` and `FALLBACK_EXEC` from the
@@ -399,5 +470,5 b' fn unsure_is_modified('
     } else {
         vfs.read(fs_path)?
     };
-    return Ok(contents_in_p1 != &*fs_contents);
+    Ok(contents_in_p1 != &*fs_contents)
 }

tests/test-backout.t

@@ -172,8 +172,7 b' transaction: in-memory dirstate changes '
   $ hg status -A
   C c
   $ hg debugstate --no-dates
-  n 644         12 set                 c (no-rhg !)
-  n   0         -1 unset               c (rhg known-bad-output !)
+  n 644         12 set                 c
   $ hg backout -d '6 0' -m 'to be rollback-ed soon' -r .
   removing c
   adding b

tests/test-dirstate-race2.t

@@ -9,9 +9,6 b''
   > EOF
 #endif
 
-TODO: fix rhg bugs that make this test fail when status is enabled
-  $ unset RHG_STATUS
-
 Checking the size/permissions/file-type of files stored in the
 dirstate after an update where the files are changed concurrently
 outside of hg's control.

tests/test-dirstate.t

@@ -9,10 +9,6 b''
   > EOF
 #endif
 
-TODO: fix rhg bugs that make this test fail when status is enabled
-  $ unset RHG_STATUS
-
-
 ------ Test dirstate._dirs refcounting
 
   $ hg init t