diff --git a/CHANGELOG.md b/CHANGELOG.md index bcaa7cdd9f..e67bb836cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,9 +59,9 @@ 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`. + 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 overrident + manually by setting `core.ignore-executable-bit = true`. ### Fixed bugs diff --git a/cli/src/config-schema.json b/cli/src/config-schema.json index 084f3fe959..122d274293 100644 --- a/cli/src/config-schema.json +++ b/cli/src/config-schema.json @@ -179,9 +179,9 @@ } } }, - "report-executable-bit": { + "ignore-executable-bit": { "type": "boolean", - "description": "Whether to report changes to the executable bit for files on unix. This is ignored on windows." + "description": "Whether to ignore changes to the executable bit for files on unix. This is always true on windows" } } }, diff --git a/cli/src/merge_tools/mod.rs b/cli/src/merge_tools/mod.rs index 195f88c993..7993d94565 100644 --- a/cli/src/merge_tools/mod.rs +++ b/cli/src/merge_tools/mod.rs @@ -26,7 +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::ignore_executable_bit; use jj_lib::settings::ConfigResultExt as _; use jj_lib::settings::UserSettings; use jj_lib::working_copy::SnapshotError; @@ -192,7 +192,7 @@ impl DiffEditor { ) -> Result { let tool = get_tool_config(settings, name)? .unwrap_or_else(|| MergeTool::external(ExternalMergeTool::with_program(name))); - let exec_config = report_executable_bit(settings.config()); + let exec_config = ignore_executable_bit(settings.config()); Self::new_inner(tool, settings, base_ignores, exec_config) } diff --git a/cli/src/ui.rs b/cli/src/ui.rs index 7009a9bb87..c518eb9e8c 100644 --- a/cli/src/ui.rs +++ b/cli/src/ui.rs @@ -32,7 +32,7 @@ use std::thread::JoinHandle; use indoc::indoc; use itertools::Itertools as _; -use jj_lib::settings::report_executable_bit; +use jj_lib::settings::ignore_executable_bit; use minus::MinusError; use minus::Pager as MinusPager; use tracing::instrument; @@ -368,7 +368,7 @@ impl Ui { paginate: pagination_setting(config)?, progress_indicator, output: UiOutput::new_terminal(), - exec_config: report_executable_bit(config), + exec_config: ignore_executable_bit(config), }) } diff --git a/docs/config.md b/docs/config.md index 3ef12688fd..64d1d4b6c1 100644 --- a/docs/config.md +++ b/docs/config.md @@ -951,24 +951,25 @@ 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 +## Ignore the executable bit -`core.report-executable-bit = true | false` +`core.ignore-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. +Whether to ignore changes to the executable bit for files on unix. On windows +there is no executable bit and this config is unused. 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`. +executable bits are supported. If they are, we will not ignore them and changing +a files' executable bit will update the repository. If they are not supported we +will only update files through manual changes 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. +On unix setting this to `true` will ignore executable bits on files and setting +it to `false` will respect executable bits on files 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 ` or +You can manually update an executable bit for a file on either unix or windows +regardless of this setting with `jj file chmod x ` or `jj file chmod x `. ## Snapshot settings diff --git a/lib/src/local_working_copy.rs b/lib/src/local_working_copy.rs index 2b1eec56a1..6d7fdfd14a 100644 --- a/lib/src/local_working_copy.rs +++ b/lib/src/local_working_copy.rs @@ -92,7 +92,7 @@ use crate::op_store::WorkspaceId; use crate::repo_path::RepoPath; use crate::repo_path::RepoPathBuf; use crate::repo_path::RepoPathComponent; -use crate::settings::report_executable_bit; +use crate::settings::ignore_executable_bit; use crate::settings::HumanByteSize; use crate::store::Store; use crate::tree::Tree; @@ -107,29 +107,22 @@ use crate::working_copy::WorkingCopy; use crate::working_copy::WorkingCopyFactory; use crate::working_copy::WorkingCopyStateError; -/// None means we are ignoring the executable bit and don't care what it is. +/// The executable bit for a filetype, potentially ignored. /// -/// On Windows there is no executable bit, so this will always be None. On Unix -/// it will usually be Some(_), but may be disabled by a config value for users -/// with non-standard filesystems or who use a nonstandard filesystem on unix. -/// -/// This represents *whether* a file is executable, but not whether that state -/// is to be reported. +/// On Windows there is no executable bit, so this will always be Ignore. On +/// Unix it will usually be Exec(true|false), but may be ignored by a config +/// value or by a check we run when we load the config if not specified. #[derive(Debug, Clone, Copy)] pub enum ExecFlag { Exec(bool), - DontCare, + Ignore, } // Note: cannot derive Eq or PartialEq since a == b == c does not imply a == c. -// e.g. Exec(true) == DontCare == Exec(false) but Exec(true) != Exec(false) +// e.g. `Exec(true) == Ignore == Exec(false)` but `Exec(true) != Exec(false)` impl Default for ExecFlag { fn default() -> Self { - #[cfg(unix)] - let exec_flag = ExecFlag::Exec(false); - #[cfg(windows)] - let exec_flag = ExecFlag::DontCare; - exec_flag + ExecFlag::Exec(false) } } @@ -137,43 +130,44 @@ impl ExecFlag { fn matches(&self, other: &Self) -> bool { match (self, other) { (ExecFlag::Exec(a), ExecFlag::Exec(b)) => a == b, - // Always treat as equal if either is None. + // Always treat as equal if either is Ignore. _ => true, } } - /// Create a bool in an environment where we can't check a ReportExec value. + /// Create a bool in an environment where we can't check a IgnoreExec value. fn from_bool_unchecked(executable: bool) -> Self { #[cfg(unix)] let exec_flag = ExecFlag::Exec(executable); #[cfg(windows)] - let exec_flag = ExecFlag::DontCare; + let exec_flag = ExecFlag::Ignore; exec_flag } } -/// Whether to report the executable bit when comparing files. The executable -/// state is not reported on windows, but may be reported on unix depending on -/// the filesystem and user configuration. +/// Whether to ignore the executable bit when comparing files. The executable +/// state is always ignored on windows, but is respected by default on unix and +/// is ignored if we find that the filesystem doesn't support it or by user +/// configuration. #[cfg(unix)] #[derive(Debug, Clone, Copy)] -struct ReportExec(bool); +struct IgnoreExec(bool); #[cfg(windows)] #[derive(Debug, Clone, Copy)] -struct ReportExec(); +struct IgnoreExec(); -impl ReportExec { +impl IgnoreExec { /// Load from user settings. If the setting is not given on unix, then we /// check whether executable bits are supported in the working copy's /// filesystem and return true or false accordingly. fn load_config(exec_config: Option, wc_path: &PathBuf) -> Self { // TODO: update comments everywhere that we are checking now. #[cfg(unix)] // check for executable support on Unix. - let report_exec = - ReportExec(exec_config.unwrap_or_else(|| check_executable_bit_support(wc_path))); + let ignore_exec = + IgnoreExec(exec_config.unwrap_or_else(|| !check_executable_bit_support(wc_path))); #[cfg(windows)] - let (report_exec, _) = (ReportExec(), exec_config); // use the variable - report_exec + let (ignore_exec, _) = (IgnoreExec(), exec_config); // use the variable + ignore_exec } /// Push back into an Option config value for roundtripping. @@ -186,18 +180,17 @@ impl ReportExec { exec_config } - /// Maybe report the executable bit as a flag for the FileType. - fn report_exec_bit bool>(self, is_executable: F) -> ExecFlag { + /// Resolve an executable bit into a flag for the FileType, potentially + /// ignoring it. + fn into_flag bool>(self, is_executable: F) -> ExecFlag { #[cfg(unix)] let exec_flag = if self.0 { - // report the bit only if we care. - ExecFlag::Exec(is_executable()) + ExecFlag::Ignore } else { - // don't report the bit (emulating windows) - ExecFlag::DontCare + ExecFlag::Exec(is_executable()) }; #[cfg(windows)] - let (exec_flag, _, _) = (ExecFlag::DontCare, self, is_executable); // use the variables + let (exec_flag, _, _) = (ExecFlag::Ignore, self, is_executable); // use the variables exec_flag } @@ -262,7 +255,7 @@ impl FileState { /// Whether this file state is compatible with another file state. The extra /// complexity here comes from executable flags which always match on - /// windows and might always match on unix if report_exec is true. + /// windows and might always match on unix if ignore_exec is true. fn matches(&self, other: &Self) -> bool { use FileType::*; let file_types_match = match (&self.file_type, &other.file_type) { @@ -460,7 +453,7 @@ pub struct TreeState { sparse_patterns: Vec, own_mtime: MillisSinceEpoch, symlink_support: bool, - report_exec: ReportExec, + ignore_exec: IgnoreExec, /// The most recent clock value returned by Watchman. Will only be set if /// the repo is configured to use the Watchman filesystem monitor and @@ -601,18 +594,17 @@ fn mtime_from_metadata(metadata: &Metadata) -> MillisSinceEpoch { ) } -fn file_state(metadata: &Metadata, report_exec: ReportExec) -> Option { +fn file_state(metadata: &Metadata, ignore_exec: IgnoreExec) -> Option { let metadata_file_type = metadata.file_type(); let file_type = if metadata_file_type.is_dir() { None } else if metadata_file_type.is_symlink() { Some(FileType::Symlink) } else if metadata_file_type.is_file() { - // TODO: here #[cfg(unix)] - let exec_flag = report_exec.report_exec_bit(|| metadata.permissions().mode() & 0o111 != 0); + let exec_flag = ignore_exec.into_flag(|| metadata.permissions().mode() & 0o111 != 0); #[cfg(windows)] - let exec_flag = ExecFlag::DontCare; + let exec_flag = ExecFlag::Ignore; Some(FileType::Normal { exec_flag }) } else { None @@ -705,7 +697,7 @@ impl TreeState { exec_config: Option, ) -> TreeState { let tree_id = store.empty_merged_tree_id(); - let report_exec = ReportExec::load_config(exec_config, &working_copy_path); + let ignore_exec = IgnoreExec::load_config(exec_config, &working_copy_path); TreeState { store, working_copy_path, @@ -716,7 +708,7 @@ impl TreeState { own_mtime: MillisSinceEpoch(0), symlink_support: check_symlink_support().unwrap_or(false), watchman_clock: None, - report_exec, + ignore_exec, } } @@ -1124,7 +1116,7 @@ impl TreeState { }); } }; - if let Some(new_file_state) = file_state(&metadata, self.report_exec) { + if let Some(new_file_state) = file_state(&metadata, self.ignore_exec) { present_files_tx.send(tracked_path.to_owned()).ok(); let update = self.get_updated_tree_value( tracked_path, @@ -1192,7 +1184,7 @@ impl TreeState { max_size: HumanByteSize(max_new_file_size), }); } - if let Some(new_file_state) = file_state(&metadata, self.report_exec) { + if let Some(new_file_state) = file_state(&metadata, self.ignore_exec) { present_files_tx.send(path.clone()).ok(); let update = self.get_updated_tree_value( &path, @@ -1332,7 +1324,7 @@ impl TreeState { let id = self.write_file_to_store(repo_path, disk_path).await?; // Use the given executable bit or return the current bit. let executable = - self.report_exec + self.ignore_exec .exec_bit_to_write(exec_flag, || match current_tree_value { Some(TreeValue::File { id: _, executable }) => *executable, _ => false, @@ -1354,7 +1346,7 @@ impl TreeState { Ok(file_id) => { // Use the given executable bit or preserve the executable // bit from the merged trees. - let executable = self.report_exec.exec_bit_to_write(exec_flag, || { + let executable = self.ignore_exec.exec_bit_to_write(exec_flag, || { if let Some(merge) = current_tree_values.to_executable_merge() { merge.resolve_trivial().copied().unwrap_or(false) } else { @@ -1458,18 +1450,13 @@ impl TreeState { disk_path: &Path, executable: bool, ) -> Result { - let exec_flag = self.report_exec.report_exec_bit(|| executable); - match exec_flag { - ExecFlag::Exec(executable) => { - let mode = if executable { 0o755 } else { 0o644 }; - #[cfg(unix)] - fs::set_permissions(disk_path, fs::Permissions::from_mode(mode)) - .map_err(|err| checkout_error_for_stat_error(err, disk_path))?; - #[cfg(windows)] - let _ = mode; // use the variable - } - ExecFlag::DontCare => {} - } + let exec_flag = self.ignore_exec.into_flag(|| executable); + #[cfg(unix)] + if let ExecFlag::Exec(executable) = exec_flag { + let mode = if executable { 0o755 } else { 0o644 }; + fs::set_permissions(disk_path, fs::Permissions::from_mode(mode)) + .map_err(|err| checkout_error_for_stat_error(err, disk_path))?; + }; Ok(exec_flag) } @@ -1653,7 +1640,7 @@ impl TreeState { let file_type = match after.into_resolved() { Ok(value) => match value.unwrap() { TreeValue::File { id: _, executable } => FileType::Normal { - exec_flag: self.report_exec.report_exec_bit(|| executable), + exec_flag: self.ignore_exec.into_flag(|| executable), }, TreeValue::Symlink(_id) => FileType::Symlink, TreeValue::Conflict(_id) => { @@ -1673,7 +1660,7 @@ impl TreeState { for value in values.adds().flatten() { // Use the *last* added filetype from the merge if let TreeValue::File { id: _, executable } = value { - let exec_flag = self.report_exec.report_exec_bit(|| *executable); + let exec_flag = self.ignore_exec.into_flag(|| *executable); file_type = FileType::Normal { exec_flag }; } } @@ -1721,7 +1708,7 @@ pub struct LocalWorkingCopy { state_path: PathBuf, checkout_state: OnceCell, tree_state: OnceCell, - report_exec: ReportExec, + ignore_exec: IgnoreExec, } impl WorkingCopy for LocalWorkingCopy { @@ -1762,7 +1749,7 @@ impl WorkingCopy for LocalWorkingCopy { // TODO: It's expensive to reload the whole tree. We should copy it from `self` if it // hasn't changed. tree_state: OnceCell::new(), - report_exec: self.report_exec, + ignore_exec: self.ignore_exec, }; let old_operation_id = wc.operation_id().clone(); let old_tree_id = wc.tree_id()?.clone(); @@ -1806,20 +1793,20 @@ impl LocalWorkingCopy { store.clone(), working_copy_path.clone(), state_path.clone(), - report_executable_bit(config), + ignore_executable_bit(config), ) .map_err(|err| WorkingCopyStateError { message: "Failed to initialize working copy state".to_string(), err: err.into(), })?; - let report_exec = tree_state.report_exec; + let ignore_exec = tree_state.ignore_exec; Ok(LocalWorkingCopy { store, working_copy_path, state_path, checkout_state: OnceCell::new(), tree_state: OnceCell::with_value(tree_state), - report_exec, + ignore_exec, }) } @@ -1829,15 +1816,15 @@ impl LocalWorkingCopy { state_path: PathBuf, config: &config::Config, ) -> LocalWorkingCopy { - let exec_config = report_executable_bit(config); - let report_exec = ReportExec::load_config(exec_config, &working_copy_path); + let exec_config = ignore_executable_bit(config); + let ignore_exec = IgnoreExec::load_config(exec_config, &working_copy_path); LocalWorkingCopy { store, working_copy_path, state_path, checkout_state: OnceCell::new(), tree_state: OnceCell::new(), - report_exec, + ignore_exec, } } @@ -1886,7 +1873,7 @@ impl LocalWorkingCopy { self.store.clone(), self.working_copy_path.clone(), self.state_path.clone(), - self.report_exec.as_config(), + self.ignore_exec.as_config(), ) }) .map_err(|err| WorkingCopyStateError { @@ -2247,5 +2234,5 @@ mod tests { assert_eq!(file_states.get(repo_path("z")), None); } - // TODO: Add new tests for `report_exec` states. + // TODO: Add new tests for `ignore_exec` states. } diff --git a/lib/src/settings.rs b/lib/src/settings.rs index 575e878897..76ef6858ab 100644 --- a/lib/src/settings.rs +++ b/lib/src/settings.rs @@ -258,23 +258,23 @@ impl UserSettings { } } -/// 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. +/// Whether to ignore changes to the executable bit for files on unix. On +/// windows there is no executable bit and this config is unused. /// /// 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 +/// whether executable bits are supported. If they are, we will respect changes +/// to files' executable bit and update the repository. If ignored, 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 +/// On unix setting this to `true` will disable automatic executable bit updates +/// and setting it to `false` will enable it regardless of the filesystem check. +/// This can be useful for users dual-booting windows or experimenting with /// nonstandard filesystems. /// /// This is not a method on UserSettings since it needs to be callable even when /// only having the config itself (currently only used in `Ui::with_config`). -pub fn report_executable_bit(config: &config::Config) -> Option { - config.get_bool("core.report-executable-bit").ok() +pub fn ignore_executable_bit(config: &config::Config) -> Option { + config.get_bool("core.ignore-executable-bit").ok() } /// This Rng uses interior mutability to allow generating random values using an