rust/rhg/src/commands/status.rs
changeset 50252 a6b8b1ab9116
parent 50094 1cffc156f7cd
parent 50226 8fcd5302243a
child 50393 98fc949bec14
child 50441 668a871454e8
--- 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
+    })
 }