git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] sequencer: honor GIT_REFLOG_ACTION

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Apr 2, 2020, 09:25 UTC
Message-ID
<b187cb5f-a6c8-2908-e3fd-e1210e6970e0@gmail.com>
In-Reply-To
<pull.746.git.git.1585773096145.gitgitgadget@gmail.com>
Hi Elijah
Thanks for fixing this
On 01/04/2020 21:31, Elijah Newren via GitGitGadget wrote:
Show 40 quoted lines
> From: Elijah Newren <newren@gmail.com>
> 
> There is a lot of code to honor GIT_REFLOG_ACTION throughout git,
> including some in sequencer.c; unfortunately, reflog_message() and its
> callers ignored it.  Instruct reflog_message() to check the existing
> environment variable, and use it when present as an override to
> action_name().
> 
> Also restructure pick_commits() to only temporarily modify
> GIT_REFLOG_ACTION for a short duration and then restore the old value,
> so that when we do this setting within a loop we do not keep adding "
> (pick)" substrings and end up with a reflog message of the form
>      rebase (pick) (pick) (pick) (finish): returning to refs/heads/master
> 
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>      sequencer: honor GIT_REFLOG_ACTION
>      
>      I'm not the best with getenv/setenv. The xstrdup() wrapping is
>      apparently necessary on mac and bsd. The xstrdup seems like it leaves us
>      with a memory leak, but since setenv(3) says to not alter or free it, I
>      think it's right. Anyone have any alternative suggestions?
> 
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-746%2Fnewren%2Fhonor-reflog-action-in-sequencer-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-746/newren/honor-reflog-action-in-sequencer-v1
> Pull-Request: https://github.com/git/git/pull/746
> 
>   sequencer.c               |  9 +++++++--
>   t/t3406-rebase-message.sh | 16 ++++++++--------
>   2 files changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/sequencer.c b/sequencer.c
> index e528225e787..5837fdaabbe 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -3708,10 +3708,11 @@ static const char *reflog_message(struct replay_opts *opts,
>   {
>   	va_list ap;
>   	static struct strbuf buf = STRBUF_INIT;
> +	char *reflog_action = getenv("GIT_REFLOG_ACTION");

Minor nit - you're using a string here rather that the pre-processor constant that is used below

Show 15 quoted lines
>   	va_start(ap, fmt);
>   	strbuf_reset(&buf);
> -	strbuf_addstr(&buf, action_name(opts));
> +	strbuf_addstr(&buf, reflog_action ? reflog_action : action_name(opts));
>   	if (sub_action)
>   		strbuf_addf(&buf, " (%s)", sub_action);
>   	if (fmt) {
> @@ -3799,8 +3800,10 @@ static int pick_commits(struct repository *r,
>   			struct replay_opts *opts)
>   {
>   	int res = 0, reschedule = 0;
> +	char *prev_reflog_action;
>   
>   	setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
> +	prev_reflog_action = xstrdup(getenv(GIT_REFLOG_ACTION));

I'm confused as to why saving the environment variable immediately after setting it works but the test shows it does - why doesn't this clobber the value of GIT_REFLOG_ACTION set by the user?

Best Wishes
Phillip
Show 57 quoted lines
>   	if (opts->allow_ff)
>   		assert(!(opts->signoff || opts->no_commit ||
>   				opts->record_origin || opts->edit));
> @@ -3845,12 +3848,14 @@ static int pick_commits(struct repository *r,
>   		}
>   		if (item->command <= TODO_SQUASH) {
>   			if (is_rebase_i(opts))
> -				setenv("GIT_REFLOG_ACTION", reflog_message(opts,
> +				setenv(GIT_REFLOG_ACTION, reflog_message(opts,
>   					command_to_string(item->command), NULL),
>   					1);
>   			res = do_pick_commit(r, item->command, item->commit,
>   					     opts, is_final_fixup(todo_list),
>   					     &check_todo);
> +			if (is_rebase_i(opts))
> +				setenv(GIT_REFLOG_ACTION, prev_reflog_action, 1);
>   			if (is_rebase_i(opts) && res < 0) {
>   				/* Reschedule */
>   				advise(_(rescheduled_advice),
> diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
> index 61b76f33019..927a4f4a4e4 100755
> --- a/t/t3406-rebase-message.sh
> +++ b/t/t3406-rebase-message.sh
> @@ -89,22 +89,22 @@ test_expect_success 'GIT_REFLOG_ACTION' '
>   	git checkout -b reflog-topic start &&
>   	test_commit reflog-to-rebase &&
>   
> -	git rebase --apply reflog-onto &&
> +	git rebase reflog-onto &&
>   	git log -g --format=%gs -3 >actual &&
>   	cat >expect <<-\EOF &&
> -	rebase finished: returning to refs/heads/reflog-topic
> -	rebase: reflog-to-rebase
> -	rebase: checkout reflog-onto
> +	rebase (finish): returning to refs/heads/reflog-topic
> +	rebase (pick): reflog-to-rebase
> +	rebase (start): checkout reflog-onto
>   	EOF
>   	test_cmp expect actual &&
>   
>   	git checkout -b reflog-prefix reflog-to-rebase &&
> -	GIT_REFLOG_ACTION=change-the-reflog git rebase --apply reflog-onto &&
> +	GIT_REFLOG_ACTION=change-the-reflog git rebase reflog-onto &&
>   	git log -g --format=%gs -3 >actual &&
>   	cat >expect <<-\EOF &&
> -	rebase finished: returning to refs/heads/reflog-prefix
> -	change-the-reflog: reflog-to-rebase
> -	change-the-reflog: checkout reflog-onto
> +	change-the-reflog (finish): returning to refs/heads/reflog-prefix
> +	change-the-reflog (pick): reflog-to-rebase
> +	change-the-reflog (start): checkout reflog-onto
>   	EOF
>   	test_cmp expect actual
>   '
> 
> base-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925
> 
Previous: Elijah NewrenNext: Elijah Newren
Message 7 of 14 in “sequencer: honor GIT_REFLOG_ACTION”
  1. sequencer: honor GIT_REFLOG_ACTIONElijah Newren via GitGitGadget, Apr 1, 2020
  2. Junio C HamanoApr 1, 2020
  3. Ian JacksonApr 1, 2020
  4. Elijah NewrenApr 2, 2020
  5. Phillip WoodApr 2, 2020
  6. Elijah NewrenApr 2, 2020
  7. Phillip WoodApr 2, 2020
  8. Elijah NewrenApr 2, 2020
  9. Phillip WoodApr 2, 2020
  10. Johannes SchindelinApr 7, 2020
  11. Elijah NewrenApr 7, 2020
  12. Johannes SchindelinApr 7, 2020
  13. Junio C HamanoApr 7, 2020
  14. sequencer: honor GIT_REFLOG_ACTIONElijah Newren via GitGitGadget, Apr 7, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.