Re: [PATCH 14/17] reset: Make hard reset remove the sequencer state

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Jul 17, 2011, 16:40 UTC
Message-ID: <CALkWK0=XP=hwsN8vVmTc2Rs3mYuZnLZOpg4=OE7mrYbAYm6nCQ@mail.gmail.com>
In-Reply-To: <20110712201532.GE14909@elie>

Hi again,

Jonathan Nieder writes:

Show 30 quoted lines

> Ramkumar Ramachandra wrote:
>> Years of muscle memory have trained users to use "git reset --hard" to
>> remove away the branch state after any sort of operation.  In
>> retrospect, while this may not have been the best design decision, we
>> are stuck with it for historical reasons.
>
> Wait, wait!  If that was a bad design decision, we should try to find
> a way to smoothly transition to a world without it, instead of
> layering workarounds on top of it.
>
> But actually I think it is good UI.  When you do "git merge", it works
> like this:
>
>        git merge <foo>; # conflicts!
>        ... hack hack hack ...
>        # Oh, bother, let me go back to a state I know well and am
>        # comfortable with.
>        git reset --hard <bar>
>
> And the same psychological effect applies in the cherry-pick case:
>
>        git cherry-pick <foo>; # conflicts!
>        ... hack hack hack ...
>        # Oh, bother, let me go back to a state I know well and am
>        # comfortable with.
>        git reset --hard <bar>
>
> See, it's about the tool working with you.  When I abandon a merge,
> I don't want to have to search through the owner's manual for the
> button to get git to clear away this unpleasant and unfamiliar state.

Fair enough. It's a large hammer to solve the problem, and it's used by many people; I use it a lot myself.

Show 15 quoted lines

> Now, by contrast, sometimes one wants something less aggressive.
> For example, to abandon a partial merge conflict resolution but
> keep unrelated changes in the worktree:
>
>        git reset --merge HEAD
>
> Or to get out of the "mergy" state but leave the worktree alone:
>
>        git reset
>
> There's no porcelain or plumbing to abandon a rebase without
> additional side-effects, but "git status" suggests a command for
> it if I remember correctly.
>
>        rm -fr .git/rebase-merge

We're not making this mistake with the sequencer :)

Show 7 quoted lines

>> Additionally, this patch ensures that some existing tests don't break
>> when features like "--reset" and "--continue" are introduced later in
>> the series.
>
> That's not "Additionally" --- it's the same problem.  One way to
> phrase it would be "Noticed by running such-and-such test after
> such-and-such change".

Right.  Minor tweaks made to the commit message.

reset: Make reset remove the sequencer state

Years of muscle memory have trained users to use "git reset --hard" to remove the branch state after any sort operation. Make it also remove the sequencer state so that this sort of workflow is now possible:

$ git cherry-pick foo..bar ... conflict encountered ... $ git reset --hard # Oops, I didn't mean that $ git cherry-pick quux..bar ... cherry-pick succeeded ...

Also, guard against accidental removal of the sequencer state by moving ".git/sequencer" to ".git/sequencer-old" in the first "git reset --hard" call, and only remove it completely only after the second call.

This patch ensures that some existing tests don't break when features like "--reset" and "--continue" are introduced later in the series. Without this patch, we would expect the last cherry-pick call in the example to fail with the complaint that an existing cherry-pick operation is in progress.

Show 7 quoted lines

>> +test_expect_success 'reset --hard cleans up sequencer state' '
>
> Hoorah!  Thanks.
>
> Why isn't the .git/sequencer removal in remove_branch_state() like
> MERGE_MSG and other similar examples are?  (Not a rhetorical question;
> it would be interesting to know.)

I thought we should only make a hard reset clear it, but all the reset codepaths reach remove_branch_state(). However, in view of consistency, I've put the removal code in remove_branch_state() now.

-- Ram