Re: [PATCH 16/17] revert: Introduce --reset to remove sequencer state

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Jul 17, 2011, 17:10 UTC
Message-ID: <CALkWK0mfNNfpyyMDFgVD_gnEC7OxHuPAPc+27-PepYNKPOR8-w@mail.gmail.com>
In-Reply-To: <20110712203011.GF14909@elie>

Hi,

On Wed, Jul 13, 2011 at 2:00 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 45 quoted lines

> Ramkumar Ramachandra wrote:
>> Protect the sequencer state from accidentally being stomped by a new
>> cherry-pick or revert invocation by ensuring that an existing one
>> isn't in progress.
>
> I first read this as "an existing sequencer state isn't in progress".
> But anyway, if I understand correctly the goal isn't to protect the
> sequencer state from corruption but to protect the user from forgetting
> about a pending cherry-pick.
>
>> While this patch would normally be expected to
>> break many tests, the earlier patches "reset: Make hard reset remove
>> the sequencer state" and "revert: Remove sequencer state when no
>> commits are pending" make sure that they don't.
>
> Why would I expect a nice change to break tests?
>
> I suppose you mean: "A naive version of this would break the following
> established way of working:
>
>        git cherry-pick X; # has conflicts
>        git reset --hard; # no, no!
>        git cherry-pick Y
>
> Or even:
>
>        git cherry-pick X; # has conflicts
>        # ... resolve ...
>        git commit
>        git cherry-pick Y
>
> But a previous patch takes care of that by making "git reset --hard"
> cancel the pending cherry-pick and by making "git commit" clean up
> after a pending cherry-pick when making the commit that would finish
> it."
>
> The above text quoted with ">" describes the main impact of the
> change.  The subject line, on the other hand, describes a less
> important part:
>
>> [Subject: revert: Introduce --reset to remove sequencer state]
>
> Is that fixable?  Maybe this could be split into two patches (since it
> does two different things), or maybe the subject line could be tweaked
> to describe both.

New commit message.

revert: Don't clobber sequencer state; introduce --reset

Protect the user from forgetting about a pending operation by immediately erroring out when an existing cherry-pick or revert operation is in progress. A naive version of this would break the following established ways of working:

$ git cherry-pick foo ... conflict ... $ git reset --hard # I actually meant "moo" when I said "foo" $ git cherry-pick moo

$ git cherry-pick foo ... conflict ... # resolve the conflict $ git commit # commit the resolution $ git cherry-pick moo # New operation

But the previous patches "reset: Make hard reset remove the sequencer state" and "revert: Remove sequencer state when no commits are pending" make sure that this does not happen.

To explicitly remove the sequencer state for a fresh cherry-pick or revert invocation, introduce a new subcommand called '--reset' which really removes the sequencer state on the very first invocation.

Show 8 quoted lines

>> Ensure that the "rebase -i" script which invokes cherry-pick or revert
>> doesn't change its behavior by using '--reset' to to clear the state
>> after every failed pick.
>
> This should be avoidable by noticing that commands like "rebase -i"
> use GIT_CHERRY_PICK_HELP to clobber our nice instructions about how to
> resume a cherry-pick anyway and therefore are unlikely to need
> cherry-pick --continue/--abort facilities, no?

Right.  Removed hunk and corresponding segment in commit message.

Thanks.

-- Ram