Re: [RFC PATCH 5/9] sequencer: move the code writing total_nr on the disk to a new function
- From
Alban Gruin <alban.gruin@gmail.com>
- Date
- Jul 19, 2019, 18:14 UTC
- Message-ID
- <6145932.27YfOFK3Rs@andromeda>
- In-Reply-To
- <xmqqk1cfxg6d.fsf@gitster-ct.c.googlers.com>
Hi,
Le 18/07/2019 à 22:04, Junio C Hamano a écrit :
Show 23 quoted lines
> Alban Gruin <alban.gruin@gmail.com> writes: > > The total amount of commands can be used to show the progression of the > > rebasing in a shell. This number 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(), > > which releases its todo list before calling sequencer_continue(), which > > reloads it immediatly afterwards. > > > > To avoid to reload the todo list from the disk, sequencer_continue() > > will be modified to accept a todo list, and if it is not null, > > read_populate_todo() will not be called. > > That may be good as a plan, but readers who are reading this step > are left puzzled as the changes so far do not seem to have much to > do with that change. Perhaps this paragraph belongs to the step > that actually makes that modification? > > > This moves the part writing total_nr to a new function so it can be > > called by complete_action(). > > So without 3/9 and 4/9, we have been simply writing out wrong > number? Good. >
Not here, the numbers computed in read_populate_todo() are correct. But they may be incorrect in complete_action(), before calling sequencer_continue(). This was not a big deal as the todo list was released before and sequencer_continue() re-read it, but it would have became a problem with 6/9.
Show 44 quoted lines
> > Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
> > ---
> >
> > sequencer.c | 16 +++++++++++-----
> > 1 file changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/sequencer.c b/sequencer.c
> > index ec9c3d4dc5..d66b80d49f 100644
> > --- a/sequencer.c
> > +++ b/sequencer.c
> > @@ -2343,6 +2343,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)
> >
> > @@ -2388,7 +2398,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))
Show 16 quoted lines
> >
> > @@ -2400,10 +2409,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;