git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [GSoC 2011] Git Sequencer
@ 2011-04-03 17:20 Ramkumar Ramachandra
  2011-04-03 17:24 ` Sverre Rabbelier
                   ` (4 more replies)
  0 siblings, 5 replies; 20+ messages in thread
From: Ramkumar Ramachandra @ 2011-04-03 17:20 UTC (permalink / raw)
  To: Git List
  Cc: Stephen Beyer, Christian Couder, Daniel Barkalow,
	Sverre Rabbelier, Jonathan Nieder

Hi,

I'd like to re-apply this year as a student because I really want to
see (among other things), a sequencer in git.git.  Also, since I
worked on areas related to fast-import and remote helpers last year, I
thought I should work on something completely orthogonal this year.

I now have a draft of my proposal ready, and I'd really appreciate
feedback.  Also, could someone mentor me?

======================================================================
Project Proposal: Git Sequencer
Student: Ramkumar Ramachandra
Mentor: ?

== The Objective ==

To write git-sequencer, a new builtin command, and implement existing
commands on top of that.  This should give the commands more
functionality, improve their error handling, and make them faster.
The project can only be considered successful if all (or most) of the
code written gets merged into upstream.

The Git Sequencer was a 2008 GSoC project as well; unfortunately most
of the code did not get merged into git.git.  The learning from all
that work should serve as a huge headstart this year.

=== The Plan ===

1. Extend 'cherry-pick' with '--continue', '--abort', and '--skip'
features, so that it works like (a subset of) the current
'git-rebase--interactive.sh'.  This will require patching
'builtin/revert.c' in place, and merging it immediately.  I plan to
roughly follow the road laid out by Christian's 2010 series [1].

1.1. Factor out all calls to 'die' with 'return error' so so that we
can pause the entire process when a commit doesn't apply
automatically.

1.2. Create and populate TODO and DONE files, similar to the one that
'git-rebase--interactive.sh' creates.  For now, it should simply give
us information about why a 'cherry-pick' failed.  Use these files with
'git-rebase--interactive.sh' to resume.

1.3. Port selective tests from the current 't3404' to make sure that
TODO and DONE are populated correctly; "stop on conflicting pick" is a
good candidate.

1.4. Decouple the 'revert' functionality from the 'cherry-pick'
functionality in 'revert.c'.  Implement '--abort' for 'cherry-pick'
and port "abort" test from 't3404'.

1.5. Implement parsing the TODO and DONE files into suitable data
structures.  Derive inspiration from the code written in 2008 to do
this.

1.6. Implement '--continue' and '--skip', and write suitable tests.
Merge into upstream.

2. Factor out the 'cherry-pick' code from 'revert.c' into a new
'builtin/sequencer.c', and expose a simple cherry-picking API in
'sequencer.h'.

3. Implement a fresh 'cherry-pick.c' as a simple API call to the
sequencer, and make sure that all the existing tests pass.  After
this, cherry-pick will not be a builtin command anymore*.  Merge into
upstream.

4. Extend the sequncer to parse commands like 'execute', 'reword',
'squash', and 'fixup' that are specific to interactive rebasing.
Carefully implement the functionality for each of these keywords in a
step-wise manner, making sure that it inter-operates with 'rebase -i'
seamlessly.

5. Port the entire testsuite from 'rebase -i', and rewrite
'git-rebase.sh', 'git-rebase--interactive.sh' to call the sequncer.
The script should essentially build to a list of TODO instructions and
pass it to the 'git-sequencer' binary.  Merge into upstream.

6. [Optional] Lib'ify the sequncer: modify the sequencer API to
include rebase-related functionality.  Write 'rebase.c' as a bunch of
API calls to the sequencer.  Make sure that the existing tests pass.
Merge into upstream.

7. [Optional] Re-implement 'git-am.sh' as a thin wrapper over the
sequncer: 'am.c'.  Bulk of this should be mbox parsing code.  Make sure
that all existing tests pass.  Merge into upstream.

* Is this a good change? Are there any forseeable issues?
** [Optional] should be read as "If time permits"

== Timeline ===

Deriving from last year's experience, I've decided not to present a
tight timeline.  Instead, I simply have an outline: Get the extended
cherry-pick functionality merged before mid-term evaluations, and the
rest before the final evaluations.

== Who am I? ==

I'm Ramkumar Ramachandra, and I first started contributing to git.git
in January 2010.  Apart from doing fast-import and remote helper
related work last year, I also authored and merged svnrdump into
Subversion trunk in the same period.

== Notes ==

[1]: http://thread.gmane.org/gmane.comp.version-control.git/162183
======================================================================

Thanks for reading.

-- Ram

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [GSoC 2011] Git Sequencer
  2011-04-03 17:20 [GSoC 2011] Git Sequencer Ramkumar Ramachandra
@ 2011-04-03 17:24 ` Sverre Rabbelier
  2011-04-03 19:07 ` Stephan Beyer
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 20+ messages in thread
From: Sverre Rabbelier @ 2011-04-03 17:24 UTC (permalink / raw)
  To: Ramkumar Ramachandra
  Cc: Git List, Stephen Beyer, Christian Couder, Daniel Barkalow,
	Jonathan Nieder

Heya,

On Sun, Apr 3, 2011 at 19:20, Ramkumar Ramachandra <artagnon@gmail.com> wrote:
> I'd like to re-apply this year as a student because I really want to
> see (among other things), a sequencer in git.git.  Also, since I
> worked on areas related to fast-import and remote helpers last year, I
> thought I should work on something completely orthogonal this year.
>
> I now have a draft of my proposal ready, and I'd really appreciate
> feedback.  Also, could someone mentor me?

While I'm very interested in this project, I have no relevant
experience. I'll definitely +1 your proposal though :).

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [GSoC 2011] Git Sequencer
  2011-04-03 17:20 [GSoC 2011] Git Sequencer Ramkumar Ramachandra
  2011-04-03 17:24 ` Sverre Rabbelier
@ 2011-04-03 19:07 ` Stephan Beyer
  2011-04-03 20:00   ` Ramkumar Ramachandra
  2011-04-03 20:08   ` Jonathan Nieder
  2011-04-03 19:49 ` Daniel Barkalow
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 20+ messages in thread
From: Stephan Beyer @ 2011-04-03 19:07 UTC (permalink / raw)
  To: Ramkumar Ramachandra
  Cc: Git List, Christian Couder, Daniel Barkalow, Sverre Rabbelier,
	Jonathan Nieder

Hi Ram,

first, some notes on my git-sequencer 2008 branches that can be found at
http://repo.or.cz/w/git/sbeyer.git ... (Not sure if I remember
everything correctly)

I've settled to develop within the "seq-builtin-dev" branch and I
sometimes merged Junio's "master" into that branch to catch up.
The "seq-builtin-dev" branch is the important one.

Using git rebase -i (using git-sequencer) I sometimes remanaged the
branch to "seq-builtin-rfc" that should represent a snapshot of a
potential patch queue. My last rebase processes of the seq-builtin-rfc
branch were pretty unmotivated and hence messy.

I have not touched the repo very often after GSOC'08 and I stopped
touching it (as I stopped following recent Git development) "20 months
ago" apparently. Quite many things may have changed since then.

The file A_SEQUENCER_TODO_FILE (added 2009-08-03) in the repo describes
the missing and buggy pieces to fix so that _I_ (only me) would have
been 100 per cent satisfied with that git-sequencer.
http://repo.or.cz/w/git/sbeyer.git/blob/9e4b4d92f681a47e3d7ad2152d2391b2ab125a0c:/A_SEQUENCER_TODO_FILE
[Some notes are also "strategy notes" to get things accepted, like the
changes on "rebase -i -p" which are "not loved by everyone". ;-)]

On 2011-04-03, 22:50 +0530, Ramkumar Ramachandra wrote: 
> * Is this a good change? Are there any forseeable issues?

I want to mention an issue that I have not foreseen before: merges.
(You need merges, for example, when doing rebase -i -p ... -p as in
--preserve-merges.)

When I began, there was code in the "next" branch that added the TODO
instructions "mark", "reset" and "merge" to do merges properly and I
based my work on it. The original pieces by Jörg Sommer can still be
found here:
http://repo.or.cz/w/git/sbeyer.git/shortlog/6fabd85e8a777c26f3ae8ce11cb7f4265502ea7f

However, there have been strong opinions that the "mark"/"reset"/"merge"
instructions are ugly and unpleasant to users and not even necessary (at
least for rebase--interactive... and for sequencer, maybe, maybe not). 
Hence, the code in "next" has been rejected later.

During GSOC 2008 I regrettably underestimated the importance to
communicate with the Git folks about these things. That's one of the
main reasons the sequencer pieces did not get into master. And after
GSOC'08 I had too little time for this... :-/

Well, the merging thing is the only *real* issue I remember.

Good luck and regards,
  Stephan

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [GSoC 2011] Git Sequencer
  2011-04-03 17:20 [GSoC 2011] Git Sequencer Ramkumar Ramachandra
  2011-04-03 17:24 ` Sverre Rabbelier
  2011-04-03 19:07 ` Stephan Beyer
@ 2011-04-03 19:49 ` Daniel Barkalow
  2011-04-04  4:06   ` Ramkumar Ramachandra
  2011-04-04  4:43 ` Christian Couder
  2011-04-05 20:00 ` [GSoC 2011 v2] " Ramkumar Ramachandra
  4 siblings, 1 reply; 20+ messages in thread
From: Daniel Barkalow @ 2011-04-03 19:49 UTC (permalink / raw)
  To: Ramkumar Ramachandra
  Cc: Git List, Stephen Beyer, Christian Couder, Sverre Rabbelier,
	Jonathan Nieder

On Sun, 3 Apr 2011, Ramkumar Ramachandra wrote:

> Hi,
> 
> I'd like to re-apply this year as a student because I really want to
> see (among other things), a sequencer in git.git.  Also, since I
> worked on areas related to fast-import and remote helpers last year, I
> thought I should work on something completely orthogonal this year.
> 
> I now have a draft of my proposal ready, and I'd really appreciate
> feedback.  Also, could someone mentor me?
> 
> ======================================================================
> Project Proposal: Git Sequencer
> Student: Ramkumar Ramachandra
> Mentor: ?
> 
> == The Objective ==
> 
> To write git-sequencer, a new builtin command, and implement existing
> commands on top of that.  This should give the commands more
> functionality, improve their error handling, and make them faster.
> The project can only be considered successful if all (or most) of the
> code written gets merged into upstream.
> 
> The Git Sequencer was a 2008 GSoC project as well; unfortunately most
> of the code did not get merged into git.git.  The learning from all
> that work should serve as a huge headstart this year.

One of the things that is hard about sequencer is that it is ultimately a 
complete replacement for several differently-implemented programs in 
different languages, with different temporary file formats and differrent 
supported operations. As such, you could probably spend an entire summer 
just getting it reviewed, revised, and accepted, starting with a working 
implementation.

So I think your proposal is good in how [1/5] includes getting something 
useful merged. My suspicion is that the outcome will be something like 
that you implemented all 7 tasks and got 4 of them merged, assuming that 
you really push getting things merged as soon as they're ready, without 
spending too much time porting other things to use the core and getting 
the ports reviewed before the core is accepted.

I actually think that it would be a worthwhile feature for git's library 
code to have a uniform mechanism for communicating that it is requesting 
human intervention in the middle of a particular operation, where library 
operations which conflict with being able to continue this operation are 
either blocked or abort the operation, and the library is able to be told 
in general that the human intervention is done and the library operation 
should be finished now (or produce complaints about the user's work). That 
is, a library-level, single-interrupted-step "sequencer". For that matter, 
it should also apply to the common '"git merge" gets a conflict' case, and 
it would be useful to get some representational uniformity between that 
and cherry-pick getting a conflict.

I think replacing existing multi-step processes is going to be a lot more 
contentious and involve user-visible changes which involve matters of 
taste and such. But I think you can make a valuable contribution in how a 
single current step is handled before getting tangled in that, and be much 
more likely to get a useful outcome than if you try to tackle the whole 
problem.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [GSoC 2011] Git Sequencer
  2011-04-03 19:07 ` Stephan Beyer
@ 2011-04-03 20:00   ` Ramkumar Ramachandra
  2011-04-03 20:08   ` Jonathan Nieder
  1 sibling, 0 replies; 20+ messages in thread
From: Ramkumar Ramachandra @ 2011-04-03 20:00 UTC (permalink / raw)
  To: Stephan Beyer
  Cc: Git List, Christian Couder, Daniel Barkalow, Sverre Rabbelier,
	Jonathan Nieder

Hi Stephen,

Stephan Beyer writes:
> first, some notes on my git-sequencer 2008 branches that can be found at
> http://repo.or.cz/w/git/sbeyer.git ... (Not sure if I remember
> everything correctly)
> 
> I've settled to develop within the "seq-builtin-dev" branch and I
> sometimes merged Junio's "master" into that branch to catch up.
> The "seq-builtin-dev" branch is the important one.

Thanks! Jonathan told me about it earlier, and I've already started
ripping out code from the seq-builtin-dev branch :) I found your
't3350-sequencer.sh' especially interesting.

> Using git rebase -i (using git-sequencer) I sometimes remanaged the
> branch to "seq-builtin-rfc" that should represent a snapshot of a
> potential patch queue. My last rebase processes of the seq-builtin-rfc
> branch were pretty unmotivated and hence messy.
> 
> I have not touched the repo very often after GSOC'08 and I stopped
> touching it (as I stopped following recent Git development) "20 months
> ago" apparently. Quite many things may have changed since then.

Okay, got it.  I saw a few patches in 'master' that were based on your
work though.  Some of the patches in Christian's series also refer to
your work.

> The file A_SEQUENCER_TODO_FILE (added 2009-08-03) in the repo describes
> the missing and buggy pieces to fix so that _I_ (only me) would have
> been 100 per cent satisfied with that git-sequencer.
> http://repo.or.cz/w/git/sbeyer.git/blob/9e4b4d92f681a47e3d7ad2152d2391b2ab125a0c:/A_SEQUENCER_TODO_FILE
> [Some notes are also "strategy notes" to get things accepted, like the
> changes on "rebase -i -p" which are "not loved by everyone". ;-)]

Okay.

> On 2011-04-03, 22:50 +0530, Ramkumar Ramachandra wrote: 
> > * Is this a good change? Are there any forseeable issues?
> 
> I want to mention an issue that I have not foreseen before: merges.
> (You need merges, for example, when doing rebase -i -p ... -p as in
> --preserve-merges.)

Ah, that's not something I thought about immediately.

> When I began, there was code in the "next" branch that added the TODO
> instructions "mark", "reset" and "merge" to do merges properly and I
> based my work on it. The original pieces by Jörg Sommer can still be
> found here:
> http://repo.or.cz/w/git/sbeyer.git/shortlog/6fabd85e8a777c26f3ae8ce11cb7f4265502ea7f
> 
> However, there have been strong opinions that the "mark"/"reset"/"merge"
> instructions are ugly and unpleasant to users and not even necessary (at
> least for rebase--interactive... and for sequencer, maybe, maybe not). 
> Hence, the code in "next" has been rejected later.

Interesting historical note.

> During GSOC 2008 I regrettably underestimated the importance to
> communicate with the Git folks about these things. That's one of the
> main reasons the sequencer pieces did not get into master. And after
> GSOC'08 I had too little time for this... :-/
> 
> Well, the merging thing is the only *real* issue I remember.

Point noted.  Yes, I noticed that your sequencer was mostly
functionally complete.  I'll make sure that I spend a lot of
interacting with the community.

Thank you for your elaborate note! I really appreciate it :)
Hopefully, we will have that sequencer by next year.

-- Ram

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [GSoC 2011] Git Sequencer
  2011-04-03 19:07 ` Stephan Beyer
  2011-04-03 20:00   ` Ramkumar Ramachandra
@ 2011-04-03 20:08   ` Jonathan Nieder
  1 sibling, 0 replies; 20+ messages in thread
From: Jonathan Nieder @ 2011-04-03 20:08 UTC (permalink / raw)
  To: Stephan Beyer
  Cc: Ramkumar Ramachandra, Git List, Christian Couder, Daniel Barkalow,
	Sverre Rabbelier

Stephan Beyer wrote:

> I want to mention an issue that I have not foreseen before: merges.
> (You need merges, for example, when doing rebase -i -p ... -p as in
> --preserve-merges.)
>
> When I began, there was code in the "next" branch that added the TODO
> instructions "mark", "reset" and "merge" to do merges properly and I
> based my work on it. The original pieces by Jörg Sommer can still be
> found here:
> http://repo.or.cz/w/git/sbeyer.git/shortlog/6fabd85e8a777c26f3ae8ce11cb7f4265502ea7f
[etc]

Some more pointers:
http://thread.gmane.org/gmane.comp.version-control.git/148059
IIRC there's some rough consensus about the design, even if I'm not
sure what it is :).

Of course a dream would be a way to rebase merge conflict resolutions
instead of being limited to replaying conflict-free merges or
resolving conflicts by hand.  But that's a more complex story.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [GSoC 2011] Git Sequencer
  2011-04-03 19:49 ` Daniel Barkalow
@ 2011-04-04  4:06   ` Ramkumar Ramachandra
  2011-04-04  4:54     ` Ramkumar Ramachandra
  0 siblings, 1 reply; 20+ messages in thread
From: Ramkumar Ramachandra @ 2011-04-04  4:06 UTC (permalink / raw)
  To: Daniel Barkalow
  Cc: Git List, Stephen Beyer, Christian Couder, Sverre Rabbelier,
	Jonathan Nieder

Hi Daniel,

Daniel Barkalow writes:
> On Sun, 3 Apr 2011, Ramkumar Ramachandra wrote:
> > To write git-sequencer, a new builtin command, and implement existing
> > commands on top of that.  This should give the commands more
> > functionality, improve their error handling, and make them faster.
> > The project can only be considered successful if all (or most) of the
> > code written gets merged into upstream.
> > 
> > The Git Sequencer was a 2008 GSoC project as well; unfortunately most
> > of the code did not get merged into git.git.  The learning from all
> > that work should serve as a huge headstart this year.
> 
> One of the things that is hard about sequencer is that it is ultimately a 
> complete replacement for several differently-implemented programs in 
> different languages, with different temporary file formats and differrent 
> supported operations. As such, you could probably spend an entire summer 
> just getting it reviewed, revised, and accepted, starting with a working 
> implementation.

Agreed.  I've chosen to use the same commands and temporary files as
'git-rebase--interactive.sh', because I think those commands are
sufficient to implement everything else.

> So I think your proposal is good in how [1/5] includes getting something 
> useful merged. My suspicion is that the outcome will be something like 
> that you implemented all 7 tasks and got 4 of them merged, assuming that 
> you really push getting things merged as soon as they're ready, without 
> spending too much time porting other things to use the core and getting 
> the ports reviewed before the core is accepted.

Hm.  In that case, we'll just have a sequencer that can cherry-pick --
I personally wouldn't be too happy with this outcome either.

> I actually think that it would be a worthwhile feature for git's library 
> code to have a uniform mechanism for communicating that it is requesting 
> human intervention in the middle of a particular operation, where library 
> operations which conflict with being able to continue this operation are 
> either blocked or abort the operation, and the library is able to be told 
> in general that the human intervention is done and the library operation 
> should be finished now (or produce complaints about the user's work). That 
> is, a library-level, single-interrupted-step "sequencer". For that matter, 
> it should also apply to the common '"git merge" gets a conflict' case, and 
> it would be useful to get some representational uniformity between that 
> and cherry-pick getting a conflict.

Until 4/7, I'd only planned to make the 'git-sequencer' binary like
the 'git-rebase--interactive.sh' script, except that it would accept a
TODO file on stdin, instead of interactively opening up an editor.

Your idea is a slightly more ambitious version of what I'd planned for
6/7, especially since 'merge' contains a lot of cruft like MERGE_HEAD
and CHERRY_PICK_HEAD.  I can shift my focus after 4/7 though -- here's
what I have in mind.  Do you have something similar in mind?

enum commit_todo_action {
     ACTION_PICK;
     ACTION_REWORD;
     ACTION_EDIT;
     ACTION_SQUASH;
     ACTION_FIXUP;
     ACTION_EXEC;
};

struct commit_todo_list {
       struct commit *item;
       enum commit_todo_action action;
       struct commit_todo_list *next;
};

int sequencer_cherry_pick(struct commit *base, struct commit_list *list);
int sequncer_rebase(struct commit *base, struct commit_todo_list *list);
int sequencer_handle_conflict(); /* Returns ABORT (1) or RESOLVED (0) */

/**
 * The sequencer_handle_conflict function essentially starts with a
 * working tree with unmerged files and results in either a working
 * tree without unmerged files (in which case it returns 0), or simply
 * returns 1.  Advantage: Consistency. Each individual script will not
 * have to maintain its own temporary files.
 */

> I think replacing existing multi-step processes is going to be a lot more 
> contentious and involve user-visible changes which involve matters of 
> taste and such. But I think you can make a valuable contribution in how a 
> single current step is handled before getting tangled in that, and be much 
> more likely to get a useful outcome than if you try to tackle the whole 
> problem.

Okay.  I'll replace 5/7 - 7/7 in my proposal with an alternative as
soon as I sketch out the details more clearly.

Thanks for your suggestions!

-- Ram

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [GSoC 2011] Git Sequencer
  2011-04-03 17:20 [GSoC 2011] Git Sequencer Ramkumar Ramachandra
                   ` (2 preceding siblings ...)
  2011-04-03 19:49 ` Daniel Barkalow
@ 2011-04-04  4:43 ` Christian Couder
  2011-04-04  5:20   ` Junio C Hamano
  2011-04-04 16:57   ` Ramkumar Ramachandra
  2011-04-05 20:00 ` [GSoC 2011 v2] " Ramkumar Ramachandra
  4 siblings, 2 replies; 20+ messages in thread
From: Christian Couder @ 2011-04-04  4:43 UTC (permalink / raw)
  To: Ramkumar Ramachandra
  Cc: Git List, Stephen Beyer, Daniel Barkalow, Sverre Rabbelier,
	Jonathan Nieder

On Sunday 03 April 2011 19:20:56 Ramkumar Ramachandra wrote:
> Hi,
> 
> I'd like to re-apply this year as a student because I really want to
> see (among other things), a sequencer in git.git.  Also, since I
> worked on areas related to fast-import and remote helpers last year, I
> thought I should work on something completely orthogonal this year.
> 
> I now have a draft of my proposal ready, and I'd really appreciate
> feedback.  Also, could someone mentor me?

Yeah! I would be happy to mentor you (or co-mentor you if someone else want to 
be involved)!

> ======================================================================
> Project Proposal: Git Sequencer
> Student: Ramkumar Ramachandra
> Mentor: ?
> 
> == The Objective ==
> 
> To write git-sequencer, a new builtin command, and implement existing
> commands on top of that.  This should give the commands more
> functionality, improve their error handling, and make them faster.

You should first talk about extending git cherry-pick with --continue, --abort 
and --skip, because it can be very valuable already if done properly with many 
tests and if it's merged of course.

> The project can only be considered successful if all (or most) of the
> code written gets merged into upstream.

Yeah, just say "most of the code". It is definitely good enough.

> The Git Sequencer was a 2008 GSoC project as well; unfortunately most
> of the code did not get merged into git.git.  The learning from all
> that work should serve as a huge headstart this year.
> 
> === The Plan ===
> 
> 1. Extend 'cherry-pick' with '--continue', '--abort', and '--skip'
> features, so that it works like (a subset of) the current
> 'git-rebase--interactive.sh'.  This will require patching
> 'builtin/revert.c' in place, and merging it immediately.  I plan to
> roughly follow the road laid out by Christian's 2010 series [1].

Yeah, the first step should be 'cherry-pick' with '--continue', '--abort', and 
'--skip' merged.

> 1.1. Factor out all calls to 'die' with 'return error' so so that we
> can pause the entire process when a commit doesn't apply
> automatically.
> 
> 1.2. Create and populate TODO and DONE files, similar to the one that
> 'git-rebase--interactive.sh' creates.  For now, it should simply give
> us information about why a 'cherry-pick' failed.  Use these files with
> 'git-rebase--interactive.sh' to resume.

I am not sure it's a good thing to use 'git-rebase--interactive.sh' to resume 
the cherry-pick. The parsing code already exists, is not very big, is in C and 
has been reviewed and tested, so I think it's better to use.

> 1.3. Port selective tests from the current 't3404' to make sure that
> TODO and DONE are populated correctly; "stop on conflicting pick" is a
> good candidate.
> 
> 1.4. Decouple the 'revert' functionality from the 'cherry-pick'
> functionality in 'revert.c'.  Implement '--abort' for 'cherry-pick'
> and port "abort" test from 't3404'.

I am not sure decoupling revert and cherry-pick functionnalities is really 
needed, or I don't know what you mean exactly.

> 1.5. Implement parsing the TODO and DONE files into suitable data
> structures.  Derive inspiration from the code written in 2008 to do
> this.

Yeah, you can use this patch:

http://article.gmane.org/gmane.comp.version-control.git/162198

> 1.6. Implement '--continue' and '--skip', and write suitable tests.
> Merge into upstream.

Great!

> 2. Factor out the 'cherry-pick' code from 'revert.c' into a new
> 'builtin/sequencer.c', and expose a simple cherry-picking API in
> 'sequencer.h'.

Yeah!

> 3. Implement a fresh 'cherry-pick.c' as a simple API call to the
> sequencer, and make sure that all the existing tests pass.  After
> this, cherry-pick will not be a builtin command anymore*.  Merge into
> upstream.

Yeah, it is closely linked with the previous point. So maybe this can be 2.2 
and the previous one can be 2.1. And this way we know that at the end of 1) 
and at the end of 2) everything should be merged upstream.

> 4. Extend the sequncer to parse commands like 'execute', 'reword',
> 'squash', and 'fixup' that are specific to interactive rebasing.
> Carefully implement the functionality for each of these keywords in a
> step-wise manner, making sure that it inter-operates with 'rebase -i'
> seamlessly.

Great!

> 5. Port the entire testsuite from 'rebase -i', and rewrite
> 'git-rebase.sh', 'git-rebase--interactive.sh' to call the sequncer.
> The script should essentially build to a list of TODO instructions and
> pass it to the 'git-sequencer' binary.  Merge into upstream.

Great! But as it is closely llinked with the previous point, maybe these 2 
points should be 3.1 and 3.2.

> 6. [Optional] Lib'ify the sequncer: modify the sequencer API to
> include rebase-related functionality.  Write 'rebase.c' as a bunch of
> API calls to the sequencer.  Make sure that the existing tests pass.
> Merge into upstream.
> 
> 7. [Optional] Re-implement 'git-am.sh' as a thin wrapper over the
> sequncer: 'am.c'.  Bulk of this should be mbox parsing code.  Make sure
> that all existing tests pass.  Merge into upstream.
> 
> * Is this a good change? 

I think it is a good proposal.

> Are there any forseeable issues?

I don't see anything that others didn't told you about.

> ** [Optional] should be read as "If time permits"
> 
> == Timeline ===
> 
> Deriving from last year's experience, I've decided not to present a
> tight timeline.  Instead, I simply have an outline: Get the extended
> cherry-pick functionality merged before mid-term evaluations, and the
> rest before the final evaluations.

I agree, but still you could perhaps state something like this:

- before mid june:
	some patch series for everything in 1) should have been sent to the list
- before midterm evaluation:
	everything in 1) should be merged upstream
	some patch series for everything in 2) and 3) (or 2.1 and 2.2 if you use 
the numbering I suggest) should have been sent to the list
- before the end of July:
	everything in 2) and 3) should be merged upstream
	some patch series for everything in 4) and 5) (or 3.1 and 3.2 if you use 
the numbering I suggest) should have been sent to the list
- before final evaluation:
	everything should be merged

I think it is better to have more details like the above because this way we 
can realize early that there is not a lot of time after the midterm 
evaluation.

> == Who am I? ==
> 
> I'm Ramkumar Ramachandra, and I first started contributing to git.git
> in January 2010.  Apart from doing fast-import and remote helper
> related work last year, I also authored and merged svnrdump into
> Subversion trunk in the same period.
> 
> == Notes ==
> 
> [1]: http://thread.gmane.org/gmane.comp.version-control.git/162183
> ======================================================================
> 
> Thanks for reading.

Thanks for applying,
Christian.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [GSoC 2011] Git Sequencer
  2011-04-04  4:06   ` Ramkumar Ramachandra
@ 2011-04-04  4:54     ` Ramkumar Ramachandra
  2011-04-04 18:59       ` Daniel Barkalow
  0 siblings, 1 reply; 20+ messages in thread
From: Ramkumar Ramachandra @ 2011-04-04  4:54 UTC (permalink / raw)
  To: Daniel Barkalow
  Cc: Git List, Stephen Beyer, Christian Couder, Sverre Rabbelier,
	Jonathan Nieder

Hi Daniel,

Ramkumar Ramachandra writes:
> Daniel Barkalow writes:
> > I actually think that it would be a worthwhile feature for git's library 
> > code to have a uniform mechanism for communicating that it is requesting 
> > human intervention in the middle of a particular operation, where library 
> > operations which conflict with being able to continue this operation are 
> > either blocked or abort the operation, and the library is able to be told 
> > in general that the human intervention is done and the library operation 
> > should be finished now (or produce complaints about the user's work). That 
> > is, a library-level, single-interrupted-step "sequencer". For that matter, 
> > it should also apply to the common '"git merge" gets a conflict' case, and 
> > it would be useful to get some representational uniformity between that 
> > and cherry-pick getting a conflict.

[...]

> int sequencer_handle_conflict(); /* Returns ABORT (1) or RESOLVED (0) */
> 
> /**
>  * The sequencer_handle_conflict function essentially starts with a
>  * working tree with unmerged files and results in either a working
>  * tree without unmerged files (in which case it returns 0), or simply
>  * returns 1.  Advantage: Consistency. Each individual script will not
>  * have to maintain its own temporary files.
>  */

Uh, no.  I wrote this part in too quickly.  Clearly needs more
thought.

-- Ram

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [GSoC 2011] Git Sequencer
  2011-04-04  4:43 ` Christian Couder
@ 2011-04-04  5:20   ` Junio C Hamano
  2011-04-05  6:23     ` Christian Couder
  2011-04-04 16:57   ` Ramkumar Ramachandra
  1 sibling, 1 reply; 20+ messages in thread
From: Junio C Hamano @ 2011-04-04  5:20 UTC (permalink / raw)
  To: Christian Couder
  Cc: Ramkumar Ramachandra, Git List, Stephen Beyer, Daniel Barkalow,
	Sverre Rabbelier, Jonathan Nieder

Christian Couder <chriscool@tuxfamily.org> writes:

> Yeah, the first step should be 'cherry-pick' with '--continue', '--abort', and 
> '--skip' merged.

I haven't looked at rebase-i machinery recently, but I wonder if it would
just be a matter of making a multi-commit cherry-pick just prepare a bunch
of "pick XXX" lines into .git/rebase-merge/rebase-todo file, make other
trivial setups (like detaching HEAD, writing head-name and head files) and
then execing "git rebase --continue"?

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [GSoC 2011] Git Sequencer
  2011-04-04  4:43 ` Christian Couder
  2011-04-04  5:20   ` Junio C Hamano
@ 2011-04-04 16:57   ` Ramkumar Ramachandra
  1 sibling, 0 replies; 20+ messages in thread
From: Ramkumar Ramachandra @ 2011-04-04 16:57 UTC (permalink / raw)
  To: Christian Couder
  Cc: Git List, Stephen Beyer, Daniel Barkalow, Sverre Rabbelier,
	Jonathan Nieder

Hi Christian,

Christian Couder writes:
> On Sunday 03 April 2011 19:20:56 Ramkumar Ramachandra wrote:
> > I'd like to re-apply this year as a student because I really want to
> > see (among other things), a sequencer in git.git.  Also, since I
> > worked on areas related to fast-import and remote helpers last year, I
> > thought I should work on something completely orthogonal this year.
> > 
> > I now have a draft of my proposal ready, and I'd really appreciate
> > feedback.  Also, could someone mentor me?
> Yeah! I would be happy to mentor you (or co-mentor you if someone else want to 
> be involved)!

Awesome! Thanks :)

> > == The Objective ==
> > 
> > To write git-sequencer, a new builtin command, and implement existing
> > commands on top of that.  This should give the commands more
> > functionality, improve their error handling, and make them faster.
> 
> You should first talk about extending git cherry-pick with --continue, --abort 
> and --skip, because it can be very valuable already if done properly with many 
> tests and if it's merged of course.
> 
> > The project can only be considered successful if all (or most) of the
> > code written gets merged into upstream.
> 
> Yeah, just say "most of the code". It is definitely good enough.

Okay, here's the new objective:

Extend 'git cherry-pick' with '--continue', '--abort', and '--skip'
features.  This will ultimately be used to write git-sequencer, a new
builtin command.  The sequencer will provide a uniform interface over
which existing commands like 'rebase', 'rebase -i' and 'am' can be
re-implented.  This should give the commands more functionality,
improve their error handling, and make them faster.  The project can
only be considered successful if most of the code written gets merged
into upstream.

> > 1.1. Factor out all calls to 'die' with 'return error' so so that we
> > can pause the entire process when a commit doesn't apply
> > automatically.
> > 
> > 1.2. Create and populate TODO and DONE files, similar to the one that
> > 'git-rebase--interactive.sh' creates.  For now, it should simply give
> > us information about why a 'cherry-pick' failed.  Use these files with
> > 'git-rebase--interactive.sh' to resume.
> 
> I am not sure it's a good thing to use 'git-rebase--interactive.sh' to resume 
> the cherry-pick. The parsing code already exists, is not very big, is in C and 
> has been reviewed and tested, so I think it's better to use.

Okay, noted.

> > 1.3. Port selective tests from the current 't3404' to make sure that
> > TODO and DONE are populated correctly; "stop on conflicting pick" is a
> > good candidate.
> > 
> > 1.4. Decouple the 'revert' functionality from the 'cherry-pick'
> > functionality in 'revert.c'.  Implement '--abort' for 'cherry-pick'
> > and port "abort" test from 't3404'.
> 
> I am not sure decoupling revert and cherry-pick functionnalities is really 
> needed, or I don't know what you mean exactly.

What I was thinking when I wrote that: we should have a 'revert.c'
independent of 'cherry-pick.c' after the sequencer is implemented (see
2).  By "decouple" I meant: move code around so we don't have to use
the enum { REVERT, CHERRY_PICK }.

> > 1.5. Implement parsing the TODO and DONE files into suitable data
> > structures.  Derive inspiration from the code written in 2008 to do
> > this.
> 
> Yeah, you can use this patch:
> 
> http://article.gmane.org/gmane.comp.version-control.git/162198

Yeah, this is exactly the patch I was referring to.  I just forgot to
include the link :p

> > 1.6. Implement '--continue' and '--skip', and write suitable tests.
> > Merge into upstream.
> 
> Great!
> 
> > 2. Factor out the 'cherry-pick' code from 'revert.c' into a new
> > 'builtin/sequencer.c', and expose a simple cherry-picking API in
> > 'sequencer.h'.
> 
> Yeah!
> 
> > 3. Implement a fresh 'cherry-pick.c' as a simple API call to the
> > sequencer, and make sure that all the existing tests pass.  After
> > this, cherry-pick will not be a builtin command anymore*.  Merge into
> > upstream.
> 
> Yeah, it is closely linked with the previous point. So maybe this can be 2.2 
> and the previous one can be 2.1. And this way we know that at the end of 1) 
> and at the end of 2) everything should be merged upstream.

Okay.

> > 4. Extend the sequncer to parse commands like 'execute', 'reword',
> > 'squash', and 'fixup' that are specific to interactive rebasing.
> > Carefully implement the functionality for each of these keywords in a
> > step-wise manner, making sure that it inter-operates with 'rebase -i'
> > seamlessly.
> 
> Great!
> 
> > 5. Port the entire testsuite from 'rebase -i', and rewrite
> > 'git-rebase.sh', 'git-rebase--interactive.sh' to call the sequncer.
> > The script should essentially build to a list of TODO instructions and
> > pass it to the 'git-sequencer' binary.  Merge into upstream.
> 
> Great! But as it is closely llinked with the previous point, maybe these 2 
> points should be 3.1 and 3.2.

Okay.

> > 6. [Optional] Lib'ify the sequncer: modify the sequencer API to
> > include rebase-related functionality.  Write 'rebase.c' as a bunch of
> > API calls to the sequencer.  Make sure that the existing tests pass.
> > Merge into upstream.
> > 
> > 7. [Optional] Re-implement 'git-am.sh' as a thin wrapper over the
> > sequncer: 'am.c'.  Bulk of this should be mbox parsing code.  Make sure
> > that all existing tests pass.  Merge into upstream.

I'll append Daniel's single-step-interrupt idea here, once I
understand how to implement it fully.

> > == Timeline ===
> > 
> > Deriving from last year's experience, I've decided not to present a
> > tight timeline.  Instead, I simply have an outline: Get the extended
> > cherry-pick functionality merged before mid-term evaluations, and the
> > rest before the final evaluations.
> 
> I agree, but still you could perhaps state something like this:
> 
> - before mid june:
> 	some patch series for everything in 1) should have been sent to the list
> - before midterm evaluation:
> 	everything in 1) should be merged upstream
> 	some patch series for everything in 2) and 3) (or 2.1 and 2.2 if you use 
> the numbering I suggest) should have been sent to the list
> - before the end of July:
> 	everything in 2) and 3) should be merged upstream
> 	some patch series for everything in 4) and 5) (or 3.1 and 3.2 if you use 
> the numbering I suggest) should have been sent to the list
> - before final evaluation:
> 	everything should be merged
> 
> I think it is better to have more details like the above because this way we 
> can realize early that there is not a lot of time after the midterm 
> evaluation.

Great suggestion! I'll include this in the next iteration of my
proposal.

Thanks for the detailed review and the suggestions.

-- Ram

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [GSoC 2011] Git Sequencer
  2011-04-04  4:54     ` Ramkumar Ramachandra
@ 2011-04-04 18:59       ` Daniel Barkalow
  2011-04-05 17:50         ` Ramkumar Ramachandra
  0 siblings, 1 reply; 20+ messages in thread
From: Daniel Barkalow @ 2011-04-04 18:59 UTC (permalink / raw)
  To: Ramkumar Ramachandra
  Cc: Git List, Stephen Beyer, Christian Couder, Sverre Rabbelier,
	Jonathan Nieder

On Mon, 4 Apr 2011, Ramkumar Ramachandra wrote:

> Hi Daniel,
> 
> Ramkumar Ramachandra writes:
> > Daniel Barkalow writes:
> > > I actually think that it would be a worthwhile feature for git's library 
> > > code to have a uniform mechanism for communicating that it is requesting 
> > > human intervention in the middle of a particular operation, where library 
> > > operations which conflict with being able to continue this operation are 
> > > either blocked or abort the operation, and the library is able to be told 
> > > in general that the human intervention is done and the library operation 
> > > should be finished now (or produce complaints about the user's work). That 
> > > is, a library-level, single-interrupted-step "sequencer". For that matter, 
> > > it should also apply to the common '"git merge" gets a conflict' case, and 
> > > it would be useful to get some representational uniformity between that 
> > > and cherry-pick getting a conflict.
> 
> [...]
> 
> > int sequencer_handle_conflict(); /* Returns ABORT (1) or RESOLVED (0) */
> > 
> > /**
> >  * The sequencer_handle_conflict function essentially starts with a
> >  * working tree with unmerged files and results in either a working
> >  * tree without unmerged files (in which case it returns 0), or simply
> >  * returns 1.  Advantage: Consistency. Each individual script will not
> >  * have to maintain its own temporary files.
> >  */
> 
> Uh, no.  I wrote this part in too quickly.  Clearly needs more
> thought.

Here's how I'm thinking about a single step:

The non-conflict case is:

  $ git cherry-pick ...
  figure out what we're asked to do
  make the change to the working directory and index
  make the commit with info from the commit we're cherry-picking

The conflict case should be:

  $ git cherry-pick ...
  figure out what we're asked to do
  make the change to the working directory and index
  discover problem, set up for human assistance, tweak info to say that we 
    needed help
  $ fix stuff
  $ git continue
  check that everything is how it should be
  make the commit with info from the commit we're cherry-picking

That is, the code that cherry-picks one commit can quit in the middle and 
resume after the user finishes helping, and the main entry point to git 
can resume that operation.

So, my thought was that you'd have something like:

cherry_pick_conflict = { 
  "cherry-pick", APPLIES_TO_CURRENT_BRANCH | IN_MIDDLE_OF_COMMIT,
  cherry_pick_verify_resolution,
  cherry_pick_abort,
  cherry_pick_post_resolution
};

int cherry_pick(struct commit *item)
{
  save info on the commit, flags, etc needed to understand what we're doing

  try to apply diff...
  if (!rejected)
    return cherry_pick_post_resolution();
  try to use merge recursive...
  if (!conflict)
    return cherry_pick_post_resolution();
  return report_conflict(cherry_pick_conflict);
}

int cherry_pick_verify_resolution(void)
{
  if (still unmerged files) {
    describe work still needed
    return 1;
  }
  return 0;
}

int cherry_pick_abort(void)
{
  restore info on what we'd started and delete state

  clean up the working directory, index, etc

  return 0;
}

int cherry_pick_post_resolution(void)
{
  restore info on what we'd started and delete state

  make the commit

  return 0;
}

The important things are:

 - arbitrary code can determine that you're in the middle of resolving 
   some conflict, that the resolution of that conflict is about doing
   something to your current branch, and how to abort what you're doing,
   and how to finish it

 - the same code gets run after the conflict has been resolved that would 
   have been run immediately if the merge went smoothly

 - cherry-pick can save whatever it needs to in its state file; that's 
   its business, and the semantics here don't have to interact with other 
   commands, because report_conflict() has taken care of interaction with 
   other commands

The next step is to be able to have:

int sequencer()
{
  save the list of steps, with no in-progress step
  return sequencer_post_resolution();
}

int sequencer_post_resolution()
{
  finish any in-progress step
  get the next step
  if (no steps left)
    return 0;
  attempt step
  if (!got_conflict)
    return sequencer_post_resolution();
  return report_conflict(sequencer_conflict);
}

Where the sequencer-level conflict nests around the cherry-pick-level 
conflict, and the generic "continue" completes things from the inside out.

I think, ultimately, that with this code structure in place, the 
am/rebase/rebase--interactive/sequencer details of how the multi-step 
process is recorded becomes less important. That way, your project can be 
successful even if you can't find a syntax for the sequencer file that 
meets the needs of all of these use cases. (Which is where I suspect 
you'll get bogged down.) If you can get all of the cases where git exits 
in order to get human intervention to share "everything that matters" and 
the core to "know what's in progress as far as anything else cares", I 
think that would be success, even if the various multi-step programs 
continue using their own state files.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [GSoC 2011] Git Sequencer
  2011-04-04  5:20   ` Junio C Hamano
@ 2011-04-05  6:23     ` Christian Couder
  2011-04-05  6:46       ` Ramkumar Ramachandra
  0 siblings, 1 reply; 20+ messages in thread
From: Christian Couder @ 2011-04-05  6:23 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Ramkumar Ramachandra, Git List, Stephen Beyer, Daniel Barkalow,
	Sverre Rabbelier, Jonathan Nieder

On Monday 04 April 2011 07:20:18 Junio C Hamano wrote:
> Christian Couder <chriscool@tuxfamily.org> writes:
> > Yeah, the first step should be 'cherry-pick' with '--continue',
> > '--abort', and '--skip' merged.
> 
> I haven't looked at rebase-i machinery recently, but I wonder if it would
> just be a matter of making a multi-commit cherry-pick just prepare a bunch
> of "pick XXX" lines into .git/rebase-merge/rebase-todo file, make other
> trivial setups (like detaching HEAD, writing head-name and head files) and
> then execing "git rebase --continue"?

It is probably quite easy to do that, but it would result in cherry-pick in C 
calling rebase-i in shell that itself calls cherry-pick in C (to pick 
individual commit). Instead with this GSoC I think we have the opportunity to 
have everything we needed in C.

Best regards,
Christian.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [GSoC 2011] Git Sequencer
  2011-04-05  6:23     ` Christian Couder
@ 2011-04-05  6:46       ` Ramkumar Ramachandra
  0 siblings, 0 replies; 20+ messages in thread
From: Ramkumar Ramachandra @ 2011-04-05  6:46 UTC (permalink / raw)
  To: Christian Couder
  Cc: Junio C Hamano, Git List, Stephen Beyer, Daniel Barkalow,
	Sverre Rabbelier, Jonathan Nieder

Hi Christian and Junio,

On Tue, Apr 5, 2011 at 11:53 AM, Christian Couder
<chriscool@tuxfamily.org> wrote:
> On Monday 04 April 2011 07:20:18 Junio C Hamano wrote:
>> Christian Couder <chriscool@tuxfamily.org> writes:
>> > Yeah, the first step should be 'cherry-pick' with '--continue',
>> > '--abort', and '--skip' merged.
>>
>> I haven't looked at rebase-i machinery recently, but I wonder if it would
>> just be a matter of making a multi-commit cherry-pick just prepare a bunch
>> of "pick XXX" lines into .git/rebase-merge/rebase-todo file, make other
>> trivial setups (like detaching HEAD, writing head-name and head files) and
>> then execing "git rebase --continue"?
>
> It is probably quite easy to do that, but it would result in cherry-pick in C
> calling rebase-i in shell that itself calls cherry-pick in C (to pick
> individual commit). Instead with this GSoC I think we have the opportunity to
> have everything we needed in C.

I thought I should clarify- in the original proposal, I didn't mean
for cherry-pick to call "rebase -i" at all. When I said "use rebase to
resume", I meant "try resuming by hand using rebase --continue to
verify that cherry-pick has written the state information correctly"
as an intermediate step in the cherry-pick development.

-- Ram

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [GSoC 2011] Git Sequencer
  2011-04-04 18:59       ` Daniel Barkalow
@ 2011-04-05 17:50         ` Ramkumar Ramachandra
  2011-04-05 18:24           ` Daniel Barkalow
  0 siblings, 1 reply; 20+ messages in thread
From: Ramkumar Ramachandra @ 2011-04-05 17:50 UTC (permalink / raw)
  To: Daniel Barkalow
  Cc: Git List, Stephen Beyer, Christian Couder, Sverre Rabbelier,
	Jonathan Nieder

Hi Daniel,

Daniel Barkalow writes:
> On Mon, 4 Apr 2011, Ramkumar Ramachandra wrote:
> > Ramkumar Ramachandra writes:
> > > Daniel Barkalow writes:
> > > > I actually think that it would be a worthwhile feature for git's library 
> > > > code to have a uniform mechanism for communicating that it is requesting 
> > > > human intervention in the middle of a particular operation, where library 
> > > > operations which conflict with being able to continue this operation are 
> > > > either blocked or abort the operation, and the library is able to be told 
> > > > in general that the human intervention is done and the library operation 
> > > > should be finished now (or produce complaints about the user's work). That 
> > > > is, a library-level, single-interrupted-step "sequencer". For that matter, 
> > > > it should also apply to the common '"git merge" gets a conflict' case, and 
> > > > it would be useful to get some representational uniformity between that 
> > > > and cherry-pick getting a conflict.
> > 
> > [...]

Thanks for the detailed response- I've rearragned your response and
added some comments.  I initially wanted to design it so that all
state is persisted by the sequencer, but I can clearly see what's
wrong with that approach now.

> I think, ultimately, that with this code structure in place, the 
> am/rebase/rebase--interactive/sequencer details of how the multi-step 
> process is recorded becomes less important. That way, your project can be 
> successful even if you can't find a syntax for the sequencer file that 
> meets the needs of all of these use cases. (Which is where I suspect 
> you'll get bogged down.) If you can get all of the cases where git exits 
> in order to get human intervention to share "everything that matters" and 
> the core to "know what's in progress as far as anything else cares", I 
> think that would be success, even if the various multi-step programs 
> continue using their own state files.

Excellent.  The crux of the idea: The sequencer should serve as the
entry/ exit point for Git when any operation requires user
intervention to proceed.  For this, it should have information about
how we got to this point, and how to proceed after the user
intervention is complete; this information is contained in:

> cherry_pick_conflict = { 
>   "cherry-pick", APPLIES_TO_CURRENT_BRANCH | IN_MIDDLE_OF_COMMIT,
>   cherry_pick_verify_resolution,
>   cherry_pick_abort,
>   cherry_pick_post_resolution
> };

Wait -- isn't it missing a skip callback?

cherry_pick_conflict = { 
  "cherry-pick", APPLIES_TO_CURRENT_BRANCH | IN_MIDDLE_OF_COMMIT,
  cherry_pick_verify_resolution,
  cherr_pick_skip,
  cherry_pick_abort,
  cherry_pick_post_resolution
};

This information is passed to report_conflict(), which takes care of
user intervention.  The user can do whatever she wants and then ask
the sequencer to "continue", "skip" or "abort":

> Where the sequencer-level conflict nests around the cherry-pick-level 
> conflict, and the generic "continue" completes things from the inside out.

Right.  And then the sequencer fires the appropriate callback and
returns control to the parent command.  More notes:

>  - cherry-pick can save whatever it needs to in its state file; that's 
>    its business, and the semantics here don't have to interact with other 
>    commands, because report_conflict() has taken care of interaction with 
>    other commands

At the end of a merge for example, the MERGE_MSG needs to be retrieved
to create a new merge commit.  The sequencer des not need to know
anything about this, since this is specific to 'merge'.

>  - arbitrary code can determine that you're in the middle of resolving 
>    some conflict, that the resolution of that conflict is about doing
>    something to your current branch, and how to abort what you're doing,
>    and how to finish it

Any arbitrary code simply has to ask the sequencer about the state of
the intermediate files that report_conflict() uses.  They don't have
to worry about command-specific intermediate files.

>  - the same code gets run after the conflict has been resolved that would 
>    have been run immediately if the merge went smoothly

Using these callbacks, there is no need for if-else ugliness inside
the specific command to decide what to do next.

I suppose we can call this  idea a "generic conflict handler".  I like
it very  much, and  I'll definitely  include this as  part of  my GSoC
work.  Thanks for taking the time to explain it in such detail :)

-- Ram

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [GSoC 2011] Git Sequencer
  2011-04-05 17:50         ` Ramkumar Ramachandra
@ 2011-04-05 18:24           ` Daniel Barkalow
  2011-04-05 18:59             ` Ramkumar Ramachandra
  0 siblings, 1 reply; 20+ messages in thread
From: Daniel Barkalow @ 2011-04-05 18:24 UTC (permalink / raw)
  To: Ramkumar Ramachandra
  Cc: Git List, Stephen Beyer, Christian Couder, Sverre Rabbelier,
	Jonathan Nieder

On Tue, 5 Apr 2011, Ramkumar Ramachandra wrote:

> Hi Daniel,
> 
> Daniel Barkalow writes:
> > On Mon, 4 Apr 2011, Ramkumar Ramachandra wrote:
> > > Ramkumar Ramachandra writes:
> > > > Daniel Barkalow writes:
> > > > > I actually think that it would be a worthwhile feature for git's library 
> > > > > code to have a uniform mechanism for communicating that it is requesting 
> > > > > human intervention in the middle of a particular operation, where library 
> > > > > operations which conflict with being able to continue this operation are 
> > > > > either blocked or abort the operation, and the library is able to be told 
> > > > > in general that the human intervention is done and the library operation 
> > > > > should be finished now (or produce complaints about the user's work). That 
> > > > > is, a library-level, single-interrupted-step "sequencer". For that matter, 
> > > > > it should also apply to the common '"git merge" gets a conflict' case, and 
> > > > > it would be useful to get some representational uniformity between that 
> > > > > and cherry-pick getting a conflict.
> > > 
> > > [...]
> 
> Thanks for the detailed response- I've rearragned your response and
> added some comments.  I initially wanted to design it so that all
> state is persisted by the sequencer, but I can clearly see what's
> wrong with that approach now.
> 
> > I think, ultimately, that with this code structure in place, the 
> > am/rebase/rebase--interactive/sequencer details of how the multi-step 
> > process is recorded becomes less important. That way, your project can be 
> > successful even if you can't find a syntax for the sequencer file that 
> > meets the needs of all of these use cases. (Which is where I suspect 
> > you'll get bogged down.) If you can get all of the cases where git exits 
> > in order to get human intervention to share "everything that matters" and 
> > the core to "know what's in progress as far as anything else cares", I 
> > think that would be success, even if the various multi-step programs 
> > continue using their own state files.
> 
> Excellent.  The crux of the idea: The sequencer should serve as the
> entry/ exit point for Git when any operation requires user
> intervention to proceed.

I'm a bit surprised by the idea of calling that "the sequencer" (rather 
than having "the sequencer" be a command), but I actually think you're 
entirely right to do so. Be sure to be very explicit about that, though, 
because people will probably start with the wrong idea of what you're 
proposing otherwise.

> For this, it should have information about
> how we got to this point, and how to proceed after the user
> intervention is complete; this information is contained in:
> 
> > cherry_pick_conflict = { 
> >   "cherry-pick", APPLIES_TO_CURRENT_BRANCH | IN_MIDDLE_OF_COMMIT,
> >   cherry_pick_verify_resolution,
> >   cherry_pick_abort,
> >   cherry_pick_post_resolution
> > };
> 
> Wait -- isn't it missing a skip callback?

I think "skip" is actually: abort the lowest-level conflict and continue 
the next-level conflict. If you're doing a rebase, and the rebase is doing 
a "pick", and the pick got a conflict, --skip means that you abort the 
pick (to get back to the state where the earlier commits have been picked 
but this one hasn't been started, followed by having the rebase continue 
with what it was going to do after the pick completed.

So I don't think you need a "skip" callback, as long as you've untangled 
the levels cleanly and get the nesting support right.

> cherry_pick_conflict = { 
>   "cherry-pick", APPLIES_TO_CURRENT_BRANCH | IN_MIDDLE_OF_COMMIT,
>   cherry_pick_verify_resolution,
>   cherr_pick_skip,
>   cherry_pick_abort,
>   cherry_pick_post_resolution
> };
> 
> This information is passed to report_conflict(), which takes care of
> user intervention.  The user can do whatever she wants and then ask
> the sequencer to "continue", "skip" or "abort":

Right, although I think:

  $ git cheery-pick some-sha1
  Conflict needs to be fixed now!

  $ git skip

should give an error message about the current conflict not being a step 
of a larger process. That is, you can always "continue" or "abort", but 
you can only "skip" if there's something to skip to, even if it's only the 
higher-order sequence reporting that it's completed successfully.

> > Where the sequencer-level conflict nests around the cherry-pick-level 
> > conflict, and the generic "continue" completes things from the inside out.
> 
> Right.  And then the sequencer fires the appropriate callback and
> returns control to the parent command.  More notes:
> 
> >  - cherry-pick can save whatever it needs to in its state file; that's 
> >    its business, and the semantics here don't have to interact with other 
> >    commands, because report_conflict() has taken care of interaction with 
> >    other commands
> 
> At the end of a merge for example, the MERGE_MSG needs to be retrieved
> to create a new merge commit.  The sequencer des not need to know
> anything about this, since this is specific to 'merge'.

Right. And remove_branch_state() wouldn't even need to know about 
MERGE_MSG like it does now, because that would be handled by aborting any 
in-progress merge.

> >  - arbitrary code can determine that you're in the middle of resolving 
> >    some conflict, that the resolution of that conflict is about doing
> >    something to your current branch, and how to abort what you're doing,
> >    and how to finish it
> 
> Any arbitrary code simply has to ask the sequencer about the state of
> the intermediate files that report_conflict() uses.  They don't have
> to worry about command-specific intermediate files.

Right.

> >  - the same code gets run after the conflict has been resolved that would 
> >    have been run immediately if the merge went smoothly
> 
> Using these callbacks, there is no need for if-else ugliness inside
> the specific command to decide what to do next.
> 
> I suppose we can call this  idea a "generic conflict handler".  I like
> it very  much, and  I'll definitely  include this as  part of  my GSoC
> work.  Thanks for taking the time to explain it in such detail :)

You're welcome. Thanks for proposing to actually implement it. :)

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [GSoC 2011] Git Sequencer
  2011-04-05 18:24           ` Daniel Barkalow
@ 2011-04-05 18:59             ` Ramkumar Ramachandra
  0 siblings, 0 replies; 20+ messages in thread
From: Ramkumar Ramachandra @ 2011-04-05 18:59 UTC (permalink / raw)
  To: Daniel Barkalow
  Cc: Git List, Stephen Beyer, Christian Couder, Sverre Rabbelier,
	Jonathan Nieder

Hi Daniel,

Daniel Barkalow writes:
> On Tue, 5 Apr 2011, Ramkumar Ramachandra wrote:
> > Excellent.  The crux of the idea: The sequencer should serve as the
> > entry/ exit point for Git when any operation requires user
> > intervention to proceed.
> 
> I'm a bit surprised by the idea of calling that "the sequencer" (rather 
> than having "the sequencer" be a command), but I actually think you're 
> entirely right to do so. Be sure to be very explicit about that, though, 
> because people will probably start with the wrong idea of what you're 
> proposing otherwise.

Ah.  I'll make sure to word it unambiguously in the proposal, and link
to this thread :)

> > For this, it should have information about
> > how we got to this point, and how to proceed after the user
> > intervention is complete; this information is contained in:
> > 
> > > cherry_pick_conflict = { 
> > >   "cherry-pick", APPLIES_TO_CURRENT_BRANCH | IN_MIDDLE_OF_COMMIT,
> > >   cherry_pick_verify_resolution,
> > >   cherry_pick_abort,
> > >   cherry_pick_post_resolution
> > > };
> > 
> > Wait -- isn't it missing a skip callback?
> 
> I think "skip" is actually: abort the lowest-level conflict and continue 
> the next-level conflict. If you're doing a rebase, and the rebase is doing 
> a "pick", and the pick got a conflict, --skip means that you abort the 
> pick (to get back to the state where the earlier commits have been picked 
> but this one hasn't been started, followed by having the rebase continue 
> with what it was going to do after the pick completed.
> 
> So I don't think you need a "skip" callback, as long as you've untangled 
> the levels cleanly and get the nesting support right.

Okay.  I'm not yet entirely clear about this yet, but I think it
should be sorted out during implementation.

> > cherry_pick_conflict = { 
> >   "cherry-pick", APPLIES_TO_CURRENT_BRANCH | IN_MIDDLE_OF_COMMIT,
> >   cherry_pick_verify_resolution,
> >   cherr_pick_skip,
> >   cherry_pick_abort,
> >   cherry_pick_post_resolution
> > };
> > 
> > This information is passed to report_conflict(), which takes care of
> > user intervention.  The user can do whatever she wants and then ask
> > the sequencer to "continue", "skip" or "abort":
> 
> Right, although I think:
> 
>   $ git cheery-pick some-sha1
>   Conflict needs to be fixed now!
> 
>   $ git skip
> 
> should give an error message about the current conflict not being a step 
> of a larger process. That is, you can always "continue" or "abort", but 
> you can only "skip" if there's something to skip to, even if it's only the 
> higher-order sequence reporting that it's completed successfully.

Right, got it.

-- Ram

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [GSoC 2011 v2] Git Sequencer
  2011-04-03 17:20 [GSoC 2011] Git Sequencer Ramkumar Ramachandra
                   ` (3 preceding siblings ...)
  2011-04-04  4:43 ` Christian Couder
@ 2011-04-05 20:00 ` Ramkumar Ramachandra
  2011-04-06  8:11   ` Christian Couder
  4 siblings, 1 reply; 20+ messages in thread
From: Ramkumar Ramachandra @ 2011-04-05 20:00 UTC (permalink / raw)
  To: Git List
  Cc: Stephen Beyer, Christian Couder, Daniel Barkalow,
	Sverre Rabbelier, Jonathan Nieder

Hi,

Thanks for all the feedback on the first iteration!

This iteration of the proposal has already been submitted via the
Melange interface. More comments/ feedback are always welcome.

======================================================================
Project Proposal: Git Sequencer
Student: Ramkumar Ramachandra
Mentor: Christian Couder

== The Objective ==

Extend 'git cherry-pick' with '--continue', '--abort', and '--skip'
features.  This will ultimately be used to write git-sequencer, a new
builtin command.  The sequencer will provide a uniform interface over
which existing commands like 'rebase', 'rebase -i' and 'am' can be
re-implented.  This should give the commands more functionality,
improve their error handling, and make them faster.  The project can
only be considered successful if most of the code written gets merged
into upstream.

The Git Sequencer was a 2008 GSoC project as well; unfortunately most
of the code did not get merged into git.git.  The learning from all
that work should serve as a huge headstart this year [1].

=== The Plan ===

1. Extend 'cherry-pick' with '--continue', '--abort', and '--skip'
features, so that it works like (a subset of) the current
'git-rebase--interactive.sh'.  This will require patching
'builtin/revert.c' in place, and merging it immediately.  I plan to
roughly follow the road laid out by Christian's 2010 series [2].

1.1. Factor out all calls to 'die' with 'return error' so so that we
can pause the entire process when a commit doesn't apply
automatically.

1.2. Create and populate TODO and DONE files, similar to the one that
'git-rebase--interactive.sh' creates.  For now, it should simply give
us information about why a 'cherry-pick' failed.

1.3. Port selective tests from the current 't3404' to make sure that
TODO and DONE are populated correctly; "stop on conflicting pick" is a
good candidate.

1.4. Decouple the 'revert' functionality from the 'cherry-pick'
functionality in 'revert.c'.  Implement '--abort' for 'cherry-pick'
and port "abort" test from 't3404'.

1.5. Implement parsing the TODO and DONE files into suitable data
structures.  Derive inspiration from the code written in 2008 to do
this [3].

1.6. Implement '--continue' and '--skip', and write suitable tests.

2. Build a sequencer so that just has cherry-picking functionality.
This mostly involves moving code written in (1) around, and crafting a
general API for handling conflicts.

2.1. Factor out the 'cherry-pick' code from 'revert.c' into a new
'builtin/sequencer.c'.

2.2. Write an API for handling conflicts, so that the sequencer is
ultimate entry/ exit point for all user intervention in a multi-step
process [4].

2.3. Implement a fresh 'cherry-pick.c' on top of the sequencer.  Make
sure that all the existing tests pass.

2.4 [Optional] Patch 'builtin/merge.c' to use the conflict handler in
the sequencer.

3. Extend the sequencer to accomodate the functionality provided by
'rebase -i'.

3.1. Parse commands like 'execute', 'reword', 'squash', and 'fixup'
that are specific to interactive rebasing.  Carefully implement the
functionality for each of these keywords in a step-wise manner.

3.2. [Optional] Port the '--preserve-merges' option of 'rebase' to the
sequencer.  Port relevant tests from 't3409'.

4. [Optional] Lib'ify the sequncer. Modify the API to
include rebase-related functionality.  Write 'rebase.c' as a bunch of
API calls to the sequencer.  Make sure that the existing tests pass.

5. [Optional] Re-implement 'git-am.sh' as a thin wrapper over the
sequncer: 'am.c'.  Bulk of this should be mbox parsing code.  Make sure
that all existing tests pass.

[Optional] should be read as "If time permits"

== Timeline ===

- Before mid june: (1) should be implemented, and a series should be
  sent out to the list for review.

- Before midterm evaluation: (1) should be merged, and an
  implementation of (2) should be sent to the list.

- Before the end of July: (2) should be merged, and an implementation
  of (3) should be sent to the list.

- Before final evaluation: (3) should be merged.

== Who am I? ==

I'm Ramkumar Ramachandra, and I first started contributing to git.git
in January 2010.  Apart from doing fast-import and remote helper
related work last year, I also authored and merged svnrdump into
Subversion trunk in the same period.

== Notes ==

[1]: http://repo.or.cz/w/git/sbeyer.git
[2]: http://thread.gmane.org/gmane.comp.version-control.git/162183
[3]: http://article.gmane.org/gmane.comp.version-control.git/162198
[4]: http://thread.gmane.org/gmane.comp.version-control.git/170834
======================================================================

Thanks for reading.

-- Ram

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [GSoC 2011 v2] Git Sequencer
  2011-04-05 20:00 ` [GSoC 2011 v2] " Ramkumar Ramachandra
@ 2011-04-06  8:11   ` Christian Couder
  2011-04-06  9:01     ` Ramkumar Ramachandra
  0 siblings, 1 reply; 20+ messages in thread
From: Christian Couder @ 2011-04-06  8:11 UTC (permalink / raw)
  To: Ramkumar Ramachandra
  Cc: Git List, Stephen Beyer, Christian Couder, Daniel Barkalow,
	Sverre Rabbelier, Jonathan Nieder

Hi,

On Tue, Apr 5, 2011 at 10:00 PM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
>
> 2. Build a sequencer so that just has cherry-picking functionality.
> This mostly involves moving code written in (1) around, and crafting a
> general API for handling conflicts.
>
> 2.1. Factor out the 'cherry-pick' code from 'revert.c' into a new
> 'builtin/sequencer.c'.
>
> 2.2. Write an API for handling conflicts, so that the sequencer is
> ultimate entry/ exit point for all user intervention in a multi-step
> process [4].

This one may be more difficult than we can guess right now, and it is
linked with 2.4 that is optional too. And I wouldn't like you to spend
too much time on it if it appears to be quite difficult. So I'd
suggest that you switch 2.2 and 2.3 and make the new 2.3 optional.

> 2.3. Implement a fresh 'cherry-pick.c' on top of the sequencer.  Make
> sure that all the existing tests pass.
>
> 2.4 [Optional] Patch 'builtin/merge.c' to use the conflict handler in
> the sequencer.
>
> 3. Extend the sequencer to accomodate the functionality provided by
> 'rebase -i'.

s/accomodate/accommodate/

> 3.1. Parse commands like 'execute', 'reword', 'squash', and 'fixup'
> that are specific to interactive rebasing.  Carefully implement the
> functionality for each of these keywords in a step-wise manner.

I think that here you could add something like:

3.2. Make 'rebase -i' use the sequencer when '--preserve-merges'
option is not used.

> 3.2. [Optional] Port the '--preserve-merges' option of 'rebase' to the
> sequencer.  Port relevant tests from 't3409'.

> 4. [Optional] Lib'ify the sequncer.

s/sequncer/sequencer/

> Modify the API to
> include rebase-related functionality.  Write 'rebase.c' as a bunch of
> API calls to the sequencer.  Make sure that the existing tests pass.
>
> 5. [Optional] Re-implement 'git-am.sh' as a thin wrapper over the
> sequncer: 'am.c'.

s/sequncer/sequencer/

> Bulk of this should be mbox parsing code.  Make sure
> that all existing tests pass.
>
> [Optional] should be read as "If time permits"

Otherwise I like it very much.

Thanks,
Christian.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [GSoC 2011 v2] Git Sequencer
  2011-04-06  8:11   ` Christian Couder
@ 2011-04-06  9:01     ` Ramkumar Ramachandra
  0 siblings, 0 replies; 20+ messages in thread
From: Ramkumar Ramachandra @ 2011-04-06  9:01 UTC (permalink / raw)
  To: Christian Couder
  Cc: Git List, Stephen Beyer, Christian Couder, Daniel Barkalow,
	Sverre Rabbelier, Jonathan Nieder

Hi Christian,

Christian Couder writes:
> On Tue, Apr 5, 2011 at 10:00 PM, Ramkumar Ramachandra
> <artagnon@gmail.com> wrote:
> >
> > 2. Build a sequencer so that just has cherry-picking functionality.
> > This mostly involves moving code written in (1) around, and crafting a
> > general API for handling conflicts.
> >
> > 2.1. Factor out the 'cherry-pick' code from 'revert.c' into a new
> > 'builtin/sequencer.c'.
> >
> > 2.2. Write an API for handling conflicts, so that the sequencer is
> > ultimate entry/ exit point for all user intervention in a multi-step
> > process [4].
> 
> This one may be more difficult than we can guess right now, and it is
> linked with 2.4 that is optional too. And I wouldn't like you to spend
> too much time on it if it appears to be quite difficult. So I'd
> suggest that you switch 2.2 and 2.3 and make the new 2.3 optional.
> 
> [...]

Okay.  Fixed other nits as well.

Thanks.

-- Ram

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2011-04-06  9:02 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-03 17:20 [GSoC 2011] Git Sequencer Ramkumar Ramachandra
2011-04-03 17:24 ` Sverre Rabbelier
2011-04-03 19:07 ` Stephan Beyer
2011-04-03 20:00   ` Ramkumar Ramachandra
2011-04-03 20:08   ` Jonathan Nieder
2011-04-03 19:49 ` Daniel Barkalow
2011-04-04  4:06   ` Ramkumar Ramachandra
2011-04-04  4:54     ` Ramkumar Ramachandra
2011-04-04 18:59       ` Daniel Barkalow
2011-04-05 17:50         ` Ramkumar Ramachandra
2011-04-05 18:24           ` Daniel Barkalow
2011-04-05 18:59             ` Ramkumar Ramachandra
2011-04-04  4:43 ` Christian Couder
2011-04-04  5:20   ` Junio C Hamano
2011-04-05  6:23     ` Christian Couder
2011-04-05  6:46       ` Ramkumar Ramachandra
2011-04-04 16:57   ` Ramkumar Ramachandra
2011-04-05 20:00 ` [GSoC 2011 v2] " Ramkumar Ramachandra
2011-04-06  8:11   ` Christian Couder
2011-04-06  9:01     ` Ramkumar Ramachandra

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).