From: Phillip Wood <phillip.wood123@gmail.com>
Date: Wed, 10 Sep 2025 14:04:00 GMT
Subject: Re: [PATCH RFC v3 02/18] sequencer: add option to rewind HEAD after picking commits
Message-ID: <f74b9dfe-b05c-431e-8872-92e2bbb75b8c@gmail.com>
In-Reply-To: <20250904-b4-pks-history-builtin-v3-2-509053514755@pks.im>

Hi Patrick

On 04/09/2025 15:27, Patrick Steinhardt wrote:
> While the sequencer infrastructure knows to rewind "HEAD" to whatever it
> was pointing to before a rebase, it doesn't do the same for non-rebase
> operations like cherry-picks. This is because the expectation is that
> the user directly picks commits on top of whatever "HEAD" points to, and
> we advance the reference pointed to by "HEAD" instead of updating it
> directly.
> 
> We're about to introduce a new command though that needs to detach
> "HEAD" while being more similar to git-cherry-pick(1) rathen than to
> git-rebase(1). As such, we'll want to restore "HEAD" to point to the
> branch that we started on while not using the more heavy-weight rebase
> machinery.
> 
> Introduce a new option `restore_head_target` to do so. Persist the
> option into the sequencer configuration so that it persists across
> different processes, e.g. when we need to stop due to a merge conflict.

As with the last patch, can we use this new option in "git rebase"? The 
sequencer is already a nest of conditionals, it would be nice to 
minimize the number of new ones.

Thanks

Phillip

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>   sequencer.c | 27 ++++++++++++++++++++++++++-
>   sequencer.h |  3 +++
>   2 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/sequencer.c b/sequencer.c
> index 7066cdc939..bff181df76 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -413,6 +413,7 @@ void replay_opts_release(struct replay_opts *opts)
>   	struct replay_ctx *ctx = opts->ctx;
>   
>   	free(opts->gpg_sign);
> +	free(opts->restore_head_target);
>   	free(opts->reflog_action);
>   	free(opts->default_strategy);
>   	free(opts->strategy);
> @@ -3142,6 +3143,8 @@ static int populate_opts_cb(const char *key, const char *value,
>   	} else if (!strcmp(key, "options.skip-commit-summary")) {
>   		opts->skip_commit_summary =
>   			git_config_bool_or_int(key, value, ctx->kvi, &error_flag);
> +	} else if (!strcmp(key, "options.restore-head-target")) {
> +		git_config_string_dup(&opts->restore_head_target, key, value);
>   	} else {
>   		return error(_("invalid key: %s"), key);
>   	}
> @@ -3709,6 +3712,10 @@ static int save_opts(struct replay_opts *opts)
>   	if (opts->skip_commit_summary)
>   		res |= repo_config_set_in_file_gently(the_repository, opts_file,
>   					"options.skip-commit-summary", NULL, "true");
> +	if (opts->restore_head_target)
> +		res |= repo_config_set_in_file_gently(the_repository, opts_file,
> +				"options.restore-head-target", NULL, opts->restore_head_target);
> +
>   	return res;
>   }
>   
> @@ -5177,6 +5184,23 @@ static int pick_commits(struct repository *r,
>   			return -1;
>   	}
>   
> +	if (opts->restore_head_target) {
> +		struct reset_head_opts reset_opts = { 0 };
> +		const char *msg;
> +
> +		msg = reflog_message(opts, "finish", "returning to %s", opts->restore_head_target);
> +
> +		reset_opts.branch = opts->restore_head_target;
> +		reset_opts.flags = RESET_HEAD_REFS_ONLY;
> +		reset_opts.branch_msg = msg;
> +		reset_opts.head_msg = msg;
> +
> +		if (reset_head(r, &reset_opts)) {
> +			error(_("could not switch HEAD back to %s"), opts->restore_head_target);
> +			return -1;
> +		}
> +	}
> +
>   	/*
>   	 * Sequence of picks finished successfully; cleanup by
>   	 * removing the .git/sequencer directory
> @@ -5533,7 +5557,8 @@ int sequencer_pick_revisions(struct repository *r,
>   	if (opts->revs->cmdline.nr == 1 &&
>   	    opts->revs->cmdline.rev->whence == REV_CMD_REV &&
>   	    opts->revs->no_walk &&
> -	    !opts->revs->cmdline.rev->flags) {
> +	    !opts->revs->cmdline.rev->flags &&
> +	    !opts->restore_head_target) {
>   		struct commit *cmit;
>   
>   		if (prepare_revision_walk(opts->revs)) {
> diff --git a/sequencer.h b/sequencer.h
> index 1767fd737e..a905f6afc7 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -72,6 +72,9 @@ struct replay_opts {
>   	/* Reflog */
>   	char *reflog_action;
>   
> +	/* Reference to which HEAD shall be reset to after the operation. */
> +	char *restore_head_target;
> +
>   	/* placeholder commit for -i --root */
>   	struct object_id squash_onto;
>   	int have_squash_onto;
>