Re: 'git stash push' isn't atomic when Ctrl-C is pressed

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jan 27, 2022, 02:36 UTC
Message-ID: <220127.86zgnhdhft.gmgdl@evledraar.gmail.com>
In-Reply-To: <xmqqczkeozpx.fsf@gitster.g>

On Wed, Jan 26 2022, Junio C Hamano wrote:

Show 8 quoted lines

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> I understood this problem as being one where we do the ref work first,
>> which we could start a transaction for, the user then ctrl+c's after the
>> ref work is done, but before the working tree is updated.
>
> If the process is killed while the working tree is half updated,
> nothing you do with ref transaction will help.

This thread is about Ctrl+C, i.e. SIGINT, but presumably we'd use sigchain_push_common() which covers that and various other signals.

Of course if you're talking about SIGKILL all bets are off.

Show 10 quoted lines

>>> I suspect that the untold thinking behind your statement was that we
>>> should try not to discourage new users from asking, and I agree with
>>> the sentiment to a certain degree.  But at the same time, I think it
>>> is simply irresponsible to do so without distinguising between
>>> asking for something realistic and unrealistic.
>>
>> I must admit I'm not deeply familiar with builtin/stash.c in particular,
>
> Then you can try to be on the conservative side, perhaps, to avoid
> misleading less experienced folks next time?  Thanks.

I meant I'm not too familiar with details of how "git stash" interacts with the working tree etc., which is part of what you brought up in your reply.

But I was mainly commenting on what I think is a fairly straightforward way to address the original report of wanting references rolled back on SIGINT, i.e. to move it to the reference transaction API, and to have it roll back changes in certain scenarios.

Of course it's possible that I'm just missing something, do you see a reason for why having a signal handler be responsible for rolling back a reference transaction wouldn't work?