From fa5103dd8913366b031ad8daa11f27e9452638be Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 13 May 2023 08:11:25 +0000 Subject: [PATCH 1/2] rebase --update-refs: fix loops The `total_nr` field in the `todo_list` structure merely serves display purposes, and should only be used when generating the progress message. In these two instances, however, we want to loop over all of the commands in the parsed rebase script. The loop limit therefore needs to be `nr`, which refers to the count of commands in the current `todo_list`. This is important because the two numbers, `nr` and `total_nr` can differ wildly, e.g. due to `total_nr` _not_ counting comments or empty lines, while `nr` skips any commands that already moved from the `git-rebase-todo` file to the `done` file. Signed-off-by: Johannes Schindelin Acked-by: Phillip Wood Signed-off-by: Junio C Hamano --- sequencer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sequencer.c b/sequencer.c index 0cf3842201a..0101a727012 100644 --- a/sequencer.c +++ b/sequencer.c @@ -4205,7 +4205,7 @@ void todo_list_filter_update_refs(struct repository *r, if (!is_null_oid(&rec->after)) continue; - for (j = 0; !found && j < todo_list->total_nr; j++) { + for (j = 0; !found && j < todo_list->nr; j++) { struct todo_item *item = &todo_list->items[j]; const char *arg = todo_list->buf.buf + item->arg_offset; @@ -4235,7 +4235,7 @@ void todo_list_filter_update_refs(struct repository *r, * For each todo_item, check if its ref is in the update_refs list. * If not, then add it as an un-updated ref. */ - for (i = 0; i < todo_list->total_nr; i++) { + for (i = 0; i < todo_list->nr; i++) { struct todo_item *item = &todo_list->items[i]; const char *arg = todo_list->buf.buf + item->arg_offset; int j, found = 0; From 170eea9750e1bca7a35b5d27def9ee77df92fdf1 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 13 May 2023 08:11:26 +0000 Subject: [PATCH 2/2] rebase -r: fix the total number shown in the progress For regular, non-`--rebase-merges` runs, there is very little work to do for the parser when determining the total number of commands in a rebase script: it is simply the number of lines after stripping the commented lines and then trimming the trailing empty line, if any. The `--rebase-merges` mode complicates things by introducing empty lines and comments in the middle of the script. These should _not_ be counted as commands, and indeed, when an interactive rebase is interrupted and subsequently resumed, the total number of commands can magically shrink, sometimes dramatically. The reason for this strange behavior is that empty lines _are_ counted in `edit_todo_list()` (but not the comments, as they are stripped via `strbuf_stripspace(..., 1)`, which is a bug. Let's fix this so that the correct total number is shown from the get-go, by carefully adjusting it according to what's in the rebase script. Extra care needs to be taken in case the user edits the script: the number of commands might be different after the user edited than beforehand. Note: Even though commented lines are skipped in `edit_todo_list()`, we still need to handle `TODO_COMMENT` items by decrementing the already-incremented `total_nr` again: empty lines are also marked as `TODO_COMMENT`. Signed-off-by: Johannes Schindelin Acked-by: Phillip Wood Signed-off-by: Junio C Hamano --- sequencer.c | 9 ++++++--- t/t3430-rebase-merges.sh | 8 ++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/sequencer.c b/sequencer.c index 0101a727012..659d98603e3 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2453,7 +2453,6 @@ void todo_list_release(struct todo_list *todo_list) static struct todo_item *append_new_todo(struct todo_list *todo_list) { ALLOC_GROW(todo_list->items, todo_list->nr + 1, todo_list->alloc); - todo_list->total_nr++; return todo_list->items + todo_list->nr++; } @@ -2609,7 +2608,7 @@ int todo_list_parse_insn_buffer(struct repository *r, char *buf, char *p = buf, *next_p; int i, res = 0, fixup_okay = file_exists(rebase_path_done()); - todo_list->current = todo_list->nr = 0; + todo_list->current = todo_list->nr = todo_list->total_nr = 0; for (i = 1; *p; i++, p = next_p) { char *eol = strchrnul(p, '\n'); @@ -2630,6 +2629,9 @@ int todo_list_parse_insn_buffer(struct repository *r, char *buf, item->commit = NULL; } + if (item->command != TODO_COMMENT) + todo_list->total_nr++; + if (fixup_okay) ; /* do nothing */ else if (is_fixup(item->command)) @@ -6096,7 +6098,8 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla todo_list_to_strbuf(r, &new_todo, &buf2, -1, 0); strbuf_swap(&new_todo.buf, &buf2); strbuf_release(&buf2); - new_todo.total_nr -= new_todo.nr; + /* Nothing is done yet, and we're reparsing, so let's reset the count */ + new_todo.total_nr = 0; if (todo_list_parse_insn_buffer(r, new_todo.buf.buf, &new_todo) < 0) BUG("invalid todo list after expanding IDs:\n%s", new_todo.buf.buf); diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh index f351701fec2..499c31a8d9a 100755 --- a/t/t3430-rebase-merges.sh +++ b/t/t3430-rebase-merges.sh @@ -517,4 +517,12 @@ test_expect_success '--rebase-merges with message matched with onto label' ' EOF ' +test_expect_success 'progress shows the correct total' ' + git checkout -b progress H && + git rebase --rebase-merges --force-rebase --verbose A 2> err && + # Expecting "Rebasing (N/14)" here, no bogus total number + grep "^Rebasing.*/14.$" err >progress && + test_line_count = 14 progress +' + test_done