Re: Rebase sequencer changes prevent exec commands from modifying the todo file?

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 2, 2017, 15:25 UTC
Message-ID: <alpine.DEB.2.20.1703021617510.3767@virtualbox>
In-Reply-To: <CAKNkOnM366uiJKkz31hS8V3NTa8qksP2pXrH4+F-zodZaNdsqg@mail.gmail.com>

Hi Stephen,

On Wed, 1 Mar 2017, Stephen Hicks wrote:

Show 17 quoted lines

> I have a preferred rebase workflow wherein I generate my own TODO file
> with 'exec' lines that further modify the TODO file.  I recently noticed
> that these changes weren't sticking: they seemed to persist until the
> end of the exec'd command, and then as soon as the next command ran
> (i.e.  GIT_EDITOR=cat git rebase --edit-todo) they were gone.
> 
> I don't understand the changes that have been going through, but I suspect
> this is a result of the sequencer refactoring.  Is there a way to prevent
> git rebase from overwriting these changes?
> 
> To reproduce:
> $ git rebase -i HEAD -x "echo x false >> \"$(git rev-parse
> --git-dir)/rebase-merge/git-rebase-todo\""
> 
> This should cause the rebase to fail (and indeed, it does on 2.11.0.390)
> since it inserts an "x false".  But somewhere between there and
> 2.12.0.rc1.440, this behavior is changed.

Do you also modify the author-script file to execute arbitrary shell commands? ;-)

Seriously again, it should not be too much of a deal to handle your use case by re-reading the git-rebase-todo file if it has changed after an `exec` has modified it. We already force a re-read of the index.

I won't be able to take care of that immediately, though, as I have a pressing other patch series I need to get done.

If you want to take a crack at it in the meantime, I think this is where I would start:

https://github.com/git-for-windows/git/blob/ce4c6ca554/sequencer.c#L2020-L2027

I would probably try to make the code smart by looking at the timestamp/size/inode fields of the stat data of git-rebase-todo, and only force a re-read in case those fields are different, the `res` variable is 0, and then the code would need to `continue;` in order to skip the increment of `todo_list->current`.

And before that, I'd turn your example into a test case in t/t3405-rebase-malformed.sh...

Ciao, Johannes