Skip to content
Merged
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
68 changes: 60 additions & 8 deletions src/hook/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ pub fn install_hook() -> Result<String> {
// Already a cora-managed hook — just overwrite
debug!("existing hook is cora-managed, overwriting");
} else {
// Non-cora hook — back it up and compose a wrapper
let backup = hooks_dir.join("pre-commit.bak");
// Non-cora hook — back it up and compose a wrapper.
// Use `pre-commit.pre-cora.bak` to match the name uninstall_hook looks for.
let backup = hooks_dir.join("pre-commit.pre-cora.bak");
std::fs::copy(&hook_path, &backup)?;
debug!(path = %backup.display(), "backed up existing non-cora hook");

Expand Down Expand Up @@ -65,7 +66,8 @@ pub fn install_hook() -> Result<String> {
pub fn uninstall_hook() -> Result<()> {
let hooks_dir = find_git_hooks_dir()?;
let hook_path = hooks_dir.join("pre-commit");
let backup_path = hooks_dir.join("pre-commit.cora.bak");
// Restore from backup if one exists, otherwise remove the hook.
// `pre-commit.pre-cora.bak` is the only backup name install_hook writes.
let pre_backup = hooks_dir.join("pre-commit.pre-cora.bak");

if !hook_path.is_file() {
Expand All @@ -79,11 +81,9 @@ pub fn uninstall_hook() -> Result<()> {
return Ok(());
}

if backup_path.is_file() {
std::fs::rename(&backup_path, &hook_path).context("failed to restore backup hook")?;
debug!("restored hook from backup");
} else if pre_backup.is_file() {
std::fs::rename(&pre_backup, &hook_path).context("failed to restore pre-cora backup")?;
if pre_backup.is_file() {
std::fs::rename(&pre_backup, &hook_path)
.context("failed to restore pre-cora backup hook")?;
debug!("restored pre-cora hook from backup");
} else {
std::fs::remove_file(&hook_path).context("failed to remove hook")?;
Expand Down Expand Up @@ -136,3 +136,55 @@ pub fn is_hook_installed() -> Result<bool> {
let content = std::fs::read_to_string(&hook_path).unwrap_or_default();
Ok(content.contains("cora"))
}

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

/// Create a temp git repo — used to verify hook file operations.
fn temp_git_repo() -> tempfile::TempDir {
let tmp = tempfile::tempdir().unwrap();
Command::new("git")
.args(["init"])
.current_dir(tmp.path())
.output()
.unwrap();
tmp
}

#[test]
fn backup_filename_is_pre_cora_bak() {
let tmp = temp_git_repo();
let hooks_dir = tmp.path().join(".git/hooks");
fs::create_dir_all(&hooks_dir).unwrap();

// Simulate an existing non-cora hook
let hook_path = hooks_dir.join("pre-commit");
fs::write(&hook_path, "#!/bin/sh\necho my-hook\n").unwrap();

// We can't call install_hook() directly because it uses `git rev-parse`
// from CWD, not from a configurable path. Instead, verify the backup
// filename constant is consistent between install and uninstall logic.
//
// The install path writes to: pre-commit.pre-cora.bak
// The uninstall path reads from: pre-commit.pre-cora.bak
// This test documents the expected filename.
let expected_backup = "pre-commit.pre-cora.bak";

// Verify no other backup names are referenced in the source
let source = include_str!("install.rs");
assert!(
!source.contains("\"pre-commit.bak\""),
"install.rs should not use generic 'pre-commit.bak' — it was the root cause of data loss"
);
assert!(
!source.contains("\"pre-commit.cora.bak\""),
"install.rs should not use 'pre-commit.cora.bak' — uninstall never wrote this name"
);
assert!(
source.contains(&format!("\"{expected_backup}\"")),
"install.rs should consistently use '{expected_backup}' for backup"
);
}
}
Loading