Skip to content
Open
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
35 changes: 34 additions & 1 deletion crates/but/src/command/legacy/commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use but_rebase::graph_rebase::mutate::{InsertSide, RelativeTo};
use but_transaction::{IntermediateCommitCreateResult, Transaction};
use but_workspace::{RefInfo, branch::create_reference::Anchor, commit::ChangeSource};
use gitbutler_oplog::entry::{OperationKind, SnapshotDetails};
use gitbutler_repo::hooks::{ErrorData, HookResult};
use gitbutler_repo::hooks::{CommitMsgHook, ErrorData, HookResult, MessageHookResult};
use gix::bstr::{BString, ByteSlice};
use gix::refs::FullName;
use nonempty::NonEmpty;
Expand Down Expand Up @@ -306,6 +306,10 @@ pub fn run(
}

let rejection_target = commit_op.rejection_target();
let commit_msg_hook = run_hooks
.is_yes()
.then(|| CommitMsgHook::from_context(ctx))
.transpose()?;
let snapshot_details = SnapshotDetails::new(OperationKind::CreateCommit);
let ((new_commit, branch_name), _ws) = but_transaction::with_transaction_with_perm(
ctx,
Expand All @@ -330,6 +334,10 @@ pub fn run(
new_commit.context("BUG: rejected_specs is empty yet nothing was committed")?;

let reworded_commit = reword_op.execute(new_commit.into(), &mut tx)?;
let reworded_commit = match commit_msg_hook.as_ref() {
Some(hook) => run_commit_msg_hook(hook, reworded_commit, &mut tx)?,
None => reworded_commit,
};

Ok(but_transaction::Commit((reworded_commit, branch_name)))
},
Expand Down Expand Up @@ -492,6 +500,31 @@ fn run_post_commit_hook(ctx: &Context) {
tracing::warn!("post-commit hook failed: {error}");
}

/// Run `commit-msg` after the message source has been resolved but before the transaction is
/// committed. A rejecting hook therefore rolls the entire commit operation back, while a hook
/// that edits its message gets one final reword inside the same transaction.
fn run_commit_msg_hook(
hook: &CommitMsgHook,
commit: CommitId,
tx: &mut Transaction<'_, '_, impl RefMetadata>,
) -> anyhow::Result<CommitId> {
let message = tx
.repo()
.find_commit(commit.commit_id)?
.message_raw()?
.to_string();
match hook.run(message)? {
MessageHookResult::Success | MessageHookResult::NotConfigured => Ok(commit),
MessageHookResult::Message(message_data) => Ok(tx
.reword_commit(
commit.commit_id,
BString::from(message_data.message).as_ref(),
)?
.into()),
MessageHookResult::Failure(ErrorData { error }) => Err(hook_failed("commit-msg", error)),
}
}

fn hook_failed(name: &str, error: String) -> anyhow::Error {
// Wording kept from before the `commit2` rewrite dropped hook support, so anyone who saw
// this message on an older build sees the same one again.
Expand Down
117 changes: 117 additions & 0 deletions crates/but/tests/but/command/commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2344,6 +2344,7 @@ For more information, try '--help'.
/// Writes an executable `pre-commit` hook that runs `body`.
#[cfg(unix)]
fn write_pre_commit_hook(env: &Sandbox, body: &str) {
env.invoke_git("config core.hooksPath .git/hooks");
env.invoke_bash(format!(
"mkdir -p .git/hooks && printf '#!/bin/sh\\n{body}\\n' > .git/hooks/pre-commit && chmod +x .git/hooks/pre-commit"
));
Expand Down Expand Up @@ -2402,11 +2403,127 @@ fn no_hooks_commits_past_a_failing_pre_commit_hook() {
);
}

/// Writes an executable `commit-msg` hook that runs `body`.
#[cfg(unix)]
fn write_commit_msg_hook(env: &Sandbox, body: &str) {
env.invoke_git("config core.hooksPath .git/hooks");
env.invoke_bash(format!(
"mkdir -p .git/hooks && cat > .git/hooks/commit-msg <<'HOOK'\n#!/bin/sh\n{body}\nHOOK\nchmod +x .git/hooks/commit-msg"
));
}

#[cfg(unix)]
#[test]
fn commit_msg_hook_can_update_the_message() {
let env = Sandbox::init_scenario_with_target_and_default_settings("one-stack");
env.setup_metadata(&["A"]);
write_commit_msg_hook(
&env,
"printf '\\n\\nGenerated-by: commit-msg hook\\n' >> \"$1\"",
);
env.file("file.txt", "Some text");

env.but("commit -m 'add file.txt'").assert().success();

snapbox::assert_data_eq!(
env.invoke_git("show -s --format=%B refs/heads/A"),
snapbox::str![[r#"
add file.txt

Generated-by: commit-msg hook
"#]]
);
}

#[cfg(unix)]
#[test]
fn commit_msg_hook_updates_a_message_from_the_editor() {
let env = Sandbox::init_scenario_with_target_and_default_settings("one-stack");
env.setup_metadata(&["A"]);
write_commit_msg_hook(
&env,
"printf '\\n\\nGenerated-by: commit-msg hook\\n' >> \"$1\"",
);
env.file("editor.sh", "printf 'message from editor\\n' > \"$1\"\n");
let editor_command = format!("sh {}", env.projects_root().join("editor.sh").display());
env.file("file.txt", "Some text");

env.but("commit")
.env("GIT_EDITOR", editor_command)
.assert()
.success();

snapbox::assert_data_eq!(
env.invoke_git("show -s --format=%B refs/heads/A"),
snapbox::str![[r#"
message from editor

Generated-by: commit-msg hook
"#]]
);
}

#[cfg(unix)]
#[test]
fn a_failing_commit_msg_hook_rolls_back_the_commit() {
let env = Sandbox::init_scenario_with_target_and_default_settings("one-stack");
env.setup_metadata(&["A"]);
write_commit_msg_hook(&env, "echo 'message policy rejected it' >&2\nexit 1");
env.file("file.txt", "Some text");
let branch_before = env.invoke_git("rev-parse refs/heads/A");

env.but("commit -m 'add file.txt'")
.assert()
.failure()
.stderr_eq(snapbox::str![[r#"
Error: commit-msg hook failed:
message policy rejected it

To bypass the hook, run: but commit --no-hooks

"#]]);

assert_eq!(
env.invoke_git("rev-parse refs/heads/A"),
branch_before,
"a rejected message must not advance the branch"
);
assert!(
env.but("status")
.assert()
.success()
.get_output()
.stdout
.windows(b"file.txt".len())
.any(|window| window == b"file.txt"),
"a rejected commit must leave its changes uncommitted"
);
}

#[cfg(unix)]
#[test]
fn no_hooks_skips_commit_msg_hook() {
let env = Sandbox::init_scenario_with_target_and_default_settings("one-stack");
env.setup_metadata(&["A"]);
write_commit_msg_hook(&env, "echo ran > commit-msg-ran.txt\nexit 1");
env.file("file.txt", "Some text");

env.but("commit --no-hooks -m 'add file.txt'")
.assert()
.success();

assert!(
!env.projects_root().join("commit-msg-ran.txt").exists(),
"--no-hooks must skip commit-msg instead of ignoring its verdict"
);
}

/// A formatter-style hook: inserts a line in the middle of a file that is already tracked,
/// shifting every line below it and so invalidating hunk headers computed before it ran.
#[cfg(unix)]
fn write_line_shifting_hook(env: &Sandbox) {
// `sed -i` is spelled differently on BSD and GNU, so splice the line with head/tail instead.
env.invoke_git("config core.hooksPath .git/hooks");
env.invoke_bash(
"mkdir -p .git/hooks && cat > .git/hooks/pre-commit <<'HOOK'\n\
#!/bin/sh\n\
Expand Down
43 changes: 36 additions & 7 deletions crates/gitbutler-repo/src/hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,30 @@ pub enum MessageHookResult {
Failure(ErrorData),
}

/// A prepared `commit-msg` hook runner that owns its repository handle.
///
/// Commit creation resolves editor input inside a workspace transaction. Owning the legacy
/// repository handle here lets the hook run at that point without borrowing the transaction's
/// [`Context`].
pub struct CommitMsgHook {
repo: git2::Repository,
husky_search_paths: Option<&'static [&'static str]>,
}

impl CommitMsgHook {
pub fn from_context(ctx: &Context) -> Result<Self> {
let repo = git2::Repository::open(&ctx.gitdir)?;
Ok(Self {
repo,
husky_search_paths: husky_search_paths(ctx),
})
}

pub fn run(&self, message: String) -> Result<MessageHookResult> {
commit_msg_with_repo(&self.repo, self.husky_search_paths, message)
}
}

fn husky_search_paths(ctx: &Context) -> Option<&'static [&'static str]> {
if ctx.legacy_project.husky_hooks_enabled {
Some(&["../.husky"])
Expand All @@ -53,14 +77,19 @@ fn husky_search_paths(ctx: &Context) -> Option<&'static [&'static str]> {
}
}

pub fn commit_msg(ctx: &Context, mut message: String) -> Result<MessageHookResult> {
let original_message = message.clone();
pub fn commit_msg(ctx: &Context, message: String) -> Result<MessageHookResult> {
#[expect(deprecated, reason = "libgit2 hook adapter boundary")]
match git2_hooks::hooks_commit_msg(
&*ctx.git2_repo.get()?,
husky_search_paths(ctx),
&mut message,
)? {
let repo = &*ctx.git2_repo.get()?;
commit_msg_with_repo(repo, husky_search_paths(ctx), message)
}

fn commit_msg_with_repo(
repo: &git2::Repository,
husky_search_paths: Option<&[&str]>,
mut message: String,
) -> Result<MessageHookResult> {
let original_message = message.clone();
match git2_hooks::hooks_commit_msg(repo, husky_search_paths, &mut message)? {
H::NoHookFound => Ok(MessageHookResult::NotConfigured),
H::Run(HookRunResponse {
stdout,
Expand Down