From: Phillip Wood Date: Sun, 03 Sep 2023 19:48:14 GMT Subject: Re: [PATCH] sequencer: update abort safety file more sparingly Message-ID: In-Reply-To: On 03/09/2023 20:25, Oswald Buddenhagen wrote: > On Sun, Sep 03, 2023 at 07:40:00PM +0100, Phillip Wood wrote: >> On 03/09/2023 16:11, Oswald Buddenhagen wrote: >>> The only situation where the file's content matters is --continue'ing >>> (after a multi-cherry-pick merge conflict). >> >> I don't think "cherry-pick --continue" consults the abort safety file, > duh, obvious blunder. > >> it only matters for "cherry-pick --skip" >> > that doesn't seem right. a --skip is just a --continue with a prior > reset, more or less. sequencer_skip() calls rollback_is_safe() which checks the abort safety file. >> and "cherry-pick --abort". >> > that one, of course. > >>> This means that it is >>> sufficient to write it in a single place, when we are prematurely >>> exiting the main workhorse. >> >> I think this introduces a regression because the safety file will not >> get updated when "cherry-pick --continue" stops for the user to >> resolve conflicts. >> > true, there is indeed this second entry point. > i'll try to find a better "choke point". I think that is probably tricky, I'm not really clear what the aim/purpose of this refactoring is. >>> which wasn't even reliable: a single pick executed during an >>> interrupted sequence would bypass the safety. >> >> An alternate view is that the abort safety file exists to prevent the >> user losing commits that have not been cherry-picked and it is >> desirable to be able to abort after cherry-picking a single pick in >> the middle of a sequence of cherry-picks. >> > if you did a fresh commit before or after the single pick, you'd lose it. > also, Oh, I can see that you'd lose a commit made before a single pick but I don't see how you'd lose a commit made after it. I'm still not convinced it is a particularly helpful change. > the feature doesn't actually prevent aborting, only the automatic > reset. Oh right, it removes the state directory but leaves HEAD untouched if it does not match the commit recorded in the abort safety file. Best Wishes Phillip > regards