On 01/08/2023 18:22, Junio C Hamano wrote:
"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.
The files will never exist when the "if TODO_BREAK" is executed because
we've removed them before entering the loop and as I tried and seemly
failed to explain in the commit message they are only created when
we're about to break out of the loop.
Best Wishes
Phillip
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);
