diff --git a/mercurial/dirstateutils/timestamp.py b/mercurial/dirstateutils/timestamp.py --- a/mercurial/dirstateutils/timestamp.py +++ b/mercurial/dirstateutils/timestamp.py @@ -96,7 +96,7 @@ 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. """ 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 @@ -102,6 +102,35 @@ 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 @@ impl From 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; diff --git a/rust/hg-core/src/dirstate/status.rs b/rust/hg-core/src/dirstate/status.rs --- a/rust/hg-core/src/dirstate/status.rs +++ b/rust/hg-core/src/dirstate/status.rs @@ -73,6 +73,10 @@ 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, + /// Tracked files whose contents have changed since the parent revision pub modified: Vec>, diff --git a/rust/hg-core/src/dirstate_tree/status.rs b/rust/hg-core/src/dirstate_tree/status.rs --- a/rust/hg-core/src/dirstate_tree/status.rs +++ b/rust/hg-core/src/dirstate_tree/status.rs @@ -61,16 +61,21 @@ 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("")); 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 @@ -13,14 +13,18 @@ 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 @@ 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 @@ 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 @@ 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 @@ fn unsure_is_modified( hg_path: &HgPath, ) -> Result { 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 @@ fn unsure_is_modified( } else { vfs.read(fs_path)? }; - return Ok(contents_in_p1 != &*fs_contents); + Ok(contents_in_p1 != &*fs_contents) } diff --git a/tests/test-backout.t b/tests/test-backout.t --- a/tests/test-backout.t +++ b/tests/test-backout.t @@ -172,8 +172,7 @@ 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 diff --git a/tests/test-dirstate-race2.t b/tests/test-dirstate-race2.t --- a/tests/test-dirstate-race2.t +++ b/tests/test-dirstate-race2.t @@ -9,9 +9,6 @@ > 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. diff --git a/tests/test-dirstate.t b/tests/test-dirstate.t --- a/tests/test-dirstate.t +++ b/tests/test-dirstate.t @@ -9,10 +9,6 @@ > 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