Add config for reporting executable bit changes

CHANGELOG.md

@@ -58,6 +58,11 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 * `jj op undo` now reports information on the operation that has been undone.
 
+* Updated how we handle executable bits in the working copy to allow unix to
+  disable reporting executable bit changes if they're using a nonstandard
+  filesystem. We try to detect this on unix by default, but it can be overrident
+  manually by setting `core.report-executable-bit = false`.
+
 ### Fixed bugs
 
  * Fixed panic when parsing invalid conflict markers of a particular form.

cli/examples/custom-working-copy/main.rs

@@ -119,22 +119,29 @@ impl ConflictsWorkingCopy {
         state_path: PathBuf,
         operation_id: OperationId,
         workspace_id: WorkspaceId,
+        config: &config::Config,
     ) -> Result<Self, WorkingCopyStateError> {
         let inner = LocalWorkingCopy::init(
             store,
             working_copy_path.clone(),
             state_path,
             operation_id,
             workspace_id,
+            config,
         )?;
         Ok(ConflictsWorkingCopy {
             inner: Box::new(inner),
             working_copy_path,
         })
     }
 
-    fn load(store: Arc<Store>, working_copy_path: PathBuf, state_path: PathBuf) -> Self {
-        let inner = LocalWorkingCopy::load(store, working_copy_path.clone(), state_path);
+    fn load(
+        store: Arc<Store>,
+        working_copy_path: PathBuf,
+        state_path: PathBuf,
+        config: &config::Config,
+    ) -> Self {
+        let inner = LocalWorkingCopy::load(store, working_copy_path.clone(), state_path, config);
         ConflictsWorkingCopy {
             inner: Box::new(inner),
             working_copy_path,
@@ -186,13 +193,15 @@ impl WorkingCopyFactory for ConflictsWorkingCopyFactory {
         state_path: PathBuf,
         operation_id: OperationId,
         workspace_id: WorkspaceId,
+        config: &config::Config,
     ) -> Result<Box<dyn WorkingCopy>, WorkingCopyStateError> {
         Ok(Box::new(ConflictsWorkingCopy::init(
             store,
             working_copy_path,
             state_path,
             operation_id,
             workspace_id,
+            config,
         )?))
     }
 
@@ -201,11 +210,13 @@ impl WorkingCopyFactory for ConflictsWorkingCopyFactory {
         store: Arc<Store>,
         working_copy_path: PathBuf,
         state_path: PathBuf,
+        config: &config::Config,
     ) -> Result<Box<dyn WorkingCopy>, WorkingCopyStateError> {
         Ok(Box::new(ConflictsWorkingCopy::load(
             store,
             working_copy_path,
             state_path,
+            config,
         )))
     }
 }

cli/src/config-schema.json

@@ -178,6 +178,10 @@
                         "description": "Whether to use triggers to monitor for changes in the background."
                       }
                     }
+                },
+                "report-executable-bit": {
+                    "type": "boolean",
+                    "description": "Whether to report changes to the executable bit for files on unix. This is ignored on windows."
                 }
             }
         },

cli/src/merge_tools/diff_working_copies.rs

@@ -84,10 +84,11 @@ fn check_out(
     state_dir: PathBuf,
     tree: &MergedTree,
     sparse_patterns: Vec<RepoPathBuf>,
+    exec_config: Option<bool>,
 ) -> Result<TreeState, DiffCheckoutError> {
     std::fs::create_dir(&wc_dir).map_err(DiffCheckoutError::SetUpDir)?;
     std::fs::create_dir(&state_dir).map_err(DiffCheckoutError::SetUpDir)?;
-    let mut tree_state = TreeState::init(store, wc_dir, state_dir)?;
+    let mut tree_state = TreeState::init(store, wc_dir, state_dir, exec_config)?;
     tree_state.set_sparse_patterns(sparse_patterns)?;
     tree_state.check_out(tree)?;
     Ok(tree_state)
@@ -135,6 +136,7 @@ pub(crate) fn check_out_trees(
     right_tree: &MergedTree,
     matcher: &dyn Matcher,
     output_is: Option<DiffSide>,
+    exec_config: Option<bool>,
 ) -> Result<DiffWorkingCopies, DiffCheckoutError> {
     let changed_files: Vec<_> = left_tree
         .diff_stream(right_tree, matcher)
@@ -153,13 +155,15 @@ pub(crate) fn check_out_trees(
         left_state_dir,
         left_tree,
         changed_files.clone(),
+        exec_config,
     )?;
     let right_tree_state = check_out(
         store.clone(),
         right_wc_dir,
         right_state_dir,
         right_tree,
         changed_files.clone(),
+        exec_config,
     )?;
     let output_tree_state = output_is
         .map(|output_side| {
@@ -174,6 +178,7 @@ pub(crate) fn check_out_trees(
                     DiffSide::Right => right_tree,
                 },
                 changed_files,
+                exec_config,
             )
         })
         .transpose()?;
@@ -200,8 +205,16 @@ impl DiffEditWorkingCopies {
         matcher: &dyn Matcher,
         output_is: Option<DiffSide>,
         instructions: Option<&str>,
+        exec_config: Option<bool>,
     ) -> Result<Self, DiffEditError> {
-        let diff_wc = check_out_trees(store, left_tree, right_tree, matcher, output_is)?;
+        let diff_wc = check_out_trees(
+            store,
+            left_tree,
+            right_tree,
+            matcher,
+            output_is,
+            exec_config,
+        )?;
         let got_output_field = output_is.is_some();
 
         set_readonly_recursively(diff_wc.left_working_copy_path())

cli/src/merge_tools/external.rs

@@ -255,6 +255,7 @@ pub fn edit_diff_external(
     matcher: &dyn Matcher,
     instructions: Option<&str>,
     base_ignores: Arc<GitIgnoreFile>,
+    exec_config: Option<bool>,
 ) -> Result<MergedTreeId, DiffEditError> {
     let got_output_field = find_all_variables(&editor.edit_args).contains(&"output");
     let store = left_tree.store();
@@ -265,6 +266,7 @@ pub fn edit_diff_external(
         matcher,
         got_output_field.then_some(DiffSide::Right),
         instructions,
+        exec_config,
     )?;
 
     let patterns = diffedit_wc.working_copies.to_command_variables();
@@ -296,7 +298,7 @@ pub fn generate_diff(
     tool: &ExternalMergeTool,
 ) -> Result<(), DiffGenerateError> {
     let store = left_tree.store();
-    let diff_wc = check_out_trees(store, left_tree, right_tree, matcher, None)?;
+    let diff_wc = check_out_trees(store, left_tree, right_tree, matcher, None, ui.exec_config)?;
     set_readonly_recursively(diff_wc.left_working_copy_path())
         .map_err(ExternalToolError::SetUpDir)?;
     set_readonly_recursively(diff_wc.right_working_copy_path())

cli/src/merge_tools/mod.rs

@@ -26,6 +26,7 @@ use jj_lib::matchers::Matcher;
 use jj_lib::merged_tree::MergedTree;
 use jj_lib::repo_path::RepoPath;
 use jj_lib::repo_path::RepoPathBuf;
+use jj_lib::settings::report_executable_bit;
 use jj_lib::settings::ConfigResultExt as _;
 use jj_lib::settings::UserSettings;
 use jj_lib::working_copy::SnapshotError;
@@ -178,6 +179,7 @@ pub struct DiffEditor {
     tool: MergeTool,
     base_ignores: Arc<GitIgnoreFile>,
     use_instructions: bool,
+    exec_config: Option<bool>,
 }
 
 impl DiffEditor {
@@ -190,7 +192,8 @@ impl DiffEditor {
     ) -> Result<Self, MergeToolConfigError> {
         let tool = get_tool_config(settings, name)?
             .unwrap_or_else(|| MergeTool::external(ExternalMergeTool::with_program(name)));
-        Self::new_inner(tool, settings, base_ignores)
+        let exec_config = report_executable_bit(settings.config());
+        Self::new_inner(tool, settings, base_ignores, exec_config)
     }
 
     /// Loads the default diff editor from the settings.
@@ -206,18 +209,20 @@ impl DiffEditor {
             None
         }
         .unwrap_or_else(|| MergeTool::external(ExternalMergeTool::with_edit_args(&args)));
-        Self::new_inner(tool, settings, base_ignores)
+        Self::new_inner(tool, settings, base_ignores, ui.exec_config)
     }
 
     fn new_inner(
         tool: MergeTool,
         settings: &UserSettings,
         base_ignores: Arc<GitIgnoreFile>,
+        exec_config: Option<bool>,
     ) -> Result<Self, MergeToolConfigError> {
         Ok(DiffEditor {
             tool,
             base_ignores,
             use_instructions: settings.config().get_bool("ui.diff-instructions")?,
+            exec_config,
         })
     }
 
@@ -242,6 +247,7 @@ impl DiffEditor {
                     matcher,
                     instructions.as_deref(),
                     self.base_ignores.clone(),
+                    self.exec_config,
                 )
             }
         }

cli/src/ui.rs

@@ -32,6 +32,7 @@ use std::thread::JoinHandle;
 
 use indoc::indoc;
 use itertools::Itertools as _;
+use jj_lib::settings::report_executable_bit;
 use minus::MinusError;
 use minus::Pager as MinusPager;
 use tracing::instrument;
@@ -259,6 +260,7 @@ pub struct Ui {
     progress_indicator: bool,
     formatter_factory: FormatterFactory,
     output: UiOutput,
+    pub exec_config: Option<bool>,
 }
 
 fn progress_indicator_setting(config: &config::Config) -> bool {
@@ -366,6 +368,7 @@ impl Ui {
             paginate: pagination_setting(config)?,
             progress_indicator,
             output: UiOutput::new_terminal(),
+            exec_config: report_executable_bit(config),
         })
     }
 

docs/config.md

@@ -951,6 +951,26 @@ snapshots on filestem changes by setting
 You can check whether Watchman is enabled and whether it is installed correctly
 using `jj debug watchman status`.
 
+## Report executable bit
+
+`core.report-executable-bit = true | false`
+
+Whether to report changes to the executable bit for files on unix. On windows
+there is no executable bit and this config is completely ignored.
+
+On unix we determine the default value by querying the filesystem as to whether
+executable bits are supported. If they are, we will report changes to files'
+executable bit and update the repository. Otherwise we will only update files if
+they are manually changed via `jj file chmod`.
+
+On unix setting this to `false` will disable executable bit reporting and
+setting it to `true` will enable it regardless of our filesystem check. This can
+be useful for users dual-booting windows or experimenting with nonstandard
+filesystems.
+
+You can manually update an executable bit with `jj file chmod x <paths>` or
+`jj file chmod x <paths>`.
+
 ## Snapshot settings
 
 ### Maximum size for new files

lib/src/file_util.rs

@@ -165,9 +165,36 @@ pub fn persist_content_addressed_temp_file<P: AsRef<Path>>(
 
 #[cfg(unix)]
 mod platform {
+    use std::fs;
     use std::io;
     use std::os::unix::fs::symlink;
+    use std::os::unix::fs::PermissionsExt;
     use std::path::Path;
+    use std::path::PathBuf;
+
+    use tempfile::NamedTempFile;
+
+    /// Whether executable bits are supported for the filesystem of this path.
+    pub fn check_executable_bit_support(path: &PathBuf) -> bool {
+        const DEFAULT: bool = true; // default to true on unix
+        fn check_exec(path: &PathBuf) -> Result<bool, std::io::Error> {
+            // get current permissions and update with exec bit:
+            let temp_file = NamedTempFile::new_in(path)?;
+            let old_mode = temp_file.as_file().metadata()?.permissions().mode();
+            // TODO: Maybe check if we can disable the exec permission if we
+            // start executable?
+            let new_mode = old_mode | 0o100;
+            let new_perms = PermissionsExt::from_mode(new_mode);
+            if fs::set_permissions(temp_file.path(), new_perms).is_ok() {
+                let mode = temp_file.as_file().metadata()?.permissions().mode();
+                Ok(new_mode == mode && new_mode != old_mode)
+            } else {
+                Ok(DEFAULT)
+            }
+            // TODO: Actually test this function.
+        }
+        check_exec(path).unwrap_or(DEFAULT)
+    }
 
     /// Symlinks are always available on UNIX
     pub fn check_symlink_support() -> io::Result<bool> {