Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
224 changes: 224 additions & 0 deletions crates/git-smee-cli/src/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,224 @@
use std::{
env, fs,
path::{Path, PathBuf},
};

use git_smee_core::{config::LifeCyclePhase, installer};

use crate::config_path::normalize_config_path_for_hook_script;

pub(crate) struct ExpectedHookScript {
config_path: String,
executable_path: Option<String>,
}

impl ExpectedHookScript {
pub(crate) fn from_current_process(config_path: &Path, repository_root: &Path) -> Self {
let normalized_config_path =
normalize_config_path_for_hook_script(config_path, repository_root)
.unwrap_or_else(|_| config_path.to_path_buf());
Self {
config_path: normalized_config_path.to_string_lossy().to_string(),
executable_path: env::current_exe()
.ok()
.map(|path| path.to_string_lossy().to_string()),
}
}

pub(crate) fn stale_reasons(&self, hook_content: &str) -> Vec<String> {
let mut reasons = Vec::new();
if !hook_content.contains(&self.config_path) {
reasons.push(format!("expected config path {}", self.config_path));
}
match &self.executable_path {
Some(expected_exe) if !hook_content.contains(expected_exe) => {
reasons.push(format!("expected executable {expected_exe}"));
}
_ => {}
}
reasons
}
}

#[derive(Debug)]
pub(crate) struct HookInspection {
phase: LifeCyclePhase,
path: PathBuf,
display_path: String,
state: HookInspectionState,
}

impl HookInspection {
pub(crate) fn phase(&self) -> LifeCyclePhase {
self.phase
}

pub(crate) fn path(&self) -> &Path {
&self.path
}

pub(crate) fn display_path(&self) -> &str {
&self.display_path
}

pub(crate) fn state(&self) -> &HookInspectionState {
&self.state
}
}

#[derive(Debug)]
pub(crate) enum HookInspectionState {
Missing,
InvalidPath,
Unmanaged,
Managed { content: String },
Unreadable { error: String },
}

pub(crate) fn inspect_hook(
repository_root: &Path,
hooks_dir: &Path,
phase: LifeCyclePhase,
) -> HookInspection {
let path = hooks_dir.join(phase.as_str());
let display_path = display_repo_path(repository_root, &path);
let state = if !path.exists() {
HookInspectionState::Missing
} else if !path.is_file() {
HookInspectionState::InvalidPath
} else {
match installer::has_managed_header(&path) {
Ok(false) => HookInspectionState::Unmanaged,
Ok(true) => match fs::read_to_string(&path) {
Ok(content) => HookInspectionState::Managed { content },
Err(error) => HookInspectionState::Unreadable {
error: error.to_string(),
},
},
Err(error) => HookInspectionState::Unreadable {
error: error.to_string(),
},
}
};

HookInspection {
phase,
path,
display_path,
state,
}
}

pub(crate) fn inspect_obsolete_managed_hooks(
repository_root: &Path,
hooks_dir: &Path,
configured_phases: &[LifeCyclePhase],
) -> Vec<HookInspection> {
LifeCyclePhase::all()
.iter()
.copied()
.filter(|phase| !configured_phases.contains(phase))
.filter_map(|phase| {
let inspection = inspect_hook(repository_root, hooks_dir, phase);
matches!(inspection.state(), HookInspectionState::Managed { .. }).then_some(inspection)
Comment on lines +122 to +123

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep obsolete detection header-only

For unconfigured hooks, status only needs to know whether the git-smee managed marker is present, but this now reuses inspect_hook, which only returns Managed after fs::read_to_string succeeds. A stale managed hook with the marker in its header but non-UTF-8 bytes later in the file will be treated as Unreadable and filtered out here, so git smee status reports no obsolete hook even though the installer/pruning logic would still treat it as managed based on the header alone. Preserve the previous header-only check for obsolete hooks instead of requiring the whole file to decode as UTF-8.

Useful? React with 👍 / 👎.

})
.collect()
}

pub(crate) fn display_repo_path(repository_root: &Path, path: &Path) -> String {
path.strip_prefix(repository_root)
.unwrap_or(path)
.display()
.to_string()
.replace('\\', "/")
}

#[cfg(test)]
mod tests {
use std::fs;

use git_smee_core::installer;

use super::*;

#[test]
fn display_repo_path_uses_forward_slashes_for_repo_relative_paths() {
assert_eq!(
display_repo_path(Path::new("/repo"), Path::new("/repo/.git/hooks/pre-commit")),
".git/hooks/pre-commit"
);
}

#[test]
fn display_repo_path_keeps_external_paths_visible() {
assert_eq!(
display_repo_path(Path::new("/repo"), Path::new("/tmp/hooks/pre-commit")),
"/tmp/hooks/pre-commit"
);
}

#[test]
fn inspect_hook_treats_marker_only_in_body_as_unmanaged() {
let temp_dir = tempfile::tempdir().expect("failed to create tempdir");
let repository_root = temp_dir.path();
let hooks_dir = repository_root.join(".git/hooks");
fs::create_dir_all(&hooks_dir).expect("failed to create hooks dir");
fs::write(
hooks_dir.join("pre-commit"),
format!("#!/bin/sh\necho '{}'\n", installer::MANAGED_FILE_MARKER),
)
.expect("failed to write hook");

let inspection = inspect_hook(repository_root, &hooks_dir, LifeCyclePhase::PreCommit);

assert!(matches!(inspection.state(), HookInspectionState::Unmanaged));
}

#[test]
fn stale_reasons_report_missing_config_and_executable() {
let expected = ExpectedHookScript {
config_path: ".git-smee.toml".to_string(),
executable_path: Some("/bin/git-smee".to_string()),
};

assert_eq!(
expected.stale_reasons("#!/bin/sh\n"),
vec![
"expected config path .git-smee.toml".to_string(),
"expected executable /bin/git-smee".to_string(),
]
);
}

#[test]
fn obsolete_managed_hook_inspection_skips_configured_phases() {
let temp_dir = tempfile::tempdir().expect("failed to create tempdir");
let repository_root = temp_dir.path();
let hooks_dir = repository_root.join(".git/hooks");
fs::create_dir_all(&hooks_dir).expect("failed to create hooks dir");
fs::write(
hooks_dir.join("pre-commit"),
installer::with_managed_header("#!/bin/sh\n"),
)
.expect("failed to write managed hook");
fs::write(
hooks_dir.join("pre-push"),
installer::with_managed_header("#!/bin/sh\n"),
)
.expect("failed to write managed hook");

let inspections = inspect_obsolete_managed_hooks(
repository_root,
&hooks_dir,
&[LifeCyclePhase::PreCommit],
);

assert_eq!(
inspections
.iter()
.map(|inspection| inspection.phase().as_str())
.collect::<Vec<_>>(),
vec!["pre-push"]
);
}
}
91 changes: 31 additions & 60 deletions crates/git-smee-cli/src/doctor.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
use std::{env, fs, path::Path};
use std::path::Path;

use git_smee_core::{installer, repository};
use serde::Serialize;

use crate::config_path::{normalize_config_path_for_hook_script, read_config_file};
use crate::{
config_path::read_config_file,
diagnostics::{ExpectedHookScript, HookInspectionState, inspect_hook},
};

#[derive(Debug, Serialize)]
struct DoctorReport {
Expand Down Expand Up @@ -121,71 +124,39 @@ fn build_doctor_report(config_path: &Path) -> DoctorReport {
}
};

let expected_config_path = normalize_config_path_for_hook_script(config_path, &repository_root)
.unwrap_or_else(|_| config_path.to_path_buf());
let expected_exe = env::current_exe().ok();
let expected_config = expected_config_path.to_string_lossy().to_string();
let expected_hook_script =
ExpectedHookScript::from_current_process(config_path, &repository_root);

let mut phases: Vec<_> = config.hooks.keys().copied().collect();
phases.sort_by_key(|phase| phase.as_str());
for phase in phases {
let hook_path = hooks_dir.join(phase.as_str());
if !hook_path.exists() {
report.errors.push(format!(
let inspection = inspect_hook(&repository_root, &hooks_dir, phase);
match inspection.state() {
HookInspectionState::Missing => report.errors.push(format!(
"missing managed wrapper for {phase} at {}; run git smee install",
hook_path.display()
));
continue;
}
if !hook_path.is_file() {
report.errors.push(format!(
inspection.path().display()
)),
HookInspectionState::InvalidPath => report.errors.push(format!(
"hook path for {phase} is not a regular file: {}; remove it or fix core.hooksPath",
hook_path.display()
));
continue;
}
let is_managed = match installer::has_managed_header(&hook_path) {
Ok(is_managed) => is_managed,
Err(error) => {
report.errors.push(format!(
"cannot read hook wrapper for {phase} at {}: {error}",
hook_path.display()
));
continue;
}
};
if !is_managed {
report.errors.push(format!(
inspection.path().display()
)),
HookInspectionState::Unmanaged => report.errors.push(format!(
"unmanaged hook file blocks install for {phase} at {}; move it aside or run git smee install --force",
hook_path.display()
));
continue;
}
let content = match fs::read_to_string(&hook_path) {
Ok(content) => content,
Err(error) => {
report.errors.push(format!(
"cannot read hook wrapper for {phase} at {}: {error}",
hook_path.display()
));
continue;
}
};
report
.ok
.push(format!("managed wrapper is installed for {phase}"));
if !content.contains(&expected_config) {
report.warnings.push(format!(
"stale managed wrapper for {phase}: expected config path {}; run git smee install",
expected_config
));
}
if let Some(expected_exe) = &expected_exe {
let expected_exe = expected_exe.to_string_lossy().to_string();
if !content.contains(&expected_exe) {
report.warnings.push(format!(
"stale managed wrapper for {phase}: expected executable {expected_exe}; run git smee install"
));
inspection.path().display()
)),
HookInspectionState::Unreadable { error } => report.errors.push(format!(
"cannot read hook wrapper for {phase} at {}: {error}",
inspection.path().display()
)),
HookInspectionState::Managed { content } => {
report
.ok
.push(format!("managed wrapper is installed for {phase}"));
for stale_reason in expected_hook_script.stale_reasons(content) {
report.warnings.push(format!(
"stale managed wrapper for {phase}: {stale_reason}; run git smee install"
));
}
}
}
}
Expand Down
1 change: 1 addition & 0 deletions crates/git-smee-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use git_smee_core::{
};

mod config_path;
mod diagnostics;
mod doctor;
mod status;

Expand Down
Loading
Loading