Re: [PATCH 7/7] sequencer: Remove sequencer state after final commit

[Jonathan Nieder <jrnieder@gmail.com>]

Hi Ram,

Ramkumar Ramachandra wrote:

> Here are some comments from my end after extensive thought.

Could be briefer. :)

[...]
> 1. Introduce a 'merge --continue' to invoke 'git commit'.  MERGE_HEAD
> helps 'git commit' finish.  Modify tests to use '--continue' instead
> of the earlier commit-to-finish workflow, and advertise this feature
> everywhere.

Why modify tests?  I think "git merge --continue" is a nice idea,
and I don't see how it's inconsistent in any way with continuing to
allow old practice.

> 2. Make 'cherry-pick --continue' invoke 'git commit' as well.
> CHERRY_PICK_HEAD helps 'git commit' finish.  If the commit finishes
> successfully: (if there is one commit left, remove the sequencer
> state; otherwise, drop the first insn on top of the list and execute
> the next insn).

Sounds like a sensible thing to do.  I assume the "one" in the
parenthesis is supposed to be "zero", making the "if" not even part of
the user-visible description of what it does --- it's just the
termination condition of a loop.

"git cherry-pick --continue" in place of "git commit" does not handle
the following scenario.  Suppose my multiple-cherry-pick has run into
conflicts, and while fixing them I notice something related that needs
to be fixed.

	... resolve conflict, leaving extra change in worktree ...
	git stash -k
	... test test test ...
	git commit

	git stash pop
	git commit; # make a separate commit for extra change

	# ok, now continue.
	git sequencer --continue

In other words, in this sequence of commands, "git commit" is used to
single-step.  So if one wants to remove CHERRY_PICK_HEAD altogether, a
nice thing to do would be to introduce a "git sequencer --single-step"
command or something similar to handle such cases.

> Modify tests to use '--continue' instead of the
> earlier commit-to-finish workflow, and advertise this feature
> everywhere.  Unfortunately, if the user executes 'git commit' instead
> of the newer '--continue', we're screwed: a stray sequencer state will
> be left behind.

As Junio hinted, it could make a lot of sense for "git cherry-pick
<single commit>" to not create sequencer state in the first place.
"git cherry-pick --continue" does not need it --- it is enough to
commit with the conflict resolved.  "git cherry-pick --abort" does not
need it, either --- it is enough to "git reset --merge HEAD".

One part I'm handwaving is what to do about commands like "git
cherry-pick foo^..foo" which use a commit range that only happens to
contain one commit.  Either behavior seems fine for such commands.

What do you think?