[PATCH v3 0/5] Use complete_action's todo list to do the rebase

This can be seen as a continuation of ag/reduce-rewriting-todo.

Currently, complete_action() releases its todo list before calling sequencer_continue(), which reloads the todo list from the disk. This series removes this useless round trip.

Patches 1, 2, and 3 originally come from a series meaning to improve rebase.missingCommitsCheck[0]. In the original series, I wanted to check for missing commits in read_populate_todo(), so a warning could be issued after a `rebase --continue' or an `exec' commands. But, in the case of the initial edit, it is already checked in complete_action(), and would be checked a second time in sequencer_continue() (a caller of read_populate_todo()). So I hacked up sequencer_continue() to accept a pointer to a todo list, and if not null, would skip the call to read_populate_todo(). (This was really ugly, to be honest.) Some issues arose with git-prompt.sh[1], hence 1, 2 and 3.

Patch 5 is a new approach to what I did first. Instead of bolting a new parameter to sequencer_continue(), this makes complete_action() calling directly pick_commits().

This is based on 4c86140027 ("Third batch").

Changes since v2:

 - Patch 1 has been reworded to fix a mistake in read_populate_todo()'s
   name, reported by Jonathan Tan.
 - Patches 4 and 5 has been reworded to improve descriptions of the code
   paths, according to comments made by Jonathan Tan.
 - Squashed b0537b0ec3 ("SQUASH??? tentative leakfix") into the 5th
   patch to fix a memory leak reported by René Sharfe.

The tip of this series is tagged as "reduce-todo-list-cont-v3" at https://github.com/agrn/git.

[0] http://public-inbox.org/git/20190717143918.7406-1-alban.gruin@gmail.com/ [1] http://public-inbox.org/git/1732521.CJWHkCQAay@andromeda/

Alban Gruin (5):
  sequencer: update `total_nr' when adding an item to a todo list
  sequencer: update `done_nr' when skipping commands in a todo list
  sequencer: move the code writing total_nr on the disk to a new
    function
  rebase: fill `squash_onto' in get_replay_opts()
  sequencer: directly call pick_commits() from complete_action()

 builtin/rebase.c |  5 +++++
 sequencer.c      | 32 +++++++++++++++++++++++---------
 2 files changed, 28 insertions(+), 9 deletions(-)

Diff-intervalle contre v2 :
1:  9215b191c7 ! 1:  11a221e82e sequencer: update `total_nr' when adding an item to a todo list
    @@ Commit message
         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_todo_list() again to update the variable to its
    +    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.
2:  7cad541092 = 2:  76a3af70b6 sequencer: update `done_nr' when skipping commands in a todo list
3:  7c9c4ddd30 = 3:  9c5bd30465 sequencer: move the code writing total_nr on the disk to a new function
4:  cd44fb4e10 ! 4:  bc3d69a10e rebase: fill `squash_onto' in get_replay_opts()
    @@ Metadata
      ## Commit message ##
         rebase: fill `squash_onto' in get_replay_opts()
     
    -    Currently, the get_replay_opts() function does not initialise the
    -    `squash_onto' field (which is used for the `--root' mode), only
    -    read_populate_opts() does.  That would lead to incorrect results when
    -    calling pick_commits() without reading the options from the disk first.
    +    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.
     
5:  523fdd35a1 ! 5:  e7691db66b sequencer: directly call pick_commits() from complete_action()
    @@ Metadata
      ## Commit message ##
         sequencer: directly call pick_commits() from complete_action()
     
    -    Currently, complete_action() calls sequencer_continue() to do the
    -    rebase.  Before the former calls pick_commits(), it
    +    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()
    +       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
    +       new rebase, so `opts' is fully populated
          - loads the todo list -- this is unnecessary as we've just populated
    -       the todo list
    +       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
    @@ sequencer.c: int complete_action(struct repository *r, struct replay_opts *opts,
      	}
      
     -	todo_list_release(&new_todo);
    --
    ++	res = -1;
    + 
      	if (checkout_onto(r, opts, onto_name, &oid, orig_head))
    - 		return -1;
    +-		return -1;
    ++		goto cleanup;
      
      	if (require_clean_work_tree(r, "rebase", "", 1, 1))
    - 		return -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;