Re: [PATCH v2] rebase: introduce --update-branches option

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi,

On Mon, 9 Sep 2019, Phillip Wood wrote:

> On 08/09/2019 00:44, Warren He wrote:
> > Everyone in this thread, thanks for your support and encouragement.
> > [...]
> > > It should not really imply `--interactive`, but `--rebase-merges`.
> >
> > `imply_interactive` doesn't fully switch on `--interactive`, i.e., causing
> > the
> > editor to open. It only selects the backend, which I think we're saying is
> > the
> > right thing. I've dropped the `-i` from the test description.
> >
> > And we don't really have to imply --rebase-merges, in case someone would
> > prefer
> > to linearize things, which who knows? Running that non-rebase-merges command
> > in
> > the example scenario from my original post should give something like this:
>
> I think it would probably be less confusing to default to preserving merges

s/preserving/rebasing/?

> and having an option to turn that off - people are going to be surprised if
> their history is linearized.

I don't think it makes any sense to linearize the history while updating
branches, as the commits will be all jumbled up. Imagine this history:

	- A - B - C - D -
	    \       /
	      E - F

Typically, it does not elicit any "bug" reports, but this can easily be
linearized to

	- A' - B' - E' - C' - F' -

In my mind, it makes no sense to update any local branches that pointed
to C and F to point to C' and F', respectively.

> [...]
> > * * *
> >
> > And then there's the discussion about using `exec git branch -f`. To
> > summarize
> > the issues collected from the entire thread:
> >
> > 1. the changes aren't atomically applied at the end of the rebase
> > 2. it fails when the branch is checked out in a worktree
> > 3. it clobbers the branch if anything else updates it during the rebase
> > 4. the way we prepare the unprefixed branch doesn't work right some exotic
> > cases
> > 5. the reflog message it leaves is uninformative
> >
> > For #4, I think we've lucked out actually. The `load_ref_decorations`
> > routine we
> > use determines that a ref is `DECORATION_REF_LOCAL` under the condition
> > `starts_with(refname, "refs/heads/")` (log-tree.c:114, add_ref_decoration),
> > so
> > `prettify_refname` will find the prefix and skip it. But that's an invariant
> > maintained by two pieces of code pretty far away from each other.
> >
> > For #5, for the convenience of readers, the reflog entry it leaves looks
> > like this:
> >
> > ```
> > 00873f2 feat-e@{0}: branch: Reset to HEAD
> > ```
> >
> > Not great.
> >
> > I haven't made any changes to this yet, but I've thought about what I want.
> > My
> > favorite so far is to add a new todo command that just does everything
> > right. It
> > would make a temparary ref `refs/rewritten-heads/xxx` (or something), and
> > update
> > `refs/heads/xxx` at the end.
>
> I think that's the best way to do it. If we had a command like 'branch
> <branch-name>' that creates a ref to remember the current HEAD and saves the
> current branch head. Then at the end rebase can update the branches to point
> to the saved commits if the branch is unchanged. If the rebase is aborted then
> we don't end up with some branches updated and others not.

I'd avoid cluttering the space with more commands. For `branch`, for
example, the natural short command would be `b`, but that already means
`break`.

In contrast, I would think that

	label --update-branch my-side-track

would make for a nicer read than

	label my-side-track
	branch my-side-track

Of course, it would be a lot harder to bring back the safety of `git
update-ref`'s `<old-revision>` safe-guard, in either forms.

And of course, the first form would _require_ the logic to be moved to
`make_script_with_merges()` because we could not otherwise guarantee
that the labels corresponding to local branch names aren't already in
use, for different commits.

> Side Note
>   I'd avoid creating another worktree local ref refs/rewritten-heads/.
>   Either store them under refs/rewritten/ or refs/worktree/

Yep.

Ciao,
Dscho