@matheusbsbs / matheus-bagliano / commits / f8b863598c

builtin/merge: honor commit-msg hook for merges

Similar to 65969d43d1 (merge: honor prepare-commit-msg hook, 2011-02-14) merge should also honor the commit-msg hook: When a merge is stopped due to conflicts or --no-commit, the subsequent commit calls the commit-msg hook. However, it is not called after a clean merge. Fix this inconsistency by invoking the hook after clean merges as well. This change is motivated by Gerrit's commit-msg hook to install a ChangeId trailer into the commit message. Without such a ChangeId, Gerrit refuses to accept any commit by default, such that the inconsistency of (not) running the commit-msg hook between commit and merge leads to confusion and might block people from getting their work done. As the githooks man page is very vocal about the possibility of skipping the commit-msg hook via the --no-verify option, implement the option in merge, too. 'git merge --continue' is currently implemented as calling cmd_commit with no further arguments. This works for most other merge related options, such as demonstrated via the --allow-unrelated-histories flag in the test. The --no-verify option however is not remembered across invocations of git-merge. Originally the author assumed an alternative in which the 'git merge --continue' command accepts the --no-verify flag, but that opens up the discussion which flags are allows to the continued merge command and which must be given in the first invocation. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Stefan Beller committed Sep 7, 2017 at 15:04 UTC f8b863598c991a5441d97e5dc59af8968ea37e37

2 files changed +68 -4

builtin/merge.c

@@ -73,6 +73,7 @@ static int show_progress = -1;

73 static int default_to_upstream = 1;

74 static int signoff;

75 static const char *sign_commit;

76 +static int verify_msg = 1;

78 static struct strategy all_strategy[] = {

79 { "recursive", DEFAULT_TWOHEAD | NO_TRIVIAL },

@@ -236,6 +237,7 @@ static struct option builtin_merge_options[] = {

237 N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },

238 OPT_BOOL(0, "overwrite-ignore", &overwrite_ignore, N_("update ignored files (default)")),

239 OPT_BOOL(0, "signoff", &signoff, N_("add Signed-off-by:")),

240 + OPT_BOOL(0, "verify", &verify_msg, N_("verify commit-msg hook")),

241 OPT_END()

242 };

243

@@ -780,6 +782,12 @@ static void prepare_to_commit(struct commit_list *remoteheads)

782 if (launch_editor(git_path_merge_msg(), NULL, NULL))

783 abort_commit(remoteheads, NULL);

784 }

785 +

786 + if (verify_msg && run_commit_hook(0 < option_edit, get_index_file(),

787 + "commit-msg",

788 + git_path_merge_msg(), NULL))

789 + abort_commit(remoteheads, NULL);

790 +

791 read_merge_msg(&msg);

792 strbuf_stripspace(&msg, 0 < option_edit);

793 if (!msg.len)

t/t7504-commit-msg-hook.sh

+60 -4

@@ -101,6 +101,10 @@ cat > "$HOOK" <<EOF

101 exit 1

102 EOF

103

104 +commit_msg_is () {

105 + test "$(git log --pretty=format:%s%b -1)" = "$1"

106 +}

107 +

108 test_expect_success 'with failing hook' '

109

110 echo "another" >> file &&

@@ -135,6 +139,32 @@ test_expect_success '--no-verify with failing hook (editor)' '

139

140 '

141

142 +test_expect_success 'merge fails with failing hook' '

143 +

144 + test_when_finished "git branch -D newbranch" &&

145 + test_when_finished "git checkout -f master" &&

146 + git checkout --orphan newbranch &&

147 + : >file2 &&

148 + git add file2 &&

149 + git commit --no-verify file2 -m in-side-branch &&

150 + test_must_fail git merge --allow-unrelated-histories master &&

151 + commit_msg_is "in-side-branch" # HEAD before merge

152 +

153 +'

154 +

155 +test_expect_success 'merge bypasses failing hook with --no-verify' '

156 +

157 + test_when_finished "git branch -D newbranch" &&

158 + test_when_finished "git checkout -f master" &&

159 + git checkout --orphan newbranch &&

160 + : >file2 &&

161 + git add file2 &&

162 + git commit --no-verify file2 -m in-side-branch &&

163 + git merge --no-verify --allow-unrelated-histories master &&

164 + commit_msg_is "Merge branch '\''master'\'' into newbranch"

165 +'

166 +

167 +

168 chmod -x "$HOOK"

169 test_expect_success POSIXPERM 'with non-executable hook' '

170

@@ -178,10 +208,6 @@ exit 0

208 EOF

209 chmod +x "$HOOK"

210

181 -commit_msg_is () {

182 - test "$(git log --pretty=format:%s%b -1)" = "$1"

183 -}

184 -

211 test_expect_success 'hook edits commit message' '

212

213 echo "additional" >> file &&

@@ -217,7 +243,36 @@ test_expect_success "hook doesn't edit commit message (editor)" '

243 echo "more plus" > FAKE_MSG &&

244 GIT_EDITOR="\"\$FAKE_EDITOR\"" git commit --no-verify &&

245 commit_msg_is "more plus"

246 +'

247

248 +test_expect_success 'hook called in git-merge picks up commit message' '

249 + test_when_finished "git branch -D newbranch" &&

250 + test_when_finished "git checkout -f master" &&

251 + git checkout --orphan newbranch &&

252 + : >file2 &&

253 + git add file2 &&

254 + git commit --no-verify file2 -m in-side-branch &&

255 + git merge --allow-unrelated-histories master &&

256 + commit_msg_is "new message"

257 +'

258 +

259 +test_expect_failure 'merge --continue remembers --no-verify' '

260 + test_when_finished "git branch -D newbranch" &&

261 + test_when_finished "git checkout -f master" &&

262 + git checkout master &&

263 + echo a >file2 &&

264 + git add file2 &&

265 + git commit --no-verify -m "add file2 to master" &&

266 + git checkout -b newbranch master^ &&

267 + echo b >file2 &&

268 + git add file2 &&

269 + git commit --no-verify file2 -m in-side-branch &&

270 + git merge --no-verify -m not-rewritten-by-hook master &&

271 + # resolve conflict:

272 + echo c >file2 &&

273 + git add file2 &&

274 + git merge --continue &&

275 + commit_msg_is not-rewritten-by-hook

276 '

277

278 # set up fake editor to replace `pick` by `reword`

@@ -237,4 +292,5 @@ test_expect_success 'hook is called for reword during `rebase -i`' '

292

293 '

294

295 +

296 test_done