From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: phillip.wood@dunelm.org.uk
Cc: Phillip Wood via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org,
Philippe Blain <levraiphilippeblain@gmail.com>,
Denton Liu <liu.denton@gmail.com>,
Johannes Schindelin <Johannes.Schindelin@gmx.de>,
Phillip Wood <phillip.wood123@gmail.com>,
Elijah Newren <newren@gmail.com>,
Junio C Hamano <gitster@pobox.com>,
Jonathan Tan <jonathantanmy@google.com>
Subject: Re: [PATCH v3 5/8] rebase: rename merge_base to branch_base
Date: Mon, 17 Oct 2022 13:27:25 +0200 [thread overview]
Message-ID: <221017.86pmeqk6yl.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <7b9d2a05-de2e-d0e0-6554-a592fa2349d4@dunelm.org.uk>
On Mon, Oct 17 2022, Phillip Wood wrote:
> On 13/10/2022 20:16, Ævar Arnfjörð Bjarmason wrote:
>> On Thu, Oct 13 2022, Phillip Wood via GitGitGadget wrote:
>>
>>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>>
>>> merge_base is not a very descriptive name, the variable always holds
>>> the merge-base of 'branch' and 'onto' which is commit at the base of
>>> the branch being rebased so rename it to branch_base.
>> To me "branch" means or has heavier implications of "named branch"
>> than
>> just a merge base, and this command is perfectly happy to work on
>> commits disconnected from any named branch.
>> > But more to the point, the rebase docs for --onto discuss a "merge
>> base", so you'd read those, and then encounter this code talking about a
>> "branch base", and wonder what the difference was...
>
> Aren't the docs saying the merge base is the base of the commits
> (i.e. branch) being rebased? I don't think merge_base is a
> particularly helpful name as it doesn't tell us what it is the merge
> base of and branch_base was the best I could come up with. I see what
> you mean in the detached HEAD case, but as the command also works with
> named branches I hope it is fairly obvious what "branch_base" is in
> the detached HEAD case.
It *optionally* works with a <branch>, but doesn't require one. E.g. try
this on git.git:
git checkout origin/next
touch f && git add f && git commit -m"file"
git rebase --onto origin/master^{} HEAD~
Here we transplant a commit on top of "next" to "master", without either
of those *names* being involved, or their branches, just the
corresponding OIDs/tips.
That will go through e.g. can_fast_forward() which you're modifying
here, and now populate a "branch_base" variable, instead of a
"merge_base".
I know that we conflate the meaning of "branch" somewhat, even in our
own docs. E.g. we sometimes use "branch" and "named branch", but usually
by "branch" we mean "named branch", and otherwise talk about a detached
HEAD, <commit> or "tip".
But in this case it's especially confusing in the post-image, because
"git rebase --onto" explicitly uses an optional "<branch>" to
distinguish the "named branch" case from the case where we're operating
on detached a HEAD, or otherwise don't care about the "<branch>" (except
as generic "restore us to where we were" behavior).
So, if anything I'd think that we'd want something like this in various
places in git-rebase.txt to make the distinction clearer:
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 9cb8931c7ac..e4700a6e777 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -18,7 +18,7 @@ DESCRIPTION
-----------
If `<branch>` is specified, `git rebase` will perform an automatic
`git switch <branch>` before doing anything else. Otherwise
-it remains on the current branch.
+it remains on the current tip or named branch.
If `<upstream>` is not specified, the upstream configured in
`branch.<name>.remote` and `branch.<name>.merge` options will be used (see
But your post-image seems to be to make this sort of thing explicitly
more confusing, and e.g. these parts:
@@ -206,8 +206,8 @@ OPTIONS
--onto <newbase>::
Starting point at which to create the new commits. If the
`--onto` option is not specified, the starting point is
- `<upstream>`. May be any valid commit, and not just an
- existing branch name.
+ `<upstream>`. May be any valid commit, and not just an <-- this
+ existing branch name. <--- this
+
As a special case, you may use "A\...B" as a shortcut for the
merge base of A and B if there is exactly one merge base. You can
To sum up why I find this confusing: Reading this from the docs onwards
I'd think (as is the case) that "<branch>" is optional. Then when I read
the code I'd think a "branch_base" is something that *only* had to do
with the "<branch>" case.
But that's not the case, it's just a generic "merge base" in the same
sense that "git merge-base" accepts all of these
$ git merge-base origin/master origin/next
d420dda0576340909c3faff364cfbd1485f70376
(These two are equivalent, just demo'ing that we don't need the peel
syntax):
$ git merge-base $(git rev-parse origin/master) $(git rev-parse origin/next)
d420dda0576340909c3faff364cfbd1485f70376
$ git merge-base origin/master^{} origin/next^{}
d420dda0576340909c3faff364cfbd1485f70376
What *would* make things much clearer is e.g. calling a variable
"branch_merge_base" *if* there is a case where that's a merge base only
for named branches, but I don't know (and didn't look carefully enough)
if you've got such a case or cases here. It just seems like a generic
"merge-base".
next prev parent reply other threads:[~2022-10-17 11:47 UTC|newest]
Thread overview: 82+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-15 15:11 [PATCH 0/5] rebase --keep-base: imply --reapply-cherry-picks and --no-fork-point Phillip Wood via GitGitGadget
2022-08-15 15:11 ` [PATCH 1/5] t3416: set $EDITOR in subshell Phillip Wood via GitGitGadget
2022-08-15 16:53 ` Junio C Hamano
2022-08-16 13:53 ` Phillip Wood
2022-08-24 22:28 ` Jonathan Tan
2022-08-30 15:12 ` Phillip Wood
2022-08-15 15:11 ` [PATCH 2/5] rebase: store orig_head as a commit Phillip Wood via GitGitGadget
2022-08-15 16:58 ` Junio C Hamano
2022-08-16 9:11 ` Johannes Schindelin
2022-08-18 7:01 ` Elijah Newren
2022-08-15 15:11 ` [PATCH 3/5] rebase: factor out merge_base calculation Phillip Wood via GitGitGadget
2022-08-15 17:22 ` Junio C Hamano
2022-08-16 9:15 ` Johannes Schindelin
2022-08-16 15:00 ` Junio C Hamano
2022-08-16 13:50 ` Phillip Wood
2022-08-16 15:03 ` Junio C Hamano
2022-08-18 7:11 ` Elijah Newren
2022-08-24 22:02 ` Jonathan Tan
2022-08-30 15:03 ` Phillip Wood
2022-08-15 15:11 ` [PATCH 4/5] rebase --keep-base: imply --reapply-cherry-picks Phillip Wood via GitGitGadget
2022-08-15 20:58 ` Junio C Hamano
2022-08-24 22:09 ` Jonathan Tan
2022-08-30 15:09 ` Phillip Wood
2022-08-25 0:29 ` Philippe Blain
2022-09-05 13:54 ` Phillip Wood
2022-08-15 15:11 ` [PATCH 5/5] rebase --keep-base: imply --no-fork-point Phillip Wood via GitGitGadget
2022-08-15 21:07 ` Junio C Hamano
2022-08-24 22:18 ` Jonathan Tan
2022-09-05 13:51 ` Phillip Wood
2022-08-16 9:23 ` [PATCH 0/5] rebase --keep-base: imply --reapply-cherry-picks and --no-fork-point Johannes Schindelin
2022-08-24 22:27 ` Jonathan Tan
2022-09-07 14:37 ` [PATCH v2 0/7] " Phillip Wood via GitGitGadget
2022-09-07 14:37 ` [PATCH v2 1/7] t3416: tighten two tests Phillip Wood via GitGitGadget
2022-09-07 18:12 ` Junio C Hamano
2022-09-07 14:37 ` [PATCH v2 2/7] t3416: set $EDITOR in subshell Phillip Wood via GitGitGadget
2022-09-07 18:12 ` Junio C Hamano
2022-09-07 14:37 ` [PATCH v2 3/7] rebase: store orig_head as a commit Phillip Wood via GitGitGadget
2022-09-07 18:12 ` Junio C Hamano
2022-09-08 13:19 ` Phillip Wood
2022-09-07 14:37 ` [PATCH v2 4/7] rebase: rename merge_base to branch_base Phillip Wood via GitGitGadget
2022-09-07 18:12 ` Junio C Hamano
2022-09-07 14:37 ` [PATCH v2 5/7] rebase: factor out branch_base calculation Phillip Wood via GitGitGadget
2022-09-07 18:12 ` Junio C Hamano
2022-09-07 14:37 ` [PATCH v2 6/7] rebase --keep-base: imply --reapply-cherry-picks Phillip Wood via GitGitGadget
2022-09-07 14:37 ` [PATCH v2 7/7] rebase --keep-base: imply --no-fork-point Phillip Wood via GitGitGadget
2022-09-08 2:44 ` Denton Liu
2022-09-08 13:21 ` Phillip Wood
2022-10-13 8:42 ` [PATCH v3 0/8] rebase --keep-base: imply --reapply-cherry-picks and --no-fork-point Phillip Wood via GitGitGadget
2022-10-13 8:42 ` [PATCH v3 1/8] t3416: tighten two tests Phillip Wood via GitGitGadget
2022-10-13 8:42 ` [PATCH v3 2/8] t3416: set $EDITOR in subshell Phillip Wood via GitGitGadget
2022-10-13 8:42 ` [PATCH v3 3/8] rebase: be stricter when reading state files containing oids Phillip Wood via GitGitGadget
2022-10-13 16:34 ` Junio C Hamano
2022-10-13 19:10 ` Ævar Arnfjörð Bjarmason
2022-10-13 20:13 ` Junio C Hamano
2022-10-13 8:42 ` [PATCH v3 4/8] rebase: store orig_head as a commit Phillip Wood via GitGitGadget
2022-10-13 16:34 ` Junio C Hamano
2022-10-13 20:49 ` Phillip Wood
2022-10-13 23:25 ` Junio C Hamano
2022-10-13 8:42 ` [PATCH v3 5/8] rebase: rename merge_base to branch_base Phillip Wood via GitGitGadget
2022-10-13 19:16 ` Ævar Arnfjörð Bjarmason
2022-10-17 9:49 ` Phillip Wood
2022-10-17 11:27 ` Ævar Arnfjörð Bjarmason [this message]
2022-10-17 13:13 ` Phillip Wood
2022-10-17 16:19 ` Ævar Arnfjörð Bjarmason
2022-10-19 15:35 ` Phillip Wood
2022-10-13 8:42 ` [PATCH v3 6/8] rebase: factor out branch_base calculation Phillip Wood via GitGitGadget
2022-10-13 19:21 ` Ævar Arnfjörð Bjarmason
2022-10-17 9:39 ` Phillip Wood
2022-10-17 11:23 ` Ævar Arnfjörð Bjarmason
2022-10-13 8:42 ` [PATCH v3 7/8] rebase --keep-base: imply --reapply-cherry-picks Phillip Wood via GitGitGadget
2022-10-13 8:42 ` [PATCH v3 8/8] rebase --keep-base: imply --no-fork-point Phillip Wood via GitGitGadget
2022-10-17 13:17 ` [PATCH v4 0/8] rebase --keep-base: imply --reapply-cherry-picks and --no-fork-point Phillip Wood via GitGitGadget
2022-10-17 13:17 ` [PATCH v4 1/8] t3416: tighten two tests Phillip Wood via GitGitGadget
2022-10-17 13:17 ` [PATCH v4 2/8] t3416: set $EDITOR in subshell Phillip Wood via GitGitGadget
2022-10-17 13:17 ` [PATCH v4 3/8] rebase: be stricter when reading state files containing oids Phillip Wood via GitGitGadget
2022-10-17 18:51 ` Junio C Hamano
2022-10-19 15:32 ` Phillip Wood
2022-10-17 13:17 ` [PATCH v4 4/8] rebase: store orig_head as a commit Phillip Wood via GitGitGadget
2022-10-17 13:17 ` [PATCH v4 5/8] rebase: rename merge_base to branch_base Phillip Wood via GitGitGadget
2022-10-17 13:17 ` [PATCH v4 6/8] rebase: factor out branch_base calculation Phillip Wood via GitGitGadget
2022-10-17 13:17 ` [PATCH v4 7/8] rebase --keep-base: imply --reapply-cherry-picks Phillip Wood via GitGitGadget
2022-10-17 13:17 ` [PATCH v4 8/8] rebase --keep-base: imply --no-fork-point Phillip Wood via GitGitGadget
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=221017.86pmeqk6yl.gmgdl@evledraar.gmail.com \
--to=avarab@gmail.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
--cc=jonathantanmy@google.com \
--cc=levraiphilippeblain@gmail.com \
--cc=liu.denton@gmail.com \
--cc=newren@gmail.com \
--cc=phillip.wood123@gmail.com \
--cc=phillip.wood@dunelm.org.uk \
/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).