@matheusbsbs / matheus-bagliano / commits / a3152edc97

sequencer: start removing private fields from public API

"struct replay_opts" has a number of fields that are for internal use. While they are marked as private having them in a public struct is a distraction for callers and means that every time the internal details are changed we have to recompile all the files that include sequencer.h even though the public API is unchanged. This commit starts the process of removing the private fields by adding an opaque pointer to a "struct replay_ctx" to "struct replay_opts" and moving the "reflog_message" member to the new private struct. The sequencer currently updates the state files on disc each time it processes a command in the todo list. This is an artifact of the scripted implementation and makes the code hard to reason about as it is not possible to get a complete view of the state in memory. In the future we will add new members to "struct replay_ctx" to remedy this and avoid writing state to disc unless the sequencer stops for user interaction. 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 a3152edc97ff37f61387c6b222c68bc4f8b19bee

2 files changed +35 -7

sequencer.c

+30 -6

@@ -207,6 +207,24 @@ static GIT_PATH_FUNC(rebase_path_no_reschedule_failed_exec, "rebase-merge/no-res

207 static GIT_PATH_FUNC(rebase_path_drop_redundant_commits, "rebase-merge/drop_redundant_commits")

208 static GIT_PATH_FUNC(rebase_path_keep_redundant_commits, "rebase-merge/keep_redundant_commits")

209

210 +/*

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

212 + */

213 +struct replay_ctx {

214 + /*

215 + * Stores the reflog message that will be used when creating a

216 + * commit. Points to a static buffer and should not be free()'d.

217 + */

218 + const char *reflog_message;

219 +};

220 +

221 +struct replay_ctx* replay_ctx_new(void)

222 +{

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

224 +

225 + return ctx;

226 +}

227 +

228 /**

229 * A 'struct update_refs_record' represents a value in the update-refs

230 * list. We use a string_list to map refs to these (before, after) pairs.

@@ -377,6 +395,7 @@ void replay_opts_release(struct replay_opts *opts)

395 if (opts->revs)

396 release_revisions(opts->revs);

397 free(opts->revs);

398 + free(opts->ctx);

399 }

400

401 int sequencer_remove_state(struct replay_opts *opts)

@@ -1054,6 +1073,7 @@ static int run_git_commit(const char *defmsg,

1073 struct replay_opts *opts,

1074 unsigned int flags)

1075 {

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

1077 struct child_process cmd = CHILD_PROCESS_INIT;

1078

1079 if ((flags & CLEANUP_MSG) && (flags & VERBATIM_MSG))

@@ -1071,7 +1091,7 @@ static int run_git_commit(const char *defmsg,

1091 gpg_opt, gpg_opt);

1092 }

1093

1074 - strvec_pushf(&cmd.env, GIT_REFLOG_ACTION "=%s", opts->reflog_message);

1094 + strvec_pushf(&cmd.env, GIT_REFLOG_ACTION "=%s", ctx->reflog_message);

1095

1096 if (opts->committer_date_is_author_date)

1097 strvec_pushf(&cmd.env, "GIT_COMMITTER_DATE=%s",

@@ -1457,6 +1477,7 @@ static int try_to_commit(struct repository *r,

1477 struct replay_opts *opts, unsigned int flags,

1478 struct object_id *oid)

1479 {

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

1481 struct object_id tree;

1482 struct commit *current_head = NULL;

1483 struct commit_list *parents = NULL;

@@ -1618,7 +1639,7 @@ static int try_to_commit(struct repository *r,

1639 goto out;

1640 }

1641

1621 - if (update_head_with_reflog(current_head, oid, opts->reflog_message,

1642 + if (update_head_with_reflog(current_head, oid, ctx->reflog_message,

1643 msg, &err)) {

1644 res = error("%s", err.buf);

1645 goto out;

@@ -4725,11 +4746,12 @@ static int pick_one_commit(struct repository *r,

4746 struct replay_opts *opts,

4747 int *check_todo, int* reschedule)

4748 {

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

4750 int res;

4751 struct todo_item *item = todo_list->items + todo_list->current;

4752 const char *arg = todo_item_get_arg(todo_list, item);

4753 if (is_rebase_i(opts))

4732 - opts->reflog_message = reflog_message(

4754 + ctx->reflog_message = reflog_message(

4755 opts, command_to_string(item->command), NULL);

4756

4757 res = do_pick_commit(r, item, opts, is_final_fixup(todo_list),

@@ -4786,9 +4808,10 @@ static int pick_commits(struct repository *r,

4808 struct todo_list *todo_list,

4809 struct replay_opts *opts)

4810 {

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

4812 int res = 0, reschedule = 0;

4813

4791 - opts->reflog_message = sequencer_reflog_action(opts);

4814 + ctx->reflog_message = sequencer_reflog_action(opts);

4815 if (opts->allow_ff)

4816 assert(!(opts->signoff || opts->no_commit ||

4817 opts->record_origin || should_edit(opts) ||

@@ -5205,6 +5228,7 @@ static int commit_staged_changes(struct repository *r,

5228

5229 int sequencer_continue(struct repository *r, struct replay_opts *opts)

5230 {

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

5232 struct todo_list todo_list = TODO_LIST_INIT;

5233 int res;

5234

@@ -5224,7 +5248,7 @@ int sequencer_continue(struct repository *r, struct replay_opts *opts)

5248 unlink(rebase_path_dropped());

5249 }

5250

5227 - opts->reflog_message = reflog_message(opts, "continue", NULL);

5251 + ctx->reflog_message = reflog_message(opts, "continue", NULL);

5252 if (commit_staged_changes(r, opts, &todo_list)) {

5253 res = -1;

5254 goto release_todo_list;

@@ -5276,7 +5300,7 @@ static int single_pick(struct repository *r,

5300 TODO_PICK : TODO_REVERT;

5301 item.commit = cmit;

5302

5279 - opts->reflog_message = sequencer_reflog_action(opts);

5303 + opts->ctx->reflog_message = sequencer_reflog_action(opts);

5304 return do_pick_commit(r, &item, opts, 0, &check_todo);

5305 }

5306

sequencer.h

+5 -1

@@ -29,6 +29,9 @@ enum commit_msg_cleanup_mode {

29 COMMIT_MSG_CLEANUP_ALL

30 };

32 +struct replay_ctx;

33 +struct replay_ctx* replay_ctx_new(void);

34 +

35 struct replay_opts {

36 enum replay_action action;

@@ -78,13 +81,14 @@ struct replay_opts {

81 struct rev_info *revs;

83 /* Private use */

81 - const char *reflog_message;

84 + struct replay_ctx *ctx;

85 };

86 #define REPLAY_OPTS_INIT { \

87 .edit = -1, \

88 .action = -1, \

89 .current_fixups = STRBUF_INIT, \

90 .xopts = STRVEC_INIT, \

91 + .ctx = replay_ctx_new(), \

92 }

94 /*