All of lore.kernel.org
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: Victoria Dye <vdye@github.com>,
	"herr.kaste" <herr.kaste@gmail.com>,
	"git@vger.kernel.org" <git@vger.kernel.org>,
	Derrick Stolee <derrickstolee@github.com>,
	Phillip Wood <phillip.wood123@gmail.com>,
	erik@cervined.in
Subject: Re: rebase -i --update-refs can lead to deletion of branches
Date: Fri, 4 Nov 2022 10:40:39 +0000	[thread overview]
Message-ID: <c195b67c-4dbc-a8b8-8513-2664e1ca2404@dunelm.org.uk> (raw)
In-Reply-To: <123628cc-1410-aaa0-0151-2dff35bd1855@github.com>

Hi Victoria

On 04/11/2022 00:31, Victoria Dye wrote:
> herr.kaste wrote:
>> Now, I should just have not used `--update-refs` in the first place but anyway
>> I decide late that I rather don't want to update "master" etc. and it should
>> probably not delete the local refs.
>>
> 
> Agreed, this doesn't seem like desired behavior - the opposite of "update
> the ref" isn't "delete the ref". ;)
> 
> The reason it's happening is because, when '--update-refs' is used, the
> rebase starts by constructing a list of 'update_ref_record's for each of the
> refs that *could* be updated. Each item in that list contains the
> corresponding ref's "before" commit OID (i.e., what it currently points to)
> and initializes the "after" OID to null. When an 'update-ref' line is
> encountered in the 'rebase-todo', the "after" OID is updated with the
> newly-rebased value. However, if an 'update-ref' line is removed from the
> 'rebase-todo', the "after" value is never updated. Then, when the rebase
> finishes and the ref state data is applied, all of the entries with null
> "after" OIDs are deleted.
> 
> The three options for a fix I can think of are:
> 
>    1. initialize the "after" OID to the value of "before".
>    2. don't update refs with a null "after" OID.
>    3. initialize the "after" OID to the value of "before", don't update the
>       ref if "before" == "after".
> 
> I think #3 is the best option, since it avoids the unnecessary updates of #1
> and leaves a cleaner path to a 'delete-ref' option (like the one proposed
> elsewhere in the thread [1]) than #2. I'll send a patch shortly.

We should be removing the entry entirely if the user removes it from the 
todo-list see b3b1a21d1a (sequencer: rewrite update-refs as user edits 
todo list, 2022-07-19) where the commit message says

1. If a '<ref>/<before>/<after>' triple in the update-refs file does not
    have a matching 'update-ref <ref>' command in the todo-list _and_ the
    <after> value is the null OID, then remove that triple. Here, the
    user removed the 'update-ref <ref>' command before it was executed,
    since if it was executed then the <after> value would store the
    commit at that position.

I think that is the best approach but it seems the implementation isn't 
actually doing that.

Best Wishes

Phillip



> [1] https://lore.kernel.org/git/CA+JQ7M-GbBTHZZ9xOLR=FitWFpUnkfuep9kSfNPxuSbJbKteGw@mail.gmail.com/
> 
> Thanks for reporting!
> - Victoria
> 

  reply	other threads:[~2022-11-04 10:40 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 [this message]
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
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=c195b67c-4dbc-a8b8-8513-2664e1ca2404@dunelm.org.uk \
    --to=phillip.wood123@gmail.com \
    --cc=derrickstolee@github.com \
    --cc=erik@cervined.in \
    --cc=git@vger.kernel.org \
    --cc=herr.kaste@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.