[Updated] [+-- ] D11774: rhg: Fix status desambiguation of symlinks and executable files

[SimonSapin]

SimonSapin updated this revision to Diff 31129.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D11774?vs=31096&id=31129

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D11774/new/

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

AFFECTED FILES
  rust/hg-core/src/dirstate/entry.rs
  rust/hg-core/src/vfs.rs
  rust/rhg/src/commands/status.rs
  tests/test-execute-bit.t
  tests/test-merge-exec.t
  tests/test-merge-types.t
  tests/test-symlinks.t

CHANGE DETAILS

diff --git a/tests/test-symlinks.t b/tests/test-symlinks.t
--- a/tests/test-symlinks.t
+++ b/tests/test-symlinks.t
@@ -11,10 +11,6 @@
   > EOF
 #endif
 
-TODO: fix rhg bugs that make this test fail when status is enabled
-  $ unset RHG_STATUS
-
-
 == tests added in 0.7 ==
 
   $ hg init test-symlinks-0.7; cd test-symlinks-0.7;
diff --git a/tests/test-merge-types.t b/tests/test-merge-types.t
--- a/tests/test-merge-types.t
+++ b/tests/test-merge-types.t
@@ -1,9 +1,5 @@
 #require symlink execbit
 
-TODO: fix rhg bugs that make this test fail when status is enabled
-  $ unset RHG_STATUS
-
-
   $ tellmeabout() {
   > if [ -h $1 ]; then
   >     echo $1 is a symlink:
diff --git a/tests/test-merge-exec.t b/tests/test-merge-exec.t
--- a/tests/test-merge-exec.t
+++ b/tests/test-merge-exec.t
@@ -4,10 +4,6 @@
 
 #require execbit
 
-TODO: fix rhg bugs that make this test fail when status is enabled
-  $ unset RHG_STATUS
-
-
 Initial setup
 ==============
 
diff --git a/tests/test-execute-bit.t b/tests/test-execute-bit.t
--- a/tests/test-execute-bit.t
+++ b/tests/test-execute-bit.t
@@ -1,9 +1,5 @@
 #require execbit
 
-TODO: fix rhg bugs that make this test fail when status is enabled
-  $ unset RHG_STATUS
-
-
   $ hg init
   $ echo a > a
   $ hg ci -Am'not executable'
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
@@ -11,11 +11,12 @@
 use clap::{Arg, SubCommand};
 use hg;
 use hg::config::Config;
-use hg::dirstate::TruncatedTimestamp;
+use hg::dirstate::{has_exec_bit, TruncatedTimestamp};
 use hg::errors::HgError;
 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::{HgPathCow, StatusOptions};
 use log::{info, warn};
@@ -302,16 +303,30 @@
 ///
 /// This meant to be used for those that the dirstate cannot resolve, due
 /// to time resolution limits.
-///
-/// TODO: detect permission bits and similar metadata modifications
 fn unsure_is_modified(
     repo: &Repo,
     manifest: &Manifest,
     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_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 dirstate
+    let fs_flags = if is_symlink {
+        Some(b'l')
+    } else if has_exec_bit(&fs_metadata) {
+        Some(b'x')
+    } else {
+        None
+    };
+
     let entry = manifest
         .find_file(hg_path)?
         .expect("ambgious file not in p1");
+    if entry.flags != fs_flags {
+        return Ok(true);
+    }
     let filelog = repo.filelog(hg_path)?;
     let filelog_entry =
         filelog.data_for_node(entry.node_id()?).map_err(|_| {
@@ -319,7 +334,10 @@
         })?;
     let contents_in_p1 = filelog_entry.data()?;
 
-    let fs_path = hg_path_to_os_string(hg_path).expect("HgPath conversion");
-    let fs_contents = repo.working_directory_vfs().read(fs_path)?;
+    let fs_contents = if is_symlink {
+        get_bytes_from_os_string(vfs.read_link(fs_path)?.into_os_string())
+    } else {
+        vfs.read(fs_path)?
+    };
     return Ok(contents_in_p1 != &*fs_contents);
 }
diff --git a/rust/hg-core/src/vfs.rs b/rust/hg-core/src/vfs.rs
--- a/rust/hg-core/src/vfs.rs
+++ b/rust/hg-core/src/vfs.rs
@@ -16,6 +16,22 @@
         self.base.join(relative_path)
     }
 
+    pub fn symlink_metadata(
+        &self,
+        relative_path: impl AsRef<Path>,
+    ) -> Result<std::fs::Metadata, HgError> {
+        let path = self.join(relative_path);
+        std::fs::symlink_metadata(&path).when_reading_file(&path)
+    }
+
+    pub fn read_link(
+        &self,
+        relative_path: impl AsRef<Path>,
+    ) -> Result<PathBuf, HgError> {
+        let path = self.join(relative_path);
+        std::fs::read_link(&path).when_reading_file(&path)
+    }
+
     pub fn read(
         &self,
         relative_path: impl AsRef<Path>,
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
@@ -580,10 +580,8 @@
         &self,
         filesystem_metadata: &std::fs::Metadata,
     ) -> bool {
-        use std::os::unix::fs::MetadataExt;
-        const EXEC_BIT_MASK: u32 = 0o100;
-        let dirstate_exec_bit = (self.mode() as u32) & EXEC_BIT_MASK;
-        let fs_exec_bit = filesystem_metadata.mode() & EXEC_BIT_MASK;
+        let dirstate_exec_bit = (self.mode() as u32 & EXEC_BIT_MASK) != 0;
+        let fs_exec_bit = has_exec_bit(filesystem_metadata);
         dirstate_exec_bit != fs_exec_bit
     }
 
@@ -641,3 +639,11 @@
         }
     }
 }
+
+const EXEC_BIT_MASK: u32 = 0o100;
+
+pub fn has_exec_bit(metadata: &std::fs::Metadata) -> bool {
+    // TODO: How to handle executable permissions on Windows?
+    use std::os::unix::fs::MetadataExt;
+    (metadata.mode() & EXEC_BIT_MASK) != 0
+}



To: SimonSapin, #hg-reviewers, Alphare
Cc: Alphare, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20211125/5c433b41/attachment-0002.html>