Re: [PATCH/RFC/GSoC 17/17] rebase-interactive: introduce interactive backend for builtin rebase

From: Paul Tan <pyokagan@gmail.com>
Date: Mar 15, 2016, 16:48 UTC
Message-ID: <CACRoPnRhhMM0e3S23KVnEANwNRDPq0P3hSqn5Zs1ksZxeaAoiA@mail.gmail.com>
In-Reply-To: <alpine.DEB.2.20.1603150800420.4690@virtualbox>

Hi Dscho,

On Tue, Mar 15, 2016 at 3:57 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 17 quoted lines

> On Sat, 12 Mar 2016, Paul Tan wrote:
>
>> Since 1b1dce4 (Teach rebase an interactive mode, 2007-06-25), git-rebase
>> supports an interactive mode when passed the -i switch.
>>
>> In interactive mode, git-rebase allows users to edit the list of patches
>> (using the user's GIT_SEQUENCE_EDITOR), so that the user can reorder,
>> edit and delete patches.
>>
>> Re-implement a skeletal version of the above feature by introducing a
>> rebase-interactive backend for our builtin-rebase. This skeletal
>> implementation is only able to pick and re-order commits.
>>
>> Signed-off-by: Paul Tan <pyokagan@gmail.com>
>
> It is a pity that both of us worked on overlapping projects in stealth
> mode. Inevitably, some of the work is now wasted :-(

No worries, I did this series for my own interest, especially to get a gauge of the speedup between rebase in shell and C. GSoC applications have opened and will close in 10 days time, so I wanted to get some data before the deadline at least :-).

Show 5 quoted lines

> Not all is lost, though.
>
> Much of the code can be salvaged, although I really want to reiterate
> that an all-or-nothing conversion of the rebase command is not going to
> fly.

Sure. I admit that I concentrated more on how the "final code" would look like, and not so much how the rewrite would be built upon in pieces.

Show 21 quoted lines

> For several reasons: it would be rather disruptive, huge and hard to
> review. It would not let anybody else work on that huge task. And you're
> prone to fall behind due to Git's source code being in constant flux
> (including the rebase bits).
>
> There is another, really important reason: if you package the conversion
> into small, neat bundles, it is much easier to avoid too narrow a focus
> that would tuck perfectly useful functions away in a location where it
> cannot be reused and where it is likely to be missed by other developers
> who need the same, or similar functionality (point in case:
> has_uncommitted_changes()). And we know that this happened in the past,
> and sometimes resulted in near-duplicated code, hence Karthik's Herculean,
> still ongoing work.
>
> Lastly, I need to point out that the conversion of rebase into a builtin
> is not the end game, it is the game's opening.
>
> [...]
>
> So you see, there was a much larger master plan behind my recommendation
> to go the rebase--helper route.

Ah I see, thanks for publishing your branch and sharing your plans.

Originally I was thinking smaller -- rewrite git-rebase first, following its shell script closely, and then doing the libification and optimization after that. However, I see now that you have grander plans than that :-).

Show 13 quoted lines

>
> As to my current state: Junio put me into quite a fix (without knowing it)
> by releasing 2.7.3 just after I took off for an extended offline weekend,
> and now I am scrambling because a change in MSYS2's runtime (actually,
> probably more like: an update of the GCC that is used to compile the
> runtime, that now causes a regression) is keeping me away from my work on
> the interactive rebase. Even so, I am pretty far along; There are only
> three major things left to do: 1) fix fixups/squashes with fast-forwarding
> picks, 2) implement 'reword', 3) display the progress.  And of course 4)
> clean up the fallout. ;-)
>
> At this point, I'd rather finish this myself than falling prey to Brooks'
> Law.

Okay, I won't touch interactive rebase then.

> I also have to admit that I would love to give you a project over the
> summer whose logical children are exciting enough to dabble with even
> during the winter. And somehow I do not see that excitement in the boring
> conversion from shell to C (even if its outcome is well-needed).

Well, that is subjective ;-).

Even with interactive rebase out-of-bounds, I don't think it's a dead end though:

1. git-rebase--am.sh, git-rebase--merge.sh and git-rebase.sh can be
rewritten to C, and call git-rebase--interactive.sh externally, like
what Duy demonstrated in his patch series. The timings show that am
and merge rebase still benefit, and that way we will be closer to a
git-rebase in full C.

2. git-commit can be libified, so that we can access its functionality
directly. (sequencer.c runs it once per commit, rebase-interactive
uses it for squashes etc.)

Or would that be stepping on your toes?

Show 5 quoted lines

> Ciao,
> Dscho
>
> Footnote *1*:
> https://github.com/git-for-windows/build-extra/blob/master/shears.sh

Regards, Paul