--- a/rust/rhg/src/commands/status.rs Thu Mar 02 04:16:47 2023 +0100
+++ b/rust/rhg/src/commands/status.rs Thu Mar 02 19:02:52 2023 +0100
@@ -21,6 +21,7 @@
use hg::manifest::Manifest;
use hg::matchers::{AlwaysMatcher, IntersectionMatcher};
use hg::repo::Repo;
+use hg::utils::debug::debug_wait_for_file;
use hg::utils::files::get_bytes_from_os_string;
use hg::utils::files::get_path_from_bytes;
use hg::utils::hg_path::{hg_path_to_path_buf, HgPath};
@@ -308,26 +309,46 @@
.unsure
.into_par_iter()
.map(|to_check| {
- unsure_is_modified(
+ // The compiler seems to get a bit confused with complex
+ // inference when using a parallel iterator + map
+ // + map_err + collect, so let's just inline some of the
+ // logic.
+ match unsure_is_modified(
working_directory_vfs,
store_vfs,
check_exec,
&manifest,
&to_check.path,
- )
- .map(|modified| (to_check, modified))
+ ) {
+ Err(HgError::IoError { .. }) => {
+ // IO errors most likely stem from the file being
+ // deleted even though we know it's in the
+ // dirstate.
+ Ok((to_check, UnsureOutcome::Deleted))
+ }
+ Ok(outcome) => Ok((to_check, outcome)),
+ Err(e) => Err(e),
+ }
})
.collect::<Result<_, _>>()?;
- for (status_path, is_modified) in res.into_iter() {
- if is_modified {
- if display_states.modified {
- ds_status.modified.push(status_path);
+ for (status_path, outcome) in res.into_iter() {
+ match outcome {
+ UnsureOutcome::Clean => {
+ if display_states.clean {
+ ds_status.clean.push(status_path.clone());
+ }
+ fixup.push(status_path.path.into_owned())
}
- } else {
- if display_states.clean {
- ds_status.clean.push(status_path.clone());
+ UnsureOutcome::Modified => {
+ if display_states.modified {
+ ds_status.modified.push(status_path);
+ }
}
- fixup.push(status_path.path.into_owned())
+ UnsureOutcome::Deleted => {
+ if display_states.deleted {
+ ds_status.deleted.push(status_path);
+ }
+ }
}
}
}
@@ -401,6 +422,13 @@
after_status,
)?;
+ // Development config option to test write races
+ if let Err(e) =
+ debug_wait_for_file(config, "status.pre-dirstate-write-file")
+ {
+ ui.write_stderr(e.as_bytes()).ok();
+ }
+
if (fixup.is_empty() || filesystem_time_at_status_start.is_none())
&& !dirstate_write_needed
{
@@ -420,9 +448,21 @@
// `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
+ let metadata_res = repo
.working_directory_vfs()
- .symlink_metadata(&fs_path)?;
+ .symlink_metadata(&fs_path);
+ let fs_metadata = match metadata_res {
+ Ok(meta) => meta,
+ Err(err) => match err {
+ HgError::IoError { .. } => {
+ // The file has probably been deleted. In any
+ // case, it was in the dirstate before, so
+ // let's ignore the error.
+ continue;
+ }
+ _ => return Err(err.into()),
+ },
+ };
if let Some(mtime) =
TruncatedTimestamp::for_reliable_mtime_of(
&fs_metadata,
@@ -449,6 +489,7 @@
// Not updating the dirstate is not ideal but not critical:
// don’t keep our caller waiting until some other Mercurial
// process releases the lock.
+ log::info!("not writing dirstate from `status`: lock is held")
}
Err(LockError::Other(HgError::IoError { error, .. }))
if error.kind() == io::ErrorKind::PermissionDenied =>
@@ -528,6 +569,16 @@
}
}
+/// Outcome of the additional check for an ambiguous tracked file
+enum UnsureOutcome {
+ /// The file is actually clean
+ Clean,
+ /// The file has been modified
+ Modified,
+ /// The file was deleted on disk (or became another type of fs entry)
+ Deleted,
+}
+
/// Check if a file is modified by comparing actual repo store and file system.
///
/// This meant to be used for those that the dirstate cannot resolve, due
@@ -538,7 +589,7 @@
check_exec: bool,
manifest: &Manifest,
hg_path: &HgPath,
-) -> Result<bool, HgError> {
+) -> Result<UnsureOutcome, HgError> {
let vfs = working_directory_vfs;
let fs_path = hg_path_to_path_buf(hg_path).expect("HgPath conversion");
let fs_metadata = vfs.symlink_metadata(&fs_path)?;
@@ -567,7 +618,7 @@
};
if entry_flags != fs_flags {
- return Ok(true);
+ return Ok(UnsureOutcome::Modified);
}
let filelog = hg::filelog::Filelog::open_vfs(&store_vfs, hg_path)?;
let fs_len = fs_metadata.len();
@@ -581,7 +632,7 @@
if filelog_entry.file_data_len_not_equal_to(fs_len) {
// No need to read file contents:
// it cannot be equal if it has a different length.
- return Ok(true);
+ return Ok(UnsureOutcome::Modified);
}
let p1_filelog_data = filelog_entry.data()?;
@@ -589,7 +640,7 @@
if p1_contents.len() as u64 != fs_len {
// No need to read file contents:
// it cannot be equal if it has a different length.
- return Ok(true);
+ return Ok(UnsureOutcome::Modified);
}
let fs_contents = if is_symlink {
@@ -597,5 +648,10 @@
} else {
vfs.read(fs_path)?
};
- Ok(p1_contents != &*fs_contents)
+
+ Ok(if p1_contents != &*fs_contents {
+ UnsureOutcome::Modified
+ } else {
+ UnsureOutcome::Clean
+ })
}