From 8638114e063f60fef678d51ca904da7c4e1ab3c0 Mon Sep 17 00:00:00 2001 From: Alban Gruin Date: Sun, 24 Nov 2019 18:43:28 +0100 Subject: [PATCH 1/5] sequencer: update `total_nr' when adding an item to a todo list `total_nr' is the total number of items, counting both done and todo, that are in a todo list. But unlike `nr', it was not updated when an item was appended to the list. This variable is mostly used by command prompts (ie. git-prompt.sh and the like). By forgetting to update it, the original code made it not reflect the reality, but this flaw was masked by the code calling unnecessarily read_populate_todo() again to update the variable to its correct value. At the end of this series, the unnecessary call will be removed, and the inconsistency addressed by this patch would start to matter. Signed-off-by: Alban Gruin Signed-off-by: Junio C Hamano --- sequencer.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sequencer.c b/sequencer.c index d648aaf416..575b852a5a 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2070,6 +2070,7 @@ 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++; } From 34065541e3f99ce23ac431032daf9d72072e650b Mon Sep 17 00:00:00 2001 From: Alban Gruin Date: Sun, 24 Nov 2019 18:43:29 +0100 Subject: [PATCH 2/5] sequencer: update `done_nr' when skipping commands in a todo list In a todo list, `done_nr' is the number of commands that were executed or skipped, but skip_unnecessary_picks() did not update it. This variable is mostly used by command prompts (ie. git-prompt.sh and the like). As in the previous commit, this inconsistent behaviour is not a problem yet, but it would start to matter at the end of this series the same reason. Signed-off-by: Alban Gruin Signed-off-by: Junio C Hamano --- sequencer.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sequencer.c b/sequencer.c index 575b852a5a..42313f8de6 100644 --- a/sequencer.c +++ b/sequencer.c @@ -5054,6 +5054,7 @@ static int skip_unnecessary_picks(struct repository *r, MOVE_ARRAY(todo_list->items, todo_list->items + i, todo_list->nr - i); todo_list->nr -= i; todo_list->current = 0; + todo_list->done_nr += i; if (is_fixup(peek_command(todo_list, 0))) record_in_rewritten(base_oid, peek_command(todo_list, 0)); From 3f34f2d8a4da82ddda48a591cbb091f24a5f3e58 Mon Sep 17 00:00:00 2001 From: Alban Gruin Date: Sun, 24 Nov 2019 18:43:30 +0100 Subject: [PATCH 3/5] sequencer: move the code writing total_nr on the disk to a new function The total number of commands can be used to show the progression of the rebasing in a shell. It is written to the disk by read_populate_todo() when the todo list is loaded from sequencer_continue() or pick_commits(), but not by complete_action(). This moves the part writing total_nr to a new function so it can be called from complete_action(). Signed-off-by: Alban Gruin Signed-off-by: Junio C Hamano --- sequencer.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/sequencer.c b/sequencer.c index 42313f8de6..ec7ea8d9e5 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2342,6 +2342,16 @@ void sequencer_post_commit_cleanup(struct repository *r, int verbose) sequencer_remove_state(&opts); } +static void todo_list_write_total_nr(struct todo_list *todo_list) +{ + FILE *f = fopen_or_warn(rebase_path_msgtotal(), "w"); + + if (f) { + fprintf(f, "%d\n", todo_list->total_nr); + fclose(f); + } +} + static int read_populate_todo(struct repository *r, struct todo_list *todo_list, struct replay_opts *opts) @@ -2387,7 +2397,6 @@ static int read_populate_todo(struct repository *r, if (is_rebase_i(opts)) { struct todo_list done = TODO_LIST_INIT; - FILE *f = fopen_or_warn(rebase_path_msgtotal(), "w"); if (strbuf_read_file(&done.buf, rebase_path_done(), 0) > 0 && !todo_list_parse_insn_buffer(r, done.buf.buf, &done)) @@ -2399,10 +2408,7 @@ static int read_populate_todo(struct repository *r, + count_commands(todo_list); todo_list_release(&done); - if (f) { - fprintf(f, "%d\n", todo_list->total_nr); - fclose(f); - } + todo_list_write_total_nr(todo_list); } return 0; From a2dd67f10523d3593f404b9889b6318f75ab988c Mon Sep 17 00:00:00 2001 From: Alban Gruin Date: Sun, 24 Nov 2019 18:43:31 +0100 Subject: [PATCH 4/5] rebase: fill `squash_onto' in get_replay_opts() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When sequencer_continue() is called by complete_action(), `opts' has been filled by get_replay_opts(). Currently, it does not initialise the `squash_onto' field (used by the `--root' mode), only read_populate_opts() does. It’s not a problem yet since sequencer_continue() calls it before pick_commits(), but it would lead to incorrect results once complete_action() is modified to call pick_commits() directly. Let’s change that. Signed-off-by: Alban Gruin Signed-off-by: Junio C Hamano --- builtin/rebase.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/builtin/rebase.c b/builtin/rebase.c index 4a20582e72..b171c86e3d 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -117,6 +117,11 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts) if (opts->strategy_opts) parse_strategy_opts(&replay, opts->strategy_opts); + if (opts->squash_onto) { + oidcpy(&replay.squash_onto, opts->squash_onto); + replay.have_squash_onto = 1; + } + return replay; } From 393adf7a6f600adca8cb75ec4e7136d523e8840d Mon Sep 17 00:00:00 2001 From: Alban Gruin Date: Sun, 24 Nov 2019 18:43:32 +0100 Subject: [PATCH 5/5] sequencer: directly call pick_commits() from complete_action() Currently, complete_action(), used by builtin/rebase.c to start a new rebase, calls sequencer_continue() to do it. Before the former calls pick_commits(), it - calls read_and_refresh_cache() -- this is unnecessary here as we've just called require_clean_work_tree() in complete_action() - calls read_populate_opts() -- this is unnecessary as we're starting a new rebase, so `opts' is fully populated - loads the todo list -- this is unnecessary as we've just populated the todo list in complete_action() - commits any staged changes -- this is unnecessary as we're starting a new rebase, so there are no staged changes - calls record_in_rewritten() -- this is unnecessary as we're starting a new rebase. This changes complete_action() to directly call pick_commits() to avoid these unnecessary steps. Signed-off-by: Alban Gruin Signed-off-by: Junio C Hamano --- sequencer.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/sequencer.c b/sequencer.c index ec7ea8d9e5..ec0b793fc5 100644 --- a/sequencer.c +++ b/sequencer.c @@ -5140,15 +5140,21 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla return error_errno(_("could not write '%s'"), todo_file); } - todo_list_release(&new_todo); + res = -1; if (checkout_onto(r, opts, onto_name, &oid, orig_head)) - return -1; + goto cleanup; if (require_clean_work_tree(r, "rebase", "", 1, 1)) - return -1; + goto cleanup; - return sequencer_continue(r, opts); + todo_list_write_total_nr(&new_todo); + res = pick_commits(r, &new_todo, opts); + +cleanup: + todo_list_release(&new_todo); + + return res; } struct subject2item_entry {