status: fix symlink dirty check

Summary:
The manifest stores a file as executable or symlink (mutually exclusive). A symlink on disk also has the executable bit(s) set, so the fs metadata (and by extension the mode in the dirstate) appears to be both symlink and executable. This is a recipe for confusion.

In Python it seems to work because the dirstate mode and fs mode both store the execute bit for symlinks, so they match when compared for executableness. (Although I wouldn't be surprised if they could get out of sync somehow.) The symlinkness check is achieved through the "flags" which does not include "x" if the file is "l".

Fix in Rust be applying the flags logic to the dirstate and fs metadata as well. In other words, if a file is a symlink, don't report it as executable. This makes the manifest file mode compare naturally with the dirstate and filesystem.

Reviewed By: quark-zju

Differential Revision: D43970590

fbshipit-source-id: b459e466dace5ee1c9cc7505d3c780be6ecd4685

eden/scm/lib/treestate/src/filestate.rs

@@ -113,7 +113,10 @@ pub struct FileStateV2 {
 
 impl FileStateV2 {
     pub fn is_executable(&self) -> bool {
-        self.mode & 0o100 == 0o100
+        // Symlinks show as executable, but don't be fooled. "executable" and
+        // "symlink" are mutually exclusive in the manifest, so it is just
+        // confusing if we have dirstate entries that claim to be both.
+        !self.is_symlink() && self.mode & 0o100 == 0o100
     }
 
     pub fn is_symlink(&self) -> bool {

eden/scm/lib/vfs/src/vfs.rs

@@ -492,15 +492,22 @@ fn metadata_eq(m1: &Metadata, m2: &Metadata) -> Result<bool> {
         && m1.file_type() == m2.file_type())
 }
 
-pub fn is_executable(metadata: &Metadata) -> bool {
-    #[cfg(unix)]
-    return metadata.permissions().mode() & 0o111 != 0;
+#[cfg(windows)]
+pub fn is_executable(_metadata: &Metadata) -> bool {
+    panic!("is_executable is not supported on Windows");
+}
 
-    #[cfg(target_os = "windows")]
-    {
-        let _ = metadata;
-        panic!("is_executable is not supported on Windows");
+#[cfg(unix)]
+pub fn is_executable(metadata: &Metadata) -> bool {
+    // Symlinks show as executable, but don't be fooled. "executable"
+    // and "symlink" are mutually exclusive in the manifest, so it is
+    // just confusing if we have filesystem metadata entries that
+    // claim to be both.
+    if is_symlink(metadata) {
+        return false;
     }
+
+    metadata.permissions().mode() & 0o111 != 0
 }
 
 pub fn is_symlink(metadata: &Metadata) -> bool {

eden/scm/lib/workingcopy/src/filechangedetector.rs

@@ -223,7 +223,13 @@ pub fn file_changed_given_metadata(
             vfs.supports_symlinks() && is_symlink(&metadata) != state.is_symlink();
 
         if size_different || exec_different || symlink_different {
-            tracing::trace!(?path, "changed (size or exec or symlink differ)");
+            tracing::trace!(
+                ?path,
+                size_different,
+                exec_different,
+                symlink_different,
+                "changed"
+            );
             return Ok(FileChangeResult::changed(path.to_owned()));
         }
     }

eden/scm/tests/test-symlinks.t

@@ -1,8 +1,6 @@
 #chg-compatible
 #debugruntest-compatible
   $ eagerepo
-FIXME(status):
-  $ setconfig status.use-rust=false
   $ setconfig workingcopy.ruststatus=False
   $ setconfig experimental.allowfilepeer=True
 
@@ -66,7 +64,8 @@ test what happens if we want to trick hg
 
 it should show a.c, dir/a.o and dir/b.o deleted
 
-  $ hg status
+FIXME(status):
+  $ hg status --config status.use-rust=false
   a.c: invalid file type (no-fsmonitor !)
   M dir/b.o
   ! a.c
@@ -177,7 +176,8 @@ directory moved and symlinked
   adding foo/a
   $ mv foo bar
   $ ln -s bar foo
-  $ hg status
+FIXME(status):
+  $ hg status --config status.use-rust=false
   ! foo/a
   ? bar/a
   ? foo