All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Tan <pyokagan@gmail.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Git List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>,
	Duy Nguyen <pclouds@gmail.com>,
	Stefan Beller <sbeller@google.com>,
	Sam Halliday <sam.halliday@gmail.com>
Subject: Re: [PATCH/RFC/GSoC 17/17] rebase-interactive: introduce interactive backend for builtin rebase
Date: Wed, 16 Mar 2016 00:48:27 +0800	[thread overview]
Message-ID: <CACRoPnRhhMM0e3S23KVnEANwNRDPq0P3hSqn5Zs1ksZxeaAoiA@mail.gmail.com> (raw)
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:
> 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 :-).

> 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.

> 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 :-).

>
> 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?

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

Regards,
Paul

  reply	other threads:[~2016-03-15 16:48 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-12 10:46 [PATCH/RFC/GSoC 00/17] A barebones git-rebase in C Paul Tan
2016-03-12 10:46 ` [PATCH/RFC/GSoC 01/17] perf: introduce performance tests for git-rebase Paul Tan
2016-03-16  7:58   ` Johannes Schindelin
2016-03-16 11:51     ` Paul Tan
2016-03-16 15:59       ` Johannes Schindelin
2016-03-18 11:01         ` Thomas Gummerer
2016-03-18 16:00           ` Johannes Schindelin
2016-03-20 14:00             ` Thomas Gummerer
2016-03-21  7:54               ` Johannes Schindelin
2016-03-12 10:46 ` [PATCH/RFC/GSoC 02/17] sha1_name: implement get_oid() and friends Paul Tan
2016-03-12 10:46 ` [PATCH/RFC/GSoC 03/17] builtin-rebase: implement skeletal builtin rebase Paul Tan
2016-03-14 18:31   ` Stefan Beller
2016-03-15  8:01     ` Johannes Schindelin
2016-03-12 10:46 ` [PATCH/RFC/GSoC 04/17] builtin-rebase: parse rebase arguments into a common rebase_options struct Paul Tan
2016-03-14 20:05   ` Stefan Beller
2016-03-15 10:54   ` Johannes Schindelin
2016-03-12 10:46 ` [PATCH/RFC/GSoC 05/17] rebase-options: implement rebase_options_load() and rebase_options_save() Paul Tan
2016-03-14 20:30   ` Stefan Beller
2016-03-16  8:04     ` Johannes Schindelin
2016-03-16 12:28       ` Paul Tan
2016-03-16 17:11         ` Johannes Schindelin
2016-03-21 14:55           ` Paul Tan
2016-03-16 12:04     ` Paul Tan
2016-03-16 17:10       ` Stefan Beller
2016-03-12 10:46 ` [PATCH/RFC/GSoC 06/17] rebase-am: introduce am backend for builtin rebase Paul Tan
2016-03-16 13:21   ` Johannes Schindelin
2016-03-12 10:46 ` [PATCH/RFC/GSoC 07/17] rebase-common: implement refresh_and_write_cache() Paul Tan
2016-03-14 21:10   ` Junio C Hamano
2016-03-16 12:56     ` Paul Tan
2016-03-12 10:46 ` [PATCH/RFC/GSoC 08/17] rebase-common: let refresh_and_write_cache() take a flags argument Paul Tan
2016-03-12 10:46 ` [PATCH/RFC/GSoC 09/17] rebase-common: implement cache_has_unstaged_changes() Paul Tan
2016-03-14 20:54   ` Johannes Schindelin
2016-03-14 21:52     ` Junio C Hamano
2016-03-15 11:51       ` Johannes Schindelin
2016-03-15 11:07     ` Duy Nguyen
2016-03-15 14:15       ` Johannes Schindelin
2016-03-12 10:46 ` [PATCH/RFC/GSoC 10/17] rebase-common: implement cache_has_uncommitted_changes() Paul Tan
2016-03-12 10:46 ` [PATCH/RFC/GSoC 11/17] rebase-merge: introduce merge backend for builtin rebase Paul Tan
2016-03-12 10:46 ` [PATCH/RFC/GSoC 12/17] rebase-todo: introduce rebase_todo_item Paul Tan
2016-03-14 13:43   ` Christian Couder
2016-03-14 20:33     ` Johannes Schindelin
2016-03-16 12:54     ` Paul Tan
2016-03-16 15:55       ` Johannes Schindelin
2016-03-12 10:46 ` [PATCH/RFC/GSoC 13/17] rebase-todo: introduce rebase_todo_list Paul Tan
2016-03-12 10:46 ` [PATCH/RFC/GSoC 14/17] status: use rebase_todo_list Paul Tan
2016-03-12 10:46 ` [PATCH/RFC/GSoC 15/17] wrapper: implement append_file() Paul Tan
2016-03-12 10:46 ` [PATCH/RFC/GSoC 16/17] editor: implement git_sequence_editor() and launch_sequence_editor() Paul Tan
2016-03-15  7:00   ` Johannes Schindelin
2016-03-16 13:06     ` Paul Tan
2016-03-16 18:21       ` Johannes Schindelin
2016-03-12 10:46 ` [PATCH/RFC/GSoC 17/17] rebase-interactive: introduce interactive backend for builtin rebase Paul Tan
2016-03-15  7:57   ` Johannes Schindelin
2016-03-15 16:48     ` Paul Tan [this message]
2016-03-15 19:45       ` Johannes Schindelin
2016-03-14 12:15 ` [PATCH/RFC/GSoC 00/17] A barebones git-rebase in C Duy Nguyen
2016-03-14 17:32   ` Stefan Beller
2016-03-14 18:43   ` Junio C Hamano
2016-03-16 12:46     ` Paul Tan
2016-03-14 20:44   ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CACRoPnRhhMM0e3S23KVnEANwNRDPq0P3hSqn5Zs1ksZxeaAoiA@mail.gmail.com \
    --to=pyokagan@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=sam.halliday@gmail.com \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.