@matheusbsbs / matheus-bagliano / commits / d188a60d72

sequencer: stop exporting GIT_REFLOG_ACTION

Each time it picks a commit the sequencer copies the GIT_REFLOG_ACITON environment variable so it can temporarily change it and then restore the previous value. This results in code that is hard to follow and also leaks memory because (i) we fail to free the copy when we've finished with it and (ii) each call to setenv() leaks the previous value. Instead pass the reflog action around in a variable and use it to set GIT_REFLOG_ACTION in the child environment when running "git commit". Within the sequencer GIT_REFLOG_ACTION is no longer set and is only read by sequencer_reflog_action(). It is still set by rebase before calling the sequencer, that will be addressed in the next commit. cherry-pick and revert are unaffected as they do not set GIT_REFLOG_ACTION before calling the sequencer. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>

Phillip Wood committed Nov 9, 2022 at 14:21 UTC d188a60d7228f051bb4004924d20d961ada636f3

2 files changed +31 -20

sequencer.c

+25 -20

@@ -375,6 +375,7 @@ int sequencer_remove_state(struct replay_opts *opts)

375 }

376

377 free(opts->gpg_sign);

378 + free(opts->reflog_action);

379 free(opts->default_strategy);

380 free(opts->strategy);

381 for (i = 0; i < opts->xopts_nr; i++)

@@ -1050,6 +1051,8 @@ static int run_git_commit(const char *defmsg,

1051 gpg_opt, gpg_opt);

1052 }

1053

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

1055 +

1056 if (opts->committer_date_is_author_date)

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

1058 opts->ignore_date ?

@@ -1589,8 +1592,8 @@ static int try_to_commit(struct repository *r,

1592 goto out;

1593 }

1594

1592 - if (update_head_with_reflog(current_head, oid,

1593 - getenv("GIT_REFLOG_ACTION"), msg, &err)) {

1595 + if (update_head_with_reflog(current_head, oid, opts->reflog_message,

1596 + msg, &err)) {

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

1598 goto out;

1599 }

@@ -3674,17 +3677,28 @@ static int do_label(struct repository *r, const char *name, int len)

3677 return ret;

3678 }

3679

3680 +static const char *sequencer_reflog_action(struct replay_opts *opts)

3681 +{

3682 + if (!opts->reflog_action) {

3683 + opts->reflog_action = getenv(GIT_REFLOG_ACTION);

3684 + opts->reflog_action =

3685 + xstrdup(opts->reflog_action ? opts->reflog_action

3686 + : action_name(opts));

3687 + }

3688 +

3689 + return opts->reflog_action;

3690 +}

3691 +

3692 __attribute__((format (printf, 3, 4)))

3693 static const char *reflog_message(struct replay_opts *opts,

3694 const char *sub_action, const char *fmt, ...)

3695 {

3696 va_list ap;

3697 static struct strbuf buf = STRBUF_INIT;

3683 - char *reflog_action = getenv(GIT_REFLOG_ACTION);

3698

3699 va_start(ap, fmt);

3700 strbuf_reset(&buf);

3687 - strbuf_addstr(&buf, reflog_action ? reflog_action : action_name(opts));

3701 + strbuf_addstr(&buf, sequencer_reflog_action(opts));

3702 if (sub_action)

3703 strbuf_addf(&buf, " (%s)", sub_action);

3704 if (fmt) {

@@ -4497,7 +4511,7 @@ static int checkout_onto(struct repository *r, struct replay_opts *opts,

4511 RESET_HEAD_RUN_POST_CHECKOUT_HOOK,

4512 .head_msg = reflog_message(opts, "start", "checkout %s",

4513 onto_name),

4500 - .default_reflog_action = "rebase"

4514 + .default_reflog_action = sequencer_reflog_action(opts)

4515 };

4516 if (reset_head(r, &ropts)) {

4517 apply_autostash(rebase_path_autostash());

@@ -4566,11 +4580,8 @@ static int pick_commits(struct repository *r,

4580 struct replay_opts *opts)

4581 {

4582 int res = 0, reschedule = 0;

4569 - char *prev_reflog_action;

4583

4571 - /* Note that 0 for 3rd parameter of setenv means set only if not set */

4572 - setenv(GIT_REFLOG_ACTION, action_name(opts), 0);

4573 - prev_reflog_action = xstrdup(getenv(GIT_REFLOG_ACTION));

4584 + opts->reflog_message = sequencer_reflog_action(opts);

4585 if (opts->allow_ff)

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

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

@@ -4618,14 +4629,12 @@ static int pick_commits(struct repository *r,

4629 }

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

4631 if (is_rebase_i(opts))

4621 - setenv(GIT_REFLOG_ACTION, reflog_message(opts,

4622 - command_to_string(item->command), NULL),

4623 - 1);

4632 + opts->reflog_message = reflog_message(opts,

4633 + command_to_string(item->command), NULL);

4634 +

4635 res = do_pick_commit(r, item, opts,

4636 is_final_fixup(todo_list),

4637 &check_todo);

4627 - if (is_rebase_i(opts))

4628 - setenv(GIT_REFLOG_ACTION, prev_reflog_action, 1);

4638 if (is_rebase_i(opts) && res < 0) {

4639 /* Reschedule */

4640 advise(_(rescheduled_advice),

@@ -5050,8 +5059,6 @@ int sequencer_continue(struct repository *r, struct replay_opts *opts)

5059 if (read_populate_opts(opts))

5060 return -1;

5061 if (is_rebase_i(opts)) {

5053 - char *previous_reflog_action;

5054 -

5062 if ((res = read_populate_todo(r, &todo_list, opts)))

5063 goto release_todo_list;

5064

@@ -5062,13 +5069,11 @@ int sequencer_continue(struct repository *r, struct replay_opts *opts)

5069 unlink(rebase_path_dropped());

5070 }

5071

5065 - previous_reflog_action = xstrdup(getenv(GIT_REFLOG_ACTION));

5066 - setenv(GIT_REFLOG_ACTION, reflog_message(opts, "continue", NULL), 1);

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

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

5074 res = -1;

5075 goto release_todo_list;

5076 }

5071 - setenv(GIT_REFLOG_ACTION, previous_reflog_action, 1);

5077 } else if (!file_exists(get_todo_path(opts)))

5078 return continue_single_pick(r, opts);

5079 else if ((res = read_populate_todo(r, &todo_list, opts)))

@@ -5116,7 +5121,7 @@ static int single_pick(struct repository *r,

5121 TODO_PICK : TODO_REVERT;

5122 item.commit = cmit;

5123

5119 - setenv(GIT_REFLOG_ACTION, action_name(opts), 0);

5124 + opts->reflog_message = sequencer_reflog_action(opts);

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

5126 }

5127

sequencer.h

@@ -63,6 +63,9 @@ struct replay_opts {

63 char **xopts;

64 size_t xopts_nr, xopts_alloc;

66 + /* Reflog */

67 + char *reflog_action;

68 +

69 /* Used by fixup/squash */

70 struct strbuf current_fixups;

71 int current_fixup_count;

@@ -73,6 +76,9 @@ struct replay_opts {

77 /* Only used by REPLAY_NONE */

78 struct rev_info *revs;

79 +

80 + /* Private use */

81 + const char *reflog_message;

82 };

83 #define REPLAY_OPTS_INIT { .edit = -1, .action = -1, .current_fixups = STRBUF_INIT }