@matheusbsbs / matheus-bagliano / commits / 53f6746615

sequencer: store commit message in private context

Add an strbuf to "struct replay_ctx" to hold the current commit message. This does not change the behavior but it will allow us to fix a bug with "git rebase --signoff" in the next commit. A future patch series will use the changes here to avoid writing the commit message to disc unless there are conflicts or the commit is being reworded. The changes in do_pick_commit() are a mechanical replacement of "msgbuf" with "ctx->message". In do_merge() the code to write commit message to disc is factored out of the conditional now that both branches store the message in the same buffer. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Phillip Wood committed Apr 18, 2024 at 14:14 UTC 53f67466153ffd291955d7d55bcb33decf664aaf

1 file changed +50 -46

sequencer.c

+50 -46

@@ -211,6 +211,11 @@ static GIT_PATH_FUNC(rebase_path_keep_redundant_commits, "rebase-merge/keep_redu

211 * A 'struct replay_ctx' represents the private state of the sequencer.

212 */

213 struct replay_ctx {

214 + /*

215 + * The commit message that will be used except at the end of a

216 + * chain of fixup and squash commands.

217 + */

218 + struct strbuf message;

219 /*

220 * The list of completed fixup and squash commands in the

221 * current chain.

@@ -226,6 +231,10 @@ struct replay_ctx {

231 * current chain.

232 */

233 int current_fixup_count;

234 + /*

235 + * Whether message contains a commit message.

236 + */

237 + unsigned have_message :1;

238 };

239

240 struct replay_ctx* replay_ctx_new(void)

@@ -233,6 +242,7 @@ struct replay_ctx* replay_ctx_new(void)

242 struct replay_ctx *ctx = xcalloc(1, sizeof(*ctx));

243

244 strbuf_init(&ctx->current_fixups, 0);

245 + strbuf_init(&ctx->message, 0);

246

247 return ctx;

248 }

@@ -399,6 +409,7 @@ static const char *gpg_sign_opt_quoted(struct replay_opts *opts)

409 static void replay_ctx_release(struct replay_ctx *ctx)

410 {

411 strbuf_release(&ctx->current_fixups);

412 + strbuf_release(&ctx->message);

413 }

414

415 void replay_opts_release(struct replay_opts *opts)

@@ -2205,7 +2216,6 @@ static int do_pick_commit(struct repository *r,

2216 const char *base_label, *next_label;

2217 char *author = NULL;

2218 struct commit_message msg = { NULL, NULL, NULL, NULL };

2208 - struct strbuf msgbuf = STRBUF_INIT;

2219 int res, unborn = 0, reword = 0, allow, drop_commit;

2220 enum todo_command command = item->command;

2221 struct commit *commit = item->commit;

@@ -2304,7 +2314,7 @@ static int do_pick_commit(struct repository *r,

2314 next = parent;

2315 next_label = msg.parent_label;

2316 if (opts->commit_use_reference) {

2307 - strbuf_addstr(&msgbuf,

2317 + strbuf_addstr(&ctx->message,

2318 "# *** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***");

2319 } else if (skip_prefix(msg.subject, "Revert \"", &orig_subject) &&

2320 /*

@@ -2313,21 +2323,21 @@ static int do_pick_commit(struct repository *r,

2323 * thus requiring excessive complexity to deal with.

2324 */

2325 !starts_with(orig_subject, "Revert \"")) {

2316 - strbuf_addstr(&msgbuf, "Reapply \"");

2317 - strbuf_addstr(&msgbuf, orig_subject);

2326 + strbuf_addstr(&ctx->message, "Reapply \"");

2327 + strbuf_addstr(&ctx->message, orig_subject);

2328 } else {

2319 - strbuf_addstr(&msgbuf, "Revert \"");

2320 - strbuf_addstr(&msgbuf, msg.subject);

2321 - strbuf_addstr(&msgbuf, "\"");

2329 + strbuf_addstr(&ctx->message, "Revert \"");

2330 + strbuf_addstr(&ctx->message, msg.subject);

2331 + strbuf_addstr(&ctx->message, "\"");

2332 }

2323 - strbuf_addstr(&msgbuf, "\n\nThis reverts commit ");

2324 - refer_to_commit(opts, &msgbuf, commit);

2333 + strbuf_addstr(&ctx->message, "\n\nThis reverts commit ");

2334 + refer_to_commit(opts, &ctx->message, commit);

2335

2336 if (commit->parents && commit->parents->next) {

2327 - strbuf_addstr(&msgbuf, ", reversing\nchanges made to ");

2328 - refer_to_commit(opts, &msgbuf, parent);

2337 + strbuf_addstr(&ctx->message, ", reversing\nchanges made to ");

2338 + refer_to_commit(opts, &ctx->message, parent);

2339 }

2330 - strbuf_addstr(&msgbuf, ".\n");

2340 + strbuf_addstr(&ctx->message, ".\n");

2341 } else {

2342 const char *p;

2343

@@ -2336,21 +2346,22 @@ static int do_pick_commit(struct repository *r,

2346 next = commit;

2347 next_label = msg.label;

2348

2339 - /* Append the commit log message to msgbuf. */

2349 + /* Append the commit log message to ctx->message. */

2350 if (find_commit_subject(msg.message, &p))

2341 - strbuf_addstr(&msgbuf, p);

2351 + strbuf_addstr(&ctx->message, p);

2352

2353 if (opts->record_origin) {

2344 - strbuf_complete_line(&msgbuf);

2345 - if (!has_conforming_footer(&msgbuf, NULL, 0))

2346 - strbuf_addch(&msgbuf, '\n');

2347 - strbuf_addstr(&msgbuf, cherry_picked_prefix);

2348 - strbuf_addstr(&msgbuf, oid_to_hex(&commit->object.oid));

2349 - strbuf_addstr(&msgbuf, ")\n");

2354 + strbuf_complete_line(&ctx->message);

2355 + if (!has_conforming_footer(&ctx->message, NULL, 0))

2356 + strbuf_addch(&ctx->message, '\n');

2357 + strbuf_addstr(&ctx->message, cherry_picked_prefix);

2358 + strbuf_addstr(&ctx->message, oid_to_hex(&commit->object.oid));

2359 + strbuf_addstr(&ctx->message, ")\n");

2360 }

2361 if (!is_fixup(command))

2362 author = get_author(msg.message);

2363 }

2364 + ctx->have_message = 1;

2365

2366 if (command == TODO_REWORD)

2367 reword = 1;

@@ -2381,7 +2392,7 @@ static int do_pick_commit(struct repository *r,

2392 }

2393

2394 if (opts->signoff && !is_fixup(command))

2384 - append_signoff(&msgbuf, 0, 0);

2395 + append_signoff(&ctx->message, 0, 0);

2396

2397 if (is_rebase_i(opts) && write_author_script(msg.message) < 0)

2398 res = -1;

@@ -2390,17 +2401,17 @@ static int do_pick_commit(struct repository *r,

2401 !strcmp(opts->strategy, "ort") ||

2402 command == TODO_REVERT) {

2403 res = do_recursive_merge(r, base, next, base_label, next_label,

2393 - &head, &msgbuf, opts);

2404 + &head, &ctx->message, opts);

2405 if (res < 0)

2406 goto leave;

2407

2397 - res |= write_message(msgbuf.buf, msgbuf.len,

2408 + res |= write_message(ctx->message.buf, ctx->message.len,

2409 git_path_merge_msg(r), 0);

2410 } else {

2411 struct commit_list *common = NULL;

2412 struct commit_list *remotes = NULL;

2413

2403 - res = write_message(msgbuf.buf, msgbuf.len,

2414 + res = write_message(ctx->message.buf, ctx->message.len,

2415 git_path_merge_msg(r), 0);

2416

2417 commit_list_insert(base, &common);

@@ -2485,7 +2496,6 @@ fast_forward_edit:

2496 leave:

2497 free_message(commit, &msg);

2498 free(author);

2488 - strbuf_release(&msgbuf);

2499 update_abort_safety_file();

2500

2501 return res;

@@ -3952,6 +3962,7 @@ static int do_merge(struct repository *r,

3962 const char *arg, int arg_len,

3963 int flags, int *check_todo, struct replay_opts *opts)

3964 {

3965 + struct replay_ctx *ctx = opts->ctx;

3966 int run_commit_flags = 0;

3967 struct strbuf ref_name = STRBUF_INIT;

3968 struct commit *head_commit, *merge_commit, *i;

@@ -4080,40 +4091,31 @@ static int do_merge(struct repository *r,

4091 write_author_script(message);

4092 find_commit_subject(message, &body);

4093 len = strlen(body);

4083 - ret = write_message(body, len, git_path_merge_msg(r), 0);

4094 + strbuf_add(&ctx->message, body, len);

4095 repo_unuse_commit_buffer(r, commit, message);

4085 - if (ret) {

4086 - error_errno(_("could not write '%s'"),

4087 - git_path_merge_msg(r));

4088 - goto leave_merge;

4089 - }

4096 } else {

4097 struct strbuf buf = STRBUF_INIT;

4092 - int len;

4098

4099 strbuf_addf(&buf, "author %s", git_author_info(0));

4100 write_author_script(buf.buf);

4096 - strbuf_reset(&buf);

4101 + strbuf_release(&buf);

4102

4103 if (oneline_offset < arg_len) {

4099 - p = arg + oneline_offset;

4100 - len = arg_len - oneline_offset;

4104 + strbuf_add(&ctx->message, arg + oneline_offset,

4105 + arg_len - oneline_offset);

4106 } else {

4102 - strbuf_addf(&buf, "Merge %s '%.*s'",

4107 + strbuf_addf(&ctx->message, "Merge %s '%.*s'",

4108 to_merge->next ? "branches" : "branch",

4109 merge_arg_len, arg);

4105 - p = buf.buf;

4106 - len = buf.len;

4107 - }

4108 -

4109 - ret = write_message(p, len, git_path_merge_msg(r), 0);

4110 - strbuf_release(&buf);

4111 - if (ret) {

4112 - error_errno(_("could not write '%s'"),

4113 - git_path_merge_msg(r));

4114 - goto leave_merge;

4110 }

4111 }

4112 + ctx->have_message = 1;

4113 + if (write_message(ctx->message.buf, ctx->message.len,

4114 + git_path_merge_msg(r), 0)) {

4115 + ret = error_errno(_("could not write '%s'"),

4116 + git_path_merge_msg(r));

4117 + goto leave_merge;

4118 + }

4119

4120 if (strategy || to_merge->next) {

4121 /* Octopus merge */

@@ -4885,6 +4887,8 @@ static int pick_commits(struct repository *r,

4887 return stopped_at_head(r);

4888 }

4889 }

4890 + strbuf_reset(&ctx->message);

4891 + ctx->have_message = 0;

4892 if (item->command <= TODO_SQUASH) {

4893 res = pick_one_commit(r, todo_list, opts, &check_todo,

4894 &reschedule);