Re: [PATCH 13/22] sequencer: remember the onelines when parsing the todo file

From: Jakub Narębski <jnareb@gmail.com>
Date: Aug 31, 2016, 19:24 UTC
Message-ID: <a9831d93-f5b4-d729-eae0-1f7c1123a6a6@gmail.com>
In-Reply-To: <xmqq8tvc21re.fsf@gitster.mtv.corp.google.com>

W dniu 31.08.2016 o 21:10, Junio C Hamano pisze:

Show 12 quoted lines

> Jakub Narębski <jnareb@gmail.com> writes:
> 
>>> diff --git a/sequencer.c b/sequencer.c
>>> index 06759d4..3398774 100644
>>> --- a/sequencer.c
>>> +++ b/sequencer.c
>>> @@ -709,6 +709,8 @@ static int read_and_refresh_cache(struct replay_opts *opts)
>>>  struct todo_item {
>>>  	enum todo_command command;
>>>  	struct commit *commit;
>>> +	const char *arg;
>>> +	int arg_len;

> I am not sure what the "commit" field of type "struct commit *" is
> for.  It is not needed until it is the commit's turn to be picked or
> reverted; if we end up stopping in the middle, parsing the commit
> object for later steps will end up being wasted effort.

From what I understand this was what sequencer did before this series, so it is not a regression (I think; the commit parsing was in different function, but I think at the same place in the callchain).

> 
> Also, when the sequencer becomes one sequencer to rule them all, the
> command set may contain something that does not even mention any
> commit at all (think: exec).

The "exec" line is a bit of exception, all other rebase -i commands take commit as parameter. It could always use NULL.

Show 9 quoted lines

>
> So I am not sure if we want a parsed commit there (I would not
> object if we kept the texual object name read from the file,
> though).  The "one sequencer to rule them all" may even have to say
> "now give name ':1' to the result of the previous operation" in one
> step and in another later step have an instruction "merge ':1'".
> When that happens, you cannot even pre-populate the commit object
> when the sequencer reads the file, as the commit has not yet been
> created at that point.

True, --preserve-merges rebase is well, different.

Best,

-- 
Jakub Narebski