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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Sep 9, 2016, 15:12 UTC
Message-ID: <alpine.DEB.2.20.1609091708480.129229@virtualbox>
In-Reply-To: <xmqqeg53wj7a.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Thu, 1 Sep 2016, Junio C Hamano wrote:

Show 16 quoted lines

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> 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.
> >
> > These considerations are pretty hypothetical. I would even place a bet
> > that we will *never* have ":1" as names, not if I have anything to say...
> > ;-)
> 
> If you can always work with pre-existing commit, then you can
> validate all object references that appear in the instructions
> upfront.

Or if *some* of the commands work with pre-existing commits, *those* commands can be validated up-front.

Which is exactly what my code does.

Show 7 quoted lines

> I was sort of expecting that, when you do the preserve-merges mode
> of "rebase -i", you would need to jump around, doing "we have
> reconstructed the side branch on a new 'onto', let's give the result
> this temporary name ':1', and then switch to the trunk (which would
> call for 'reset <commit>' instruction) and merge that thing (which
> would be 'merge :1' or perhaps called 'pick :1')", and at that point
> you no longer validate the object references upfront.

Except that is not how --preserve-merges works: it *still* uses the SHA-1s as identifiers, even when the SHA-1 may have changed in the meantime.

That is part of why it was a bad design.

> If you do not have to have such a "mark this point" and a "refer to
> that point we previously marked", then I agree that you should be
> able to pre-validate and keep the result in the structure.

Even then, those markers should *still* be validated. They, too, need to be created and later used, usage before creation would be an error.

But...

1) this is not yet a problem, so why are we discussing it here? Do we not
   have actual problems with these patches to discuss anymore?

2) the SHA-1s that *can* be validated *should* be validated, so I find the
   objection a little bogus.

Ciao, Dscho