From cbbcbb30942cc2eb8535b46160f43f986373106b Mon Sep 17 00:00:00 2001 From: Wilfred Hughes Date: Tue, 20 Feb 2024 00:07:06 -0800 Subject: [PATCH] Parse file mode properly from git and pass through --- CHANGELOG.md | 3 ++ src/main.rs | 93 ++++++++++++++------------------------------------ src/options.rs | 91 +++++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 115 insertions(+), 72 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 61fa19421..705b4f7b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,9 @@ Added support for Smali. Fixed an issue with paths not showing the containing directory when using difftastic with `git difftool`. +Difftastic now correctly reports file permission changes when called +from `git diff`. + Fixed an issue with the experimental JSON display mode where it ignored `--skip-unchanged`. diff --git a/src/main.rs b/src/main.rs index dd50d1f9b..7aec57912 100644 --- a/src/main.rs +++ b/src/main.rs @@ -44,6 +44,7 @@ extern crate log; use display::style::print_warning; use log::info; use mimalloc::MiMalloc; +use options::FilePermissions; use options::USAGE; use crate::conflicts::apply_conflict_markers; @@ -70,7 +71,6 @@ use crate::parse::syntax; #[global_allocator] static GLOBAL: MiMalloc = MiMalloc; -use std::fs::Permissions; use std::path::Path; use std::{env, thread}; @@ -219,6 +219,8 @@ fn main() { language_overrides, lhs_path, rhs_path, + lhs_permissions, + rhs_permissions, display_path, renamed, } => { @@ -298,6 +300,8 @@ fn main() { renamed, &lhs_path, &rhs_path, + lhs_permissions.as_ref(), + rhs_permissions.as_ref(), &display_options, &diff_options, false, @@ -328,68 +332,14 @@ fn main() { }; } -#[cfg(unix)] -fn compare_permissions(lhs: &Permissions, rhs: &Permissions) -> Option { - use std::os::unix::fs::PermissionsExt; - - let lhs_mode = lhs.mode(); - let rhs_mode = rhs.mode(); - - if lhs_mode != rhs_mode { - let lhs_mode = format!("{:o}", lhs_mode); - let rhs_mode = format!("{:o}", rhs_mode); - Some(format!( - "File permissions changed from {} to {}.", - lhs_mode, rhs_mode - )) - } else { - None - } -} - -#[cfg(windows)] -fn compare_permissions(lhs: &Permissions, rhs: &Permissions) -> Option { - if lhs.readonly() != rhs.readonly() { - Some(format!( - "File permissions changed from {} to {}.", - if lhs.readonly() { - "readonly" - } else { - "read-write" - }, - if rhs.readonly() { - "readonly" - } else { - "read-write" - }, - )) - } else { - None - } -} - -#[cfg(not(any(unix, windows)))] -fn compare_permissions(_lhs: &Permissions, _rhs: &Permissions) -> Option { - None -} - -fn describe_permissions_change(lhs_path: &FileArgument, rhs_path: &FileArgument) -> Option { - match (lhs_path, rhs_path) { - (FileArgument::NamedPath(lhs_path), FileArgument::NamedPath(rhs_path)) => { - let lhs_metadata = std::fs::metadata(lhs_path).ok()?; - let rhs_metadata = std::fs::metadata(rhs_path).ok()?; - compare_permissions(&lhs_metadata.permissions(), &rhs_metadata.permissions()) - } - _ => None, - } -} - /// Print a diff between two files. fn diff_file( display_path: &str, renamed: Option, lhs_path: &FileArgument, rhs_path: &FileArgument, + lhs_permissions: Option<&FilePermissions>, + rhs_permissions: Option<&FilePermissions>, display_options: &DisplayOptions, diff_options: &DiffOptions, missing_as_empty: bool, @@ -420,12 +370,19 @@ fn diff_file( } let mut extra_info = renamed; - if let Some(permissions_change) = describe_permissions_change(lhs_path, rhs_path) { - if let Some(extra_info) = &mut extra_info { - extra_info.push('\n'); - extra_info.push_str(&permissions_change); - } else { - extra_info = Some(permissions_change); + if let (Some(lhs_perms), Some(rhs_perms)) = (lhs_permissions, rhs_permissions) { + if lhs_perms != rhs_perms { + let msg = format!( + "File permissions changed from {} to {}.", + lhs_perms, rhs_perms + ); + + if let Some(extra_info) = &mut extra_info { + extra_info.push('\n'); + extra_info.push_str(&msg); + } else { + extra_info = Some(msg); + } } } @@ -780,14 +737,16 @@ fn diff_directories<'a>( paths.into_par_iter().map(move |rel_path| { info!("Relative path is {:?} inside {:?}", rel_path, lhs_dir); - let lhs_path = Path::new(lhs_dir).join(&rel_path); - let rhs_path = Path::new(rhs_dir).join(&rel_path); + let lhs_path = FileArgument::NamedPath(Path::new(lhs_dir).join(&rel_path)); + let rhs_path = FileArgument::NamedPath(Path::new(rhs_dir).join(&rel_path)); diff_file( &rel_path.display().to_string(), None, - &FileArgument::NamedPath(lhs_path), - &FileArgument::NamedPath(rhs_path), + &lhs_path, + &rhs_path, + lhs_path.permissions().as_ref(), + rhs_path.permissions().as_ref(), &display_options, &diff_options, true, diff --git a/src/options.rs b/src/options.rs index e513fe283..6092d5f75 100644 --- a/src/options.rs +++ b/src/options.rs @@ -1,6 +1,11 @@ //! CLI option parsing. -use std::{env, ffi::OsStr, fmt::Display, path::Path, path::PathBuf}; +use std::{ + env, + ffi::OsStr, + fmt::Display, + path::{Path, PathBuf}, +}; use clap::{crate_authors, crate_description, Arg, Command}; use const_format::formatcp; @@ -312,6 +317,61 @@ pub(crate) enum FileArgument { DevNull, } +impl FileArgument { + pub(crate) fn permissions(&self) -> Option { + match self { + FileArgument::NamedPath(path) => { + let metadata = std::fs::metadata(path).ok()?; + Some(metadata.permissions().into()) + } + FileArgument::Stdin => None, + FileArgument::DevNull => None, + } + } +} + +/// A cross-platform representation of file permissions. +/// +/// We're only interested in whether two permissions are the same, and +/// how to display them, so internally this is just a human-friendly +/// string. +#[derive(Debug, Eq, PartialEq)] +pub(crate) struct FilePermissions(String); + +impl Display for FilePermissions { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} + +impl From for FilePermissions { + fn from(s: String) -> Self { + Self(s) + } +} + +impl From<&OsStr> for FilePermissions { + fn from(s: &OsStr) -> Self { + Self(s.to_string_lossy().into_owned()) + } +} + +impl From for FilePermissions { + fn from(perms: std::fs::Permissions) -> Self { + if cfg!(unix) { + use std::os::unix::fs::PermissionsExt; + Self(format!("{:o}", perms.mode())) + } else { + let s = if perms.readonly() { + "readonly" + } else { + "read-write" + }; + Self(s.to_owned()) + } + } +} + fn try_canonicalize(path: &Path) -> PathBuf { path.canonicalize().unwrap_or_else(|_| path.into()) } @@ -378,6 +438,8 @@ pub(crate) enum Mode { /// The path where we can read the RHS file. This is often a /// temporary file generated by source control. rhs_path: FileArgument, + lhs_permissions: Option, + rhs_permissions: Option, /// The path that we show to the user. display_path: String, /// If this file has been renamed, a description of the change. @@ -660,23 +722,38 @@ pub(crate) fn parse_args() -> Mode { info!("CLI arguments: {:?}", args); // TODO: document these different ways of calling difftastic. - let (display_path, lhs_path, rhs_path, renamed) = match &args[..] { + let (display_path, lhs_path, rhs_path, lhs_permissions, rhs_permissions, renamed) = match &args + [..] + { [lhs_path, rhs_path] => { let lhs_arg = FileArgument::from_cli_argument(lhs_path); let rhs_arg = FileArgument::from_cli_argument(rhs_path); let display_path = build_display_path(&lhs_arg, &rhs_arg); - (display_path, lhs_arg, rhs_arg, None) + + let lhs_permissions = lhs_arg.permissions(); + let rhs_permissions = rhs_arg.permissions(); + + ( + display_path, + lhs_arg, + rhs_arg, + lhs_permissions, + rhs_permissions, + None, + ) } - [display_path, lhs_tmp_file, _lhs_hash, _lhs_mode, rhs_tmp_file, _rhs_hash, _rhs_mode] => { + [display_path, lhs_tmp_file, _lhs_hash, lhs_mode, rhs_tmp_file, _rhs_hash, rhs_mode] => { // https://git-scm.com/docs/git#Documentation/git.txt-codeGITEXTERNALDIFFcode ( display_path.to_string_lossy().to_string(), FileArgument::from_path_argument(lhs_tmp_file), FileArgument::from_path_argument(rhs_tmp_file), + Some((*lhs_mode).into()), + Some((*rhs_mode).into()), None, ) } - [old_name, lhs_tmp_file, _lhs_hash, _lhs_mode, rhs_tmp_file, _rhs_hash, _rhs_mode, new_name, _similarity] => + [old_name, lhs_tmp_file, _lhs_hash, lhs_mode, rhs_tmp_file, _rhs_hash, rhs_mode, new_name, _similarity] => { // Rename file. // TODO: where does git document these 9 arguments? @@ -689,6 +766,8 @@ pub(crate) fn parse_args() -> Mode { new_name, FileArgument::from_path_argument(lhs_tmp_file), FileArgument::from_path_argument(rhs_tmp_file), + Some((*lhs_mode).into()), + Some((*rhs_mode).into()), Some(renamed), ) } @@ -749,6 +828,8 @@ pub(crate) fn parse_args() -> Mode { language_overrides, lhs_path, rhs_path, + lhs_permissions, + rhs_permissions, display_path, renamed, }