Re: [PATCH v1 5/5] sequencer: directly call pick_commits() from complete_action()
- From
Alban Gruin <alban.gruin@gmail.com>
- Date
- Oct 26, 2019, 11:27 UTC
- Message-ID
- <9b81e879-091b-bbf7-2d1b-51236d19ee9a@gmail.com>
- In-Reply-To
- <67c98856-55dc-df95-c4a6-61f1af85c480@web.de>
Hi René,
Le 26/10/2019 à 09:47, René Scharfe a écrit :
Show 46 quoted lines
> Am 25.09.19 um 22:13 schrieb Alban Gruin:
>> Currently, complete_action() calls sequencer_continue() to do the
>> rebase. Even though the former already has the todo list, the latter
>> loads it from the disk and parses it. Calling directly pick_commits()
>> from complete_action() avoids this unnecessary round trip.
>>
>> Signed-off-by: Alban Gruin <alban.gruin@gmail.com>
>> ---
>> sequencer.c | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/sequencer.c b/sequencer.c
>> index ec7ea8d9e5..b395dd6e11 100644
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> @@ -5140,15 +5140,17 @@ 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);
>> -
>
> Moving this call down leaks new_todo on error...
>
>> if (checkout_onto(r, opts, onto_name, &oid, orig_head))
>> return -1;
>
> ... here ...
>
>>
>> if (require_clean_work_tree(r, "rebase", "", 1, 1))
>> return -1;
> ... and here. Perhaps set res to -1 and jump to the end?
>
>>
>> - return sequencer_continue(r, opts);
>> + todo_list_write_total_nr(&new_todo);
>> + res = pick_commits(r, &new_todo, opts);
>> + todo_list_release(&new_todo);
>> +
>> + return res;
>> }
>>
>> struct subject2item_entry {
>>
> Thank you for the heads up.
Junio, how do you want me to fix that? Should I reroll the series altogether, send a "fixup!" commit, or send a standalone patch?
Cheers, Alban