Re: [PATCH 12/14] revert: Introduce --reset to cleanup sequencer data

Hi again,

Jonathan Nieder writes:

Ramkumar Ramachandra wrote:

quoted

quoted

--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh

[...]

quoted

As you can see, there is no "reset --hard" in these

In this one, there's "git checkout -f && git read-tree -u --reset HEAD
&& git clean -fdx", which is almost the same thing.

Ofcourse, but it's a combination of three commands -- are you
suggesting that I replace this sequence of commands with "git reset
--hard"?.

quoted

quoted

--- a/t/t3505-cherry-pick-empty.sh
+++ b/t/t3505-cherry-pick-empty.sh

This one is not a typical script, I think --- if you knew the
cherry-pick was going to be empty, why did you try it in the first
place?  I think it would make sense to make it "git reset --hard" at
the beginning of each test as a separate, preparatory patch with
explanation.

[...]

quoted

There is however, one other thing I can do: if there is
nothing left to cherry-pick after a successful conflict resolution +
git commit, I can modify commit.c to blow away the sequencer state
after checking appropriately.  This will also have a nice end-user
experience side-effect:
$ git cherry-pick moo
fatal: Conflict in foo!
$ echo "Resolved" > foo
$ git add moo
$ git commit
$ git cherry-pick --continue # This no-op will be unnecessary

Though it's not obvious to me how this would affect the scripts above,
it sounds like a nice enhancement to me independently, fwiw.

Oh, it fixes everything :)
Just see my GitHub fork.

quoted

Then again, teaching commit about the sequencer is inelegant,

It's possible to add some hook-like thing to do this, or to structure
the code as if a hook was used.

quoted

and it's
possible to achieve this effect in another way: when a conflict is
encountered in the sequencer && length(todo_file) == 1, throw away the
sequencer state.

Yep, that seems like basically the same effect.  Are there downsides?
(Maybe years from now when a "git cherry-pick --rewind" is introduced
we would regret this?  But that can be figured out years from now.)

Doh, I'm not thinking straight.  Would this break "git cherry-pick
--abort", or is there some hack layered on top to avoid that?  Making
"git commit" remove the .git/sequencer for the last commit of the
sequence seems a little saner.

We could always inject a hack to avoid this.  I'm not yet convinced
that we should teach commit about the sequencer, especially since the
patch to do it from the sequencer end is so simple.  Please have a
look at the patch [1], and let me know what you think.  I could submit
it as an RFC patch to the list for convenience, but I'm afraid it'll
be missing context.

[1]: https://github.com/artagnon/git/commit/0653bcccfa8d69687ed939f07f5b32dd14d302d3

-- Ram