[RFC/PATCH] merge: honor prepare-commit-msg hook

Jay Soffian· Jul 20, 2010, 04:29 UTC · lore

--- I couldn't figure out why my prepare-commit-msg wasn't being honored by git merge. I assumed that the standard commit hooks would be honored, as after-all, merge is creating a new commit. Alas, this is not the case since merge calls commit_tree directly.

I wonder whether the low-level commit_tree should be responsible for invoking this hook instead of the commit porcelain?

In any case, this does what I want, but I'm not sure it's the best approach.

Comments?

 builtin/merge.c |   38 ++++++++++++++++++++++++++++++--------
 1 files changed, 30 insertions(+), 8 deletions(-)

Show changes to builtin/merge.c +30 −8

diff --git a/builtin/merge.c b/builtin/merge.c
index 37ce4f5..28a9960 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -768,6 +768,33 @@ static void add_strategies(const char *string, unsigned attr)
 
 }
 
+static void write_merge_msg(void)
+{
+	int fd = open(git_path("MERGE_MSG"), O_WRONLY | O_CREAT, 0666);
+	if (fd < 0)
+		die_errno("Could not open '%s' for writing",
+			  git_path("MERGE_MSG"));
+	if (write_in_full(fd, merge_msg.buf, merge_msg.len) !=
+		merge_msg.len)
+		die_errno("Could not write to '%s'", git_path("MERGE_MSG"));
+	close(fd);
+}
+
+static void read_merge_msg(void)
+{
+	strbuf_reset(&merge_msg);
+	if (strbuf_read_file(&merge_msg, git_path("MERGE_MSG"), 0) < 0)
+		die_errno("Could not read from '%s'", git_path("MERGE_MSG"));
+}
+
+static void run_prepare_commit_msg(void)
+{
+	write_merge_msg();
+	run_hook(get_index_file(), "prepare-commit-msg",
+		 git_path("MERGE_MSG"), "merge", NULL, NULL);
+	read_merge_msg();
+}
+
 static int merge_trivial(void)
 {
 	unsigned char result_tree[20], result_commit[20];
@@ -779,6 +806,7 @@ static int merge_trivial(void)
 	parent->next = xmalloc(sizeof(*parent->next));
 	parent->next->item = remoteheads->item;
 	parent->next->next = NULL;
+	run_prepare_commit_msg();
 	commit_tree(merge_msg.buf, result_tree, parent, result_commit, NULL);
 	finish(result_commit, "In-index merge");
 	drop_save();
@@ -808,6 +836,7 @@ static int finish_automerge(struct commit_list *common,
 	}
 	free_commit_list(remoteheads);
 	strbuf_addch(&merge_msg, '\n');
+	run_prepare_commit_msg();
 	commit_tree(merge_msg.buf, result_tree, parents, result_commit, NULL);
 	strbuf_addf(&buf, "Merge made by %s.", wt_strategy);
 	finish(result_commit, buf.buf);
@@ -1276,14 +1305,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 			die_errno("Could not write to '%s'", git_path("MERGE_HEAD"));
 		close(fd);
 		strbuf_addch(&merge_msg, '\n');
-		fd = open(git_path("MERGE_MSG"), O_WRONLY | O_CREAT, 0666);
-		if (fd < 0)
-			die_errno("Could not open '%s' for writing",
-				  git_path("MERGE_MSG"));
-		if (write_in_full(fd, merge_msg.buf, merge_msg.len) !=
-			merge_msg.len)
-			die_errno("Could not write to '%s'", git_path("MERGE_MSG"));
-		close(fd);
+		write_merge_msg();
 		fd = open(git_path("MERGE_MODE"), O_WRONLY | O_CREAT | O_TRUNC, 0666);
 		if (fd < 0)
 			die_errno("Could not open '%s' for writing",

-- 
1.7.2.rc3.53.g670aa

Junio C Hamano· Jul 22, 2010, 00:14 UTC · re: Jay Soffian · lore

Re: [RFC/PATCH] merge: honor prepare-commit-msg hook

Jay Soffian <jaysoffian@gmail.com> writes:

> ---
> I couldn't figure out why my prepare-commit-msg wasn't being honored
> by git merge.

It has been that way from day one, it appears.

The bypassing of pre-commit hook was and remains to be a conscious design decision. When you are pulling from your contributors who may have objectionable contents that you have to merge, the damage is already done; you _could_ yell at them to fix their branch and re-pull in theory, but that wouldn't work very well in practice.

On the other hand, I think letting people use prepare-commit-msg for merges might make sense. Indeed, "git commit" is prepared to call prepare-commit-msg telling the hook that it is concluding a merge, when your "git merge" stopped due to a conflict (or you stopped it from making a new commit with --no-commit).

I don't know about the other hooks "git commit" normally calls. Both "commit-msg" and "post-commit" may make sense, but I don't care too deeply either way---I don't care too deeply for pre-commit either ;-).

RFC patchmerge: honor prepare-commit-msg hook