From: Phillip Wood <phillip.wood123@gmail.com>
To: Taylor Blau <me@ttaylorr.com>, Victoria Dye <vdye@github.com>
Cc: git@vger.kernel.org, Phillip Wood <phillip.wood123@gmail.com>,
"herr . kaste" <herr.kaste@gmail.com>,
Derrick Stolee <derrickstolee@github.com>
Subject: Re: [PATCH] rebase --update-refs: avoid unintended ref deletion
Date: Fri, 4 Nov 2022 20:17:53 +0000 [thread overview]
Message-ID: <1ad5f905-f9f1-5a06-e419-476032a0d237@dunelm.org.uk> (raw)
In-Reply-To: <Y2VrhR6b0SzG1HEA@nand.local>
Hi Taylor
On 04/11/2022 19:44, Taylor Blau wrote:
> 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
> 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
next prev parent reply other threads:[~2022-11-04 20:17 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-20 17:01 rebase -i --update-refs can lead to deletion of branches herr.kaste
2022-10-20 20:49 ` Erik Cervin Edin
2022-11-03 9:32 ` Phillip Wood
2022-11-03 15:25 ` herr.kaste
2022-11-03 16:52 ` Erik Cervin Edin
2022-11-04 0:31 ` Victoria Dye
2022-11-04 10:40 ` Phillip Wood
2022-11-04 15:28 ` Victoria Dye
2022-11-04 16:57 ` [PATCH] rebase --update-refs: avoid unintended ref deletion Victoria Dye
2022-11-04 19:44 ` Taylor Blau
2022-11-04 20:17 ` Phillip Wood [this message]
2022-11-04 20:12 ` Phillip Wood
2022-11-07 2:39 ` Derrick Stolee
2022-11-07 17:47 ` [PATCH v2] " Victoria Dye
2022-11-07 19:17 ` Taylor Blau
2022-11-07 19:25 ` Derrick Stolee
2022-11-08 9:58 ` Phillip Wood
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1ad5f905-f9f1-5a06-e419-476032a0d237@dunelm.org.uk \
--to=phillip.wood123@gmail.com \
--cc=derrickstolee@github.com \
--cc=git@vger.kernel.org \
--cc=herr.kaste@gmail.com \
--cc=me@ttaylorr.com \
--cc=phillip.wood@dunelm.org.uk \
--cc=vdye@github.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).