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

[Ramkumar Ramachandra <artagnon@gmail.com>]

Hi again,

Jonathan Nieder writes:
> Ramkumar Ramachandra wrote:
>
>>> --- a/t/t3507-cherry-pick-conflict.sh
>>> +++ b/t/t3507-cherry-pick-conflict.sh
> [...]
>> 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"?.

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

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