Re: [PATCH] rebase --update-refs: avoid unintended ref deletion

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Nov 4, 2022, 20:17 UTC
Message-ID: <1ad5f905-f9f1-5a06-e419-476032a0d237@dunelm.org.uk>
In-Reply-To: <Y2VrhR6b0SzG1HEA@nand.local>

Hi Taylor

On 04/11/2022 19:44, Taylor Blau wrote:

Show 11 quoted lines

> On Fri, Nov 04, 2022 at 09:57:36AM -0700, Victoria Dye wrote:
>> However, because 'write_update_refs_state()' will not update the state if
>> the 'refs_to_oids' list was empty, removing *all* 'update-ref' lines will
>> result in the state remaining unchanged from how it was initialized (with
>> all refs' "after" OID being null). Then, when the ref update is applied, all
>> refs will be updated to null and consequently deleted.
> 
> Good catch.
> 
> I wonder, though: should we only add pending ref updates to the
> update-refs state after we reach that point in the sequence?

If I remember correctly the aim of the current behavior is to detect if another process also updates the ref while we're rebasing. To do that we need to record all the branch heads that have update-ref commands at the start of the rebase.

Best Wishes

Phillip

Show 17 quoted lines

> IOW: there is no world where deleting an update-refs command means to
> drop the affected branch, right? So the initial state would be an empty
> list, which would cause us to not update any references.
> 
> Then as we proceed through the rebase, we accumulate update-refs
> commands, and know their after_oid immediately. Then when we're done, we
> can process the update-refs commands for the branches that we do have.
> 
> The more I think about this, the more that I am convinced that the bug
> is in how we initialize the pending list, not our treatment of it later
> on.
> 
> The bug fix works as-is, but I can't help wonder if the above approach
> might be more direct.
> 
> Thanks,
> Taylor