local_working_copy: Allow user to configure exec bit behavior on Unix

CHANGELOG.md

@@ -69,6 +69,11 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 * `jj commit` and `jj describe` now accept `--author` option allowing to quickly change
   author of given commit.
 
+* Updated how we handle executable bits in the working copy to allow Unix to
+  ignore executable bit changes when using a nonstandard filesystem. We try to
+  detect the filesystem's behavior on Unix by default, but it can be overridden
+  manually by setting `core.ignore-executable-bit = true`.
+
 ### Fixed bugs
 
  * Update working copy before reporting changes. This prevents errors during reporting

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

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

cli/src/config-schema.json

@@ -178,6 +178,10 @@
                         "description": "Whether to use triggers to monitor for changes in the background."
                       }
                     }
+                },
+                "ignore-executable-bit": {
+                    "type": "boolean",
+                    "description": "Whether to ignore changes to the executable bit for files on Unix. This is always true 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::ignore_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 = ignore_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::ignore_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: ignore_executable_bit(config),
         })
     }
 

docs/config.md

@@ -951,6 +951,38 @@ snapshots on filestem changes by setting
 You can check whether Watchman is enabled and whether it is installed correctly
 using `jj debug watchman status`.
 
+## Ignore the executable bit
+
+Whether to ignore changes to the executable bit for files on Unix. This option
+is unused on Windows.
+
+`core.ignore-executable-bit = true | false`
+
+On Unix systems, files have permission bits for whether they are executable as
+scripts or binary code. Jujutsu stores this executable bit within each commit
+and will update it for files as you operate on a repository. If you update your
+working copy to a commit where a file is recorded as executable or not, `jj`
+will adjust the permission of that file. If you change a file's executable bit
+through the filesystem, `jj` will record that change in the working-copy commit
+on the next snapshot.
+
+Setting this to `true` will make Jujutsu ignore the executable bit on the
+filesystem and only store the executable bit in the commit itself. In addition,
+`jj` will not attempt to modify a file's executable bit and will add new files
+as "not executable." This is already the behavior on Windows, and having the
+option to enable this behavior is especially useful for Unix users dual-booting
+Windows, Windows users accessing files from WSL, or anyone experimenting with
+nonstandard filesystems.
+
+On Unix, Jujutsu will try to detect whether the filesystem supports executable
+bits to choose a default value, but a configured value will always override the
+default. On Windows systems there is no executable bit, and this option is not
+used.
+
+You can always use `jj file chmod` to update the recorded executable bit for a
+file in the commit manually. If this option is not set, `jj` will attempt to
+propagate that change to the filesystem.
+
 ## Snapshot settings
 
 ### Maximum size for new files

lib/src/local_working_copy.rs

@@ -92,7 +92,9 @@ use crate::op_store::WorkspaceId;
 use crate::repo_path::RepoPath;
 use crate::repo_path::RepoPathBuf;
 use crate::repo_path::RepoPathComponent;
+use crate::settings::ignore_executable_bit;
 use crate::settings::HumanByteSize;
+use crate::settings::UserSettings;
 use crate::store::Store;
 use crate::tree::Tree;
 use crate::working_copy::CheckoutError;
@@ -675,8 +677,9 @@ impl TreeState {
         store: Arc<Store>,
         working_copy_path: PathBuf,
         state_path: PathBuf,
+        exec_config: Option<bool>,
     ) -> Result<TreeState, TreeStateError> {
-        let mut wc = TreeState::empty(store, working_copy_path, state_path, None);
+        let mut wc = TreeState::empty(store, working_copy_path, state_path, exec_config);
         wc.save()?;
         Ok(wc)
     }
@@ -714,9 +717,7 @@ impl TreeState {
         let tree_state_path = state_path.join("tree_state");
         let file = match File::open(&tree_state_path) {
             Err(ref err) if err.kind() == std::io::ErrorKind::NotFound => {
-                let mut wc = TreeState::empty(store, working_copy_path, state_path, exec_config);
-                wc.save()?;
-                return Ok(wc);
+                return TreeState::init(store, working_copy_path, state_path, exec_config);
             }
             Err(err) => {
                 return Err(TreeStateError::ReadTreeState {
@@ -1775,6 +1776,7 @@ impl LocalWorkingCopy {
         state_path: PathBuf,
         operation_id: OperationId,
         workspace_id: WorkspaceId,
+        settings: &UserSettings,
     ) -> Result<LocalWorkingCopy, WorkingCopyStateError> {
         let proto = crate::protos::working_copy::Checkout {
             operation_id: operation_id.to_bytes(),
@@ -1786,13 +1788,16 @@ impl LocalWorkingCopy {
             .open(state_path.join("checkout"))
             .unwrap();
         file.write_all(&proto.encode_to_vec()).unwrap();
-        let tree_state =
-            TreeState::init(store.clone(), working_copy_path.clone(), state_path.clone()).map_err(
-                |err| WorkingCopyStateError {
-                    message: "Failed to initialize working copy state".to_string(),
-                    err: err.into(),
-                },
-            )?;
+        let tree_state = TreeState::init(
+            store.clone(),
+            working_copy_path.clone(),
+            state_path.clone(),
+            ignore_executable_bit(settings.config()),
+        )
+        .map_err(|err| WorkingCopyStateError {
+            message: "Failed to initialize working copy state".to_string(),
+            err: err.into(),
+        })?;
         let ignore_exec = tree_state.ignore_exec;
         Ok(LocalWorkingCopy {
             store,
@@ -1808,8 +1813,10 @@ impl LocalWorkingCopy {
         store: Arc<Store>,
         working_copy_path: PathBuf,
         state_path: PathBuf,
+        settings: &UserSettings,
     ) -> LocalWorkingCopy {
-        let ignore_exec = IgnoreExec::load_config(None, &working_copy_path);
+        let exec_config = ignore_executable_bit(settings.config());
+        let ignore_exec = IgnoreExec::load_config(exec_config, &working_copy_path);
         LocalWorkingCopy {
             store,
             working_copy_path,
@@ -1928,13 +1935,15 @@ impl WorkingCopyFactory for LocalWorkingCopyFactory {
         state_path: PathBuf,
         operation_id: OperationId,
         workspace_id: WorkspaceId,
+        settings: &UserSettings,
     ) -> Result<Box<dyn WorkingCopy>, WorkingCopyStateError> {
         Ok(Box::new(LocalWorkingCopy::init(
             store,
             working_copy_path,
             state_path,
             operation_id,
             workspace_id,
+            settings,
         )?))
     }
 
@@ -1943,11 +1952,13 @@ impl WorkingCopyFactory for LocalWorkingCopyFactory {
         store: Arc<Store>,
         working_copy_path: PathBuf,
         state_path: PathBuf,
+        settings: &UserSettings,
     ) -> Result<Box<dyn WorkingCopy>, WorkingCopyStateError> {
         Ok(Box::new(LocalWorkingCopy::load(
             store,
             working_copy_path,
             state_path,
+            settings,
         )))
     }
 }