From: Phillip Wood <phillip.wood123@gmail.com>
Date: Mon, 29 Apr 2019 15:10:53 GMT
Subject: Re: [PATCH v6 23/27] switch: reject if some operation is in progress
Message-ID: <38c4e7fb-0d32-7696-e2d0-83766027383d@gmail.com>
In-Reply-To: <CACsJy8ASYRwUGV8VnVKLhBqdv9V+UTAhb6U3+4BUjJFnJ+2Viw@mail.gmail.com>

On 29/04/2019 10:16, Duy Nguyen wrote:
> On Thu, Apr 25, 2019 at 5:33 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>>
>> On 29/03/2019 10:39, Nguyễn Thái Ngọc Duy wrote:
>>> Unless you know what you're doing, switching to another branch to do
>>> something then switching back could be confusing. Worse, you may even
>>> forget that you're in the middle of something. By the time you realize,
>>> you may have done a ton of work and it gets harder to go back.
>>>
>>> A new option --ignore-in-progress was considered but dropped because it
>>> was not exactly clear what should happen. Sometimes you can switch away
>>> and get back safely and resume the operation. Sometimes not. And the
>>> git-checkout behavior is automatically clear merge/revert/cherry-pick,
>>> which makes it a bit even more confusing [1].
>>>
>>> We may revisit and add this option in the future. But for now play it
>>> safe and not allow it (you can't even skip this check with --force).
>>
>> I think this is a good compromise, lets see how it goes (I think I
>> broadly agree with Elijah's suggestion to allow the switch if we can
>> safely switch back again if we want to add --ignore-in-progress in the
>> future).
> 
> I probably will revisit this topic much sooner than I thought. I did a
> bisect today and found out "git switch" would not let me choose some
> "random" commit to test, which I suspected more likely where the
> problem was, or at least helped reduce the bisect steps. I had to go
> back to "git checkout" and was not so happy.

Oh that's a pain, but should be safe as you describe below.

> This probably falls under the "safe to switch" (and not even back)
> category, as long as switching does not destroy any data, since bisect
> is basically jumping between commits with a clean worktree/index until
> you find the right one. >>> +static void die_if_some_operation_in_progress(void)
>>> +{
>>> +     struct wt_status_state state;
>>> +
>>> +     memset(&state, 0, sizeof(state));
>>> +     wt_status_get_state(the_repository, &state, 0);
>>> +
>>> +     if (state.merge_in_progress)
>>> +             die(_("cannot switch branch while merging\n"
>>> +                   "Consider \"git merge --quit\" "
>>> +                   "or \"git worktree add\"."));
>>
>> I'm not sure merge --quit exists, 'git grep \"quit origin/pu' shows
>> matches for builtin/{am.c,rebase.c,revert.c}. The --quit option for the
>> sequencer command does not touch the index or working tree (that's the
>> difference between --quit and --abort) so the switch can still fail due
>> changes in the index and worktree that would be overwritten by the switch.
> 
> Eck! Let me check if --abort is the same thing there or we need to add --quit...

I think the --quit options generally leave the index and worktree alone, 
they just remove the state files whereas abort resets the index and 
worktree (with reset --mixed for merge and cherry-pick/revert, I think 
rebase does reset --hard) and also rewinds HEAD for rebases and 
sequences of cherry-picks and revert but that wont apply to merges as 
they are one-shot operations.

Best Wishes

Phillip