Re: [PATCH 0/8] Sequencer Foundations

Hi,

On Wed, May 11, 2011 at 3:14 PM, Jonathan Nieder [off-list ref] wrote:

Ramkumar Ramachandra wrote:

quoted

Ramkumar Ramachandra (8):
  revert: Improve error handling by cascading errors upwards
  revert: Make "commit" and "me" local variables
  revert: Introduce a struct to parse command-line options into
  revert: Separate cmdline argument handling from the functional code
  revert: Catch incompatible command-line options early
  revert: Introduce head, todo, done files to persist state
  revert: Implement parsing --continue, --abort and --skip
  revert: Implement --abort processing

I had no time to look at this yet but I will try do to so in the coming days.

The heart is patch 6/8.  I have not thought about this deeply yet, but
I wonder if it would be simpler if the behavior of "git cherry-pick
1..10" looked like this:

. if there is state in .git/sequencer already, error out
. lock .git/sequencer/head with the lockfile API to prevent
 concurrent access
. write current state, including remaining commits to cherry-pick
. unlock .git/sequencer/head
. cherry-pick commit #1
. lock sequencer, check state, update state, unlock
. cherry-pick commit #2
 ...

This way, even if cherry-picking causes git to segfault, the sequencer
state is in good order and we know where to pick up.  More
importantly, massive refactoring of the merge_recursive API would not
be needed to keep everything in working order.  An atexit and sigchain
handler could be added to print advice for the reader about how to
resume, but that's just an extra hint and it's okay if it sometimes
doesn't happen sometimes.

What do you think?

I think that the risk at this point might be to overengineer things
and to lose time, and then we will perhaps find out that we need to do
some refactoring of the merge_recursive API anyway.
If we have cherry-pick with --abort, --continue and --skip that just
works as well or nearly as well (because it's new) as other stuff it
will be already a very good thing. And with enough tests we will
hopefully be able to build and refactor safely after that. Maybe we
will eventually find out that what you suggest is in fact needed even
for cherry-pick with --abort, --continue and --skip, but for now I
would prefer trying to make it work with as few changes and work as
possible.

Thanks,
Christian.