Patchwork D11840: rhg: Update the dirstate on disk after status

login
register
mail settings
Submitter phabricator
Date Dec. 2, 2021, 4:49 p.m.
Message ID <differential-rev-PHID-DREV-zatx5b6qej454uqjp7pp-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/50166/
State Superseded
Headers show

Comments

phabricator - Dec. 2, 2021, 4:49 p.m.
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D11840

AFFECTED FILES
  rust/hg-core/src/dirstate/entry.rs
  rust/rhg/src/commands/status.rs
  tests/test-backout.t
  tests/test-dirstate-race2.t
  tests/test-dirstate.t

CHANGE DETAILS




To: SimonSapin, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

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
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-backout.t b/tests/test-backout.t
--- a/tests/test-backout.t
+++ b/tests/test-backout.t
@@ -172,8 +172,7 @@ 
   $ 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/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 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
@@ -222,6 +226,7 @@ 
             &ds_status.unsure
         );
     }
+    let mut fixup = Vec::new();
     if !ds_status.unsure.is_empty()
         && (display_states.modified || display_states.clean)
     {
@@ -230,14 +235,17 @@ 
             CommandError::from((e, &*format!("{:x}", p1.short())))
         })?;
         for to_check in ds_status.unsure {
-            if unsure_is_modified(repo, &manifest, &to_check)? {
+            if let Some((mode, size, mtime)) =
+                unsure_is_clean(repo, &manifest, &to_check)?
+            {
+                if display_states.clean {
+                    ds_status.clean.push(to_check.clone());
+                }
+                fixup.push((to_check.into_owned(), mode, size, mtime))
+            } else {
                 if display_states.modified {
                     ds_status.modified.push(to_check);
                 }
-            } else {
-                if display_states.clean {
-                    ds_status.clean.push(to_check);
-                }
             }
         }
     }
@@ -311,6 +319,38 @@ 
             b"C",
         )?;
     }
+    if !fixup.is_empty() {
+        // Update the dirstate on disk if we can
+        let with_lock_result =
+            repo.try_with_wlock_no_wait(|| -> Result<(), CommandError> {
+                for (path, mode, size, mtime) in fixup {
+                    let mut entry = dmap
+                        .get(&path)?
+                        .expect("ambiguous file not in dirstate");
+                    entry.set_clean(mode, size, mtime);
+                    dmap.add_file(&path, entry)?
+                }
+                drop(dmap); // Avoid "already mutably borrowed" RefCell panics
+                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(())
 }
 
@@ -355,13 +395,15 @@ 
 ///
 /// This meant to be used for those that the dirstate cannot resolve, due
 /// to time resolution limits.
-fn unsure_is_modified(
+///
+/// If the file is clean, return `Some((mode, size, mtime))` to update its dirstate entry.
+fn unsure_is_clean(
     repo: &Repo,
     manifest: &Manifest,
     hg_path: &HgPath,
-) -> Result<bool, HgError> {
+) -> Result<Option<(u32, u32, TruncatedTimestamp)>, 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
@@ -378,7 +420,7 @@ 
         .find_file(hg_path)?
         .expect("ambgious file not in p1");
     if entry.flags != fs_flags {
-        return Ok(true);
+        return Ok(None);
     }
     let filelog = repo.filelog(hg_path)?;
     let filelog_entry =
@@ -388,9 +430,18 @@ 
     let contents_in_p1 = filelog_entry.data()?;
 
     let fs_contents = if is_symlink {
-        get_bytes_from_os_string(vfs.read_link(fs_path)?.into_os_string())
+        get_bytes_from_os_string(vfs.read_link(&fs_path)?.into_os_string())
+    } else {
+        vfs.read(&fs_path)?
+    };
+    if contents_in_p1 != &*fs_contents {
+        Ok(None)
     } else {
-        vfs.read(fs_path)?
-    };
-    return Ok(contents_in_p1 != &*fs_contents);
+        use std::os::unix::fs::MetadataExt;
+        let mode = fs_metadata.mode();
+        let size = fs_contents.len() as u32 & RANGE_MASK_31BIT;
+        let mtime = TruncatedTimestamp::for_mtime_of(&fs_metadata)
+            .when_reading_file(&fs_path)?;
+        Ok(Some((mode, size, 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
@@ -173,7 +173,7 @@ 
 }
 
 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;