"Phillip Wood via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> From: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
>
> At the start of each iteration the loop that picks commits removes
> state files from the previous pick. However some of these are only
> written if there are conflicts and so we break out of the loop after
> writing them. Therefore they only need to be removed when the rebase
> continues, not in each iteration.
"They only need to be removed before the loop" assumes that the SOLE
purpose of the removal is to give the next iteration a clean slate
to work with, but is that really the case? The original unlink's is
followed by "if TODO_BREAK, break out of the loop", presumably to
give control back to the end-user. So three files that were not
available to the user after "break" are now suddenly visible to
them.
Perhaps that is the effect the series wanted to have. Or it could
be an unintended side effect that may be a regression. Or perhaps
the visibility of these three files (but not others?) is considered
an implementation detail no users should ever depend on.
It is hard to tell from the above description and the patch text
which is the case. Care to enlighten?
Thanks.
> Signed-off-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
> ---
> sequencer.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index cc9821ece2c..de66bda9d5b 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -4656,6 +4656,10 @@ static int pick_commits(struct repository *r,
> if (read_and_refresh_cache(r, opts))
> return -1;
>
> + unlink(rebase_path_message());
> + unlink(rebase_path_stopped_sha());
> + unlink(rebase_path_amend());
> +
> while (todo_list->current < todo_list->nr) {
> struct todo_item *item = todo_list->items + todo_list->current;
> const char *arg = todo_item_get_arg(todo_list, item);
> @@ -4679,10 +4683,7 @@ static int pick_commits(struct repository *r,
> todo_list->total_nr,
> opts->verbose ? "\n" : "\r");
> }
> - unlink(rebase_path_message());
> unlink(rebase_path_author_script());
> - unlink(rebase_path_stopped_sha());
> - unlink(rebase_path_amend());
> unlink(git_path_merge_head(r));
> unlink(git_path_auto_merge(r));
> delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
