"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
James Cook <falsifian@falsifian.org>
Subject:
Re: fast-forwarding
To:
gameoftrees@openbsd.org
Date:
Fri, 12 May 2023 03:06:25 +0000

Download raw body.

Thread
> > - Make "got merge" fast-forward when possible. (Possible conflict: some
> >   users may want the ability to explicitly create a merge commit even
> >   when they could have fast-forwarded. E.g. I've seen this recommended
> >   when merging pull requests: then the commit graph clearly
> >   distinguishes between the proposed change and the act of merging it
> >   into the main branch; this would be lost if main were simply fast
> >   forwarded to the pull request's commit.
> 
> Is there ever any non-cosmetic reason to pefer merge commits over
> fast-forwarding?

I don't know what cosmetic would mean. Searching Google for "no-ff" (as
in "git merge --no-ff" brings up
	https://nvie.com/posts/a-successful-git-branching-model/
which I haven't read carefull but seems to indicate some people like
these merges (search for "--no-ff").

This is speculation, but I wonder if they're useful for signing commit
messages, if you want to incorporate an untrusted contributor's commit
and vouch for it by signing the merge commit.

Anyway, it seems to me support for these could be added later, if
wanted, with an equivalent to git-merge's --no-ff

> I would prefer this being added to 'got merge' rather than adding a
> new command. 'got rebase' already fast-forwards when possible. The fact
> that 'got merge' does not this yet is a mistake. I simply forgot to
> consider this case when 'got merge' was added as a new command to
> provide the ability to create merge commits. At the time I didn't
> realize that fast-forwarding with 'got merge' would be useful after
> fetching branches from remote repositories. And by the way, I also
> forgot about this when 'got rebase' was initially added. Fast-forwarding
> was added later when someone (kn@ I think?) pointed out to me that it
> was missing.
> 
> 'got merge' could offer a command line option and/or environment variable
> and/or config knob to prevent fast-forwarding for situations where users
> want to avoid it. But it should fast-forward by default whenever possible,
> at least until we figure out a good reason for why it should not do this.
> 
> Thanks to histedit users can always go back and redo the merge if they
> don't like the default merge result, and if they care deeply enough they
> will remember to pass the no-fast-forward flag next time (or just use Git).

That seems good to me.

I don't actually have a lot of experience collaborating with others
using git. But having "got merge" fast-forward when possible would be
consistent with "git merge", at least.

> Turns out there is an item in the TODO file which lists details from
> prior discussions we've had. This shows why some amount of heuristics
> will be needed:
> 
> - Teach 'got merge' to forward a branch reference if possible, instead of
>   creating a merge commit. Forwarding should only be done if linear
>   history exists from the tip of the branch being merged to the tip of
>   the work tree's branch, and if the tip of the work tree's branch is
>   itself not a merge commit (this makes "stacked" merges possible
>   by default, and prevents a 'main' branch reference from being forwarded
>   to a vendor branch in case no new commits were added to 'main' since
>   the previous vendor merge). Provide an option (-M) which forces creation
>   of a merge commit, for cases where users deem forwarding undesirable.

I don't see why there need to be any requirements other than: the (tip of
the) work tree's branch is an ancestor of the (tip of the) branch being
merged.

I remember in IRC I raised the vendor branch scenario, but I think I was
confused. Consider this example scenario (reverse chronological order;
main branch on the left, vendor on the right):

main    vendor
       o
     o |
     |\|
     | o
     | |
     o |
     | |
     o |
     |\|
     | o
     |
     o

Here, no new commits were added to "main" since the previous vendor
merge. But the tip of "main" is not an ancestor to the tip of "vendor"
so there's nothing to worry about: the simpler heuristic (just the
ancestor check) will do the right thing.

Conversely, we may want to fast-forward when the tip of the work tree
branch is a merge. E.g. perhaps yesterday I merged "vendor" into "main",
then today someone else pushed some changes on top of that. When I pull
today's changes, there's no reason not to just fast-forward; the fact
that yesterday ended with a merge commit is just a coincidence.

But maybe I'm missing something?

If I've got it right, then I guess the first step is to make "got merge"
fast-forward when possible; -M could optionally be added after that.

-- 
James