Re: [PATCH v3 6/8] rebase: add --update-refs option

From: Elijah Newren <newren@gmail.com>
To: Derrick Stolee <derrickstolee@github.com>
Cc: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>,
	Git Mailing List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Taylor Blau <me@ttaylorr.com>,
	Jeff Hostetler <git@jeffhostetler.com>,
	Phillip Wood <phillip.wood123@gmail.com>
Subject: Re: [PATCH v3 6/8] rebase: add --update-refs option
Date: Thu, 7 Jul 2022 19:27:04 -0700	[thread overview]
Message-ID: <CABPp-BFQbG-qQK2biKMV7cE641BzNwk324Gn5eEFUyiz7k4c4g@mail.gmail.com> (raw)
In-Reply-To: <84da5ea6-b6a9-7756-66f4-a3832144a68e@github.com>

On Tue, Jul 5, 2022 at 3:22 PM Derrick Stolee <derrickstolee@github.com> wrote:
>
> On 7/4/22 11:57 AM, Elijah Newren wrote:
> > Actually, I checked out ds/rebase-update-ref just now to try it, and
> > it seems like it does the right thing:
> >
> >     pick 111111 first commit
> >     pick 222222 second commit
> >     fixup 555555 fixup! second commit
> >     update-ref refs/heads/branch1
> >
> >     pick 333333 third commit
> >     pick 444444 fourth commit
>
> Thanks for trying it out! This is definitely the main goal of the
> feature, although it is also helpful when resolving conflicts or
> doing 'edit' steps.
>
> >     # Ref refs/heads/branch2 checked out at '...'
> >
> > The last line was very disorienting to me at first and made me think
> > we had a bug, but the update-refs stuff is built on top of the normal
> > rebase mechanism and branch2 will be updated by that logic rather than
> > by the special update-refs handling.  If I add another branch with a
> > few commits on top of branch2, then branch2 is indeed updated and
> > after the pick of 444444 (and the additional branch, say branch3,
> > would be updated by the normal rebase logic instead of by the
> > update-refs handling).  So it all works correctly, but users might get
> > worried or confused along the way wondering whether it will function
> > correctly.
>
> I'll add a patch that removes the comment in the case of the HEAD
> ref. Thanks for the idea!

Thanks, that'd improve things.  I'm curious whether it'd be even
better to have an update-ref line for HEAD, so that users don't wonder
whether it's omitted from the updates.  (That would leave an open
question whether you filter out HEAD before actually calling
update-ref, or use some other mechanism to make it all work behind the
scenes.)

> > Another part that users might find disorienting is that at the end,
> > the rebase reports:
> >     Successfully rebased and updated refs/heads/branch2.
> > which is correct but totally ignores the fact that it *also* rebased
> > and updated other branches.
>
> Good point. I can add an extra message at the end (as well as a
> warning for any refs that did not properly update at the end).

Sounds good.