I recently picked up work where I regularly rebase large branch thickets consisting of thousands of commits. During those rebases, I could not fail to notice that the progress initially showed a total number around 2,100, when the actual number of commands was more like 1,850. And indeed, when resuming the rebase after being interrupted due to a break command or due to a merge conflict, the progress showed the correct number! So I set out to fix this, stumbling over an incorrect use of total_nr in the --update-refs code, so I fixed that, too. Note: These patches apply cleanly on top of ds/rebase-update-ref as well as on top of the current main branch. Changes since v1: * Clarified the pattern expected by the test case in the progress output, as suggested by Junio. * Simplified the code as suggested by Phillipp, based on the insight that complete_action() (naming is hard!) is only called at the very beginning of a rebase, and therefore there cannot be any already-done commands in the todo list. Johannes Schindelin (2): rebase --update-refs: fix loops rebase -r: fix the total number shown in the progress sequencer.c | 13 ++++++++----- t/t3430-rebase-merges.sh | 8 ++++++++ 2 files changed, 16 insertions(+), 5 deletions(-) base-commit: 4611884ea883908a9638cafbd824c401c41cf7f6 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1531%2Fdscho%2Ffix-rebase-i-progress-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1531/dscho/fix-rebase-i-progress-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/1531 Range-diff vs v1: 1: 2ac7c7a7c61 = 1: 2ac7c7a7c61 rebase --update-refs: fix loops 2: d12d5469f8c ! 2: cac809bcffd rebase -r: fix the total number shown in the progress @@ Commit message Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx> ## sequencer.c ## +@@ sequencer.c: 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++; + } + @@ sequencer.c: 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()); @@ sequencer.c: int todo_list_parse_insn_buffer(struct repository *r, char *buf, for (i = 1; *p; i++, p = next_p) { char *eol = strchrnul(p, '\n'); @@ sequencer.c: int todo_list_parse_insn_buffer(struct repository *r, char *buf, - item->arg_offset = p - buf; - item->arg_len = (int)(eol - p); item->commit = NULL; -- } -+ } else if (item->command == TODO_COMMENT) -+ todo_list->total_nr--; + } ++ if (item->command != TODO_COMMENT) ++ todo_list->total_nr++; ++ if (fixup_okay) ; /* do nothing */ + else if (is_fixup(item->command)) @@ sequencer.c: int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla - struct todo_list new_todo = TODO_LIST_INIT; - struct strbuf *buf = &todo_list->buf, buf2 = STRBUF_INIT; - struct object_id oid = onto->object.oid; -- int res; -+ int new_total_nr, res; - - find_unique_abbrev_r(shortonto, &oid, DEFAULT_ABBREV); - -@@ sequencer.c: int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla - return error(_("nothing to do")); - } - -+ new_total_nr = todo_list->total_nr - count_commands(todo_list); - res = edit_todo_list(r, todo_list, &new_todo, shortrevisions, - shortonto, flags); - if (res == -1) -@@ sequencer.c: int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla - return -1; - } - -+ new_total_nr += count_commands(&new_todo); -+ new_todo.total_nr = new_total_nr; -+ - /* Expand the commit IDs */ 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); @@ t/t3430-rebase-merges.sh: test_expect_success '--rebase-merges with message matc +test_expect_success 'progress shows the correct total' ' + git checkout -b progress H && + git rebase --rebase-merges --force-rebase --verbose A 2> err && -+ grep "^Rebasing.*14.$" err >progress && ++ # Expecting "Rebasing (N/14)" here, no bogus total number ++ grep "^Rebasing.*/14.$" err >progress && + test_line_count = 14 progress +' + -- gitgitgadget
