merge: --ff-one-only to apply FF if commit is one

From: Ruslan Yakauleu <[email protected]>

A new option --ff-one-only to control the merging strategy. For one commit option works like -ff to avoid extra merge commit. In other cases the option works like --no-ff to create merge commit for complex features.

---
merge: --ff-one-only to apply FF if commit is one

A new option --ff-one-only to control the merging strategy. For one commit option works like -ff to avoid extra merge commit. In other cases the option works like --no-ff to create merge commit for complex features.

Plenty of developers want to simplify merge history. We have two main merging strategies:

* Fast-forward (--ff) - There we can lose merge commits for complex features and if we need to roll back some feature we can't revert just one commit.
* Merge (--no-ff) - There we have extra merges for extra simple changes.

Before, the user had to choose between --ff/--no-ff every time to have history without extra merges for simple changes and to use merges for complex features.

Ways to use the new option: git merge --ff-one-only git config merge.ff one-only git config branch.master.mergeoptions --ff-one-only

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1599%2FQuAzI%2Fmerge%2Fff-one-only-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1599/QuAzI/merge/ff-one-only-v1
Pull-Request: https://github.com/git/git/pull/1599

 Documentation/config/merge.txt |  3 +++
 builtin/merge.c                | 18 +++++++++++++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

index 8851b6cedef..6cd5daa4d64 100644
@@ -31,6 +31,9 @@ merge.ff::
 	a case (equivalent to giving the `--no-ff` option from the command
 	line). When set to `only`, only such fast-forward merges are
 	allowed (equivalent to giving the `--ff-only` option from the
+	command line). When set to `one-only`, fast-forward merge allowed
+	only for one commit, in other way extra merge commit should be
+	created (equivalent to giving the `--ff-one-only` option from the
 	command line).
 
 merge.verifySignatures::

index d748d46e135..100f0021e56 100644
@@ -110,7 +110,8 @@ static const char *pull_twohead, *pull_octopus;
 enum ff_type {
 	FF_NO,
 	FF_ALLOW,
-	FF_ONLY
+	FF_ONLY,
+	FF_ONE_ONLY
 };
 
 static enum ff_type fast_forward = FF_ALLOW;

@@ -258,6 +259,7 @@ static struct option builtin_merge_options[] = {
 		N_("edit message before committing")),
 	OPT_CLEANUP(&cleanup_arg),
 	OPT_SET_INT(0, "ff", &fast_forward, N_("allow fast-forward (default)"), FF_ALLOW),
+	OPT_SET_INT(0, "ff-one-only", &fast_forward, N_("allow fast-forward if only one commit"), FF_ONE_ONLY),
 	OPT_SET_INT_F(0, "ff-only", &fast_forward,
 		      N_("abort if fast-forward is not possible"),
 		      FF_ONLY, PARSE_OPT_NONEG),

@@ -631,6 +633,8 @@ static int git_merge_config(const char *k, const char *v,
 			fast_forward = boolval ? FF_ALLOW : FF_NO;
 		} else if (v && !strcmp(v, "only")) {
 			fast_forward = FF_ONLY;
+		} else if (v && !strcmp(v, "one-only")) {
+			fast_forward = FF_ONE_ONLY;
 		} /* do not barf on values from future versions of git */
 		return 0;
 	} else if (!strcmp(k, "merge.defaulttoupstream")) {

@@ -1527,6 +1531,18 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		free(list);
 	}
 
+	if (fast_forward == FF_ONE_ONLY) {
+		fast_forward = FF_NO;
+
+		/* check that we have one and only one commit to merge */
+		if (squash || ((!remoteheads->next &&
+				!common->next &&
+				oideq(&common->item->object.oid, &head_commit->object.oid)) &&
+				oideq(&remoteheads->item->parents->item->object.oid, &head_commit->object.oid))) {
+			fast_forward = FF_ALLOW;
+		}
+	}
+
 	update_ref("updating ORIG_HEAD", "ORIG_HEAD",
 		   &head_commit->object.oid, NULL, 0, UPDATE_REFS_DIE_ON_ERR);
 

base-commit: 2e8e77cbac8ac17f94eee2087187fa1718e38b14

gitgitgadget

Hi

I do something similar for my pull requests:

- If more than one commit: --no-ff
- One commit: rebase/squash

Your change would (according to the commit message) only *not* create a merge if it so happens to be the case that you can fast-forward the branch. So it would create a merge commit if (say) your branch was three commits behind and one ahead (your commit) of the target branch. But isn’t it fine to just rebase/squash in this case? That seems more general.

(Maybe --squash-one-only.)

But it seems that your new option would work nicely with “semi-linear” merges:[1]

- Rebase on the target branch
- --ff-one-only
- Now you either get:
- a “semi-linear merge”; or
- a single commit (as opposed to a merge which joins together a branch which is 0 behind and 1 ahead of the target branch)

Kristoffer

Hi

If we use squash we will lose some context and occasionally, we need multiple small features combined into one big release. We would rather not mix it into one monolithic non-readable blob. For us, sometimes it is better to rebase something to make history more accurate than squash everything into one commit. We can use squash only for one story.

Anyway, squash is a different feature.
Same as rebase (of course we're doing the rebase before merge to clarify history and to make some regression tests)

Then we have a set of branches. Some branches contain only one commit.

For example, we have 4 branches:
- two commits
- one commit
- one commit
- three commits

With --no-ff (only merges) we have next graph with extra merges (E', F') for branches with one commit

  B---C   E   F   G---H---I
 /     \ / \ / \ /         \
A-------D---E'--F'----------J

With --ff (fast-forward everything) we miss merge branches (D, J) and it's harder to fast revert some problematic releases properly, because it's not clear that commits G-H-I - is one release

A---B---C---E---F---G---H---I

Story which --ff-one-only should build without manual control

  B---C           G---H---I
 /     \         /         \
A-------D---E---F-----------J

There we have merge commits (D, J) only for complex branches. Branches E and F fast-forwarded to prevent extra merges.

Sorry if my explanation isn't clear enough

--
Ruslan

The only context you lose is the point where your branch started, if that matters. (Later in the email you say that you rebase before you merge so apparently it does not matter to you.)

To squash one single commit doesn’t make a “monolithic” commit—it is exactly as monolithic as the commit you started with.

No, squash one single commit. Not multiple.

Squash and rebase are functionally identical in this case. And since you rebase before merge anyway it is identical to the flow of

1. Rebase before merge
2. Merge with your new option

Except you don’t have to split it into two steps.

Sorry, but for me git merge --squash doesn't work.

Currently, I have a global option --no-ff for master

$ git config branch.master.mergeoptions --no-ff
In this way `git merge --squash` crashes with message

In other way it writes something like

and... not updates parent branch.

For example, GitHub propose for PR's:
- Create a merge commit - the same as git merge --no-ff
- Rebase and merge - the same as git rebase ...; git merge --ff-only
- Squash AND commit - like two different operations. So after squash we still have to merge our commit properly into parent branch.

The new option just dynamically selects between --ff and --no-ff for git merge. Nothing else.

Ruslan

Hi Ruslan,

This seems like a pretty niche feature to want to introduce a new option for. I would imagine the alternative is something like:

    ff="--no-ff"
    if test 1 -eq $(git rev-list @{u}..)
    then
        ff="--ff"
    fi

    [on upstream @{u}]
    git merge "$ff" "$branch"

I don't have a great sense of how many users might want or benefit from something like this. My sense is that there aren't many, but I could very easily be wrong here.

In any case, my sense is that this is probably too niche to introduce a new command-line option just to implement this behavior when the above implementation is pretty straightforward. Regardless, here's my review of the patch...

Another more fundamental objection is "Why do we special case only a singleton commit?"

Why isn't a trivial two-patch series also OK to fast-forward? Three? There is no inherent reason to draw a line on one commit topic---given that a single commit could be a large and involved one that could have been a multi commmit series.

And you cannot decide if the "topic" is large enough to deserve a binding merge commit even if it is a single commit topic, or if is small enough and you want to allow fast-forward, without looking at it first. So from that point of view, too, I do not think this new option is a good idea. Let's not add an option to encourage a bad discipline.

Thanks.

Hi Junio,

Command of developers can decide which policy use and why. If a team decides to avoid extra merges - why not?

Create one non-reviewable commit or losing feature scope is much worse practice

For me git log --graph is the most representative form to describe what's going on with a project and I prefer not to complicate it. But it must show changes properly.

--ff-one-only is handy for keeping the appearance of a linear commit history and makes merge commit messages (from other branches) more meaningful because they actually do represent some branch being folded into the master.

As an example, merge commit for one commit is overkill and doesn't bring new meaning. All one-commit meanings described in the commit message. In this case extra branching (extra merges) just adds confusion. Plenty of extra merges turns into merge hell when you have countless merge messages, but you can't get sense what's going on with project.

People use rebase and fast-forward to make commit history more linear, avoid merge hell and keep the repository tree clear - it's fine. But it's a big mistake to merge complex features as one non-reviewable commit. And even worse to make all history absolutely linear when you can't find where a complex feature starts and where it finishes. If it wasn't combined in a special branch, it looks in the main branch like a set of different, not connected features.
In the case of problems with absolutely linear history, you don't know how many commits you have to revert.
In case when one commit (merge commit for complex features) brings all changes into the main branch - you know that you can revert feature or, for complex changes, even part of a specific feature. You can revert just A/B tests for example, if you don't need them anymore but keep complex features - this case makes squash even more useless.
You can try to use branch names to find where each branch merged into the main branch, but it's not as descriptive as a graph and every so often you can lose branch names. As an example, GitLab has an option "Delete source branch when merge request accepted" and many teams use it.

--
Ruslan

I never said "--ff" is a bad option that should not exist, and I am not sure why I had to be thrown such a rhetorical question at.

I think it’s about the --first-parent view. Then you get a one-commit view of each pull request that was merged. For a merge commit it serves as a summary of multiple commits. And a merge commit of one commit would serve as a summary of one commit. So in that case you remove that extra level of indirection.

But the pull request is already given: it either has one commit or several. And you can for sure look at it and either argue that it should be reduced (squashed) to one commit or maybe expanded (split out) into several commits.

The point at which you use such a merge feature is when you are already happy with the pull request (or patch series or whatever else). And then you (according to this strategy) prefer to avoid merge commits for single-commit pull requests.

— —

This is not an argument for making this a built-in strategy. Just an explanation from my vantage point.

Kristoffer Haugsbakk

Yeah, I understand that position very well. After all, I was heavily involved in the introduction of the first-parent view to the system at around 0053e902 (git-log --first-parent: show only the first parent log, 2007-03-13). Soon after that, d66424c4 (git-merge: add --ff and --no-ff options, 2007-09-24) added --no-ff to make it easier to maintain the first-parent worldview.

Strictly speaking, the log message on a merge commit serves two purposes, one is to summarize commit(s) on the side branch that gets merged with the merge, and as you said above, it is not needed when merging a topic with just one commit. But the other is to justify why the topic suits the objective of the line of history (which is needed even when merging a single commit topic---imagine a commit that is not incorrect per-se. It may or may not be suitable for the maintenance track, and a merge commit of such a commit into the track can explain if/how the commit being merged is maint-worthy).

A project that does not need the latter can do without a "--no-ff" merge of a single commit topic.

But that argues against the "--ff-one-only" option, doesn't it?

If you looked at the side branch before you decide to merge it, you know if the topic has only one commit (in which case you decide not to use "--no-ff"), or if the topic consists of multiple commits (in which case you decide to use "--no-ff"). And the only effect to have the "--ff-one-only" option is to allow you *not* to look at what is on the side branch.

Thanks.

Moreover, it allows not to forget choose the right way and allow use a lot of external tools (GitExtensions for example) as not all in command are console ninja

Currently, when complex features merged with --ff accidentally we have to do in main branch something like

$ git reset ...
$ git checkout <some_old_commit_which_we_have_to_find>
$ git merge --no-ff <latest commit>
$ git push --force

And that's what I prefer to avoid

--
Ruslan

Yes. If you have multiple release/maintenance branches which you need to apply something to then you can’t use this .

No. The only effect is that you streamline the process of “decide not to use --no-ff” since the strategy does it for you.

It would act like a small git my-merge alias. That is all.

Kristoffer Haugsbakk

OK. I do not mind a feature to help maintain the first-parent worldview better to exist, but have a few comments on the patch.

* Nowhere in the name of feature --ff-one-only, the proposed commit log message, added documentation and in-code comments, it is made clear to readers that it is to maintain the first-parent view better. The "first-parent" was only brought up between you and I as our conjecture on what the feature is for. The should explain the feature a bit better to our readers and users.

SIDE NOTE: in general, it is not the best way to name and explain a feature after what it does (e.g., "fast-forward only when it has one commit"); it is better to include why the user want it to do what it does. It it especially true because "fast-forward only when the other branch is ahead by one commit" may later turn out not to be the best design to ensure "maintain first-parent worldview", if the latter is what the feature is really about.

* The proposed commit log message needs a bit of proofreading and polishing, paying attention to the grammar.

* The "allow fast-forward only when the other branch is ahead by one commit" design misses an important case you would want to, and you can detect easily, fast-forward.

Imagine that a developer has a rather complex topic with multiple commits, asks the maintainer (or the auto-merger at their forge) to pull, but due to modification on the upstream side, there are heavy conflicts. The maintainer can tell (and Git was designed to support this mode of operation better---it is called "distributed development") the developer:

Since you know your topic much better than I do, can you do the merge into the upstream for me?

The contributor would then help the maintainer, perhaps like so:

    $ git checkout origin/main
    $ git merge [--no-ff] my-topic

to pretend as if the contributer were the maintainer, merge and resolve the conflicts, and then summarizes the topic in the log message of the merge commit. The contributor then updates their topic locally, perhaps with

    $ git push . HEAD:my-topic

which would of course fast-forward, and then ask the maintainer (or the auto-merger at their forge) to pull again from "my-topic".

Now, the updated "my-topic" is ahead of the origin by many commits (i.e., the number of commits on the topic, plus the merge commit the controbutor created to help the maintainer), but if we want to see the resulting history as if the original pull request was handled with the "--ff-one-only" option by the maintainer who did the merge themself, then we should fast-forward this merge. Even though the tip commit of "my-topic" has more commits behind it, it is already the binding merge of the side topic that "--ff-one-only" would have forced to create if the maintainer did the merge.

So, a better design than "allow fast-forward, only if the branch being merged is ahead by one commit" is to allow fast-forward when the branch's first-parent is the current tip of the branch pull/merge is trying to update. "only by one commit" can be handled as a natural degenerate case of this more general criteria, and a good thing is that it is much easier and more efficient to compute (i.e., in "git merge OTHER", allow ff if "OTHER^1" and "HEAD" are the same).

As I said, I do not mind a feature to help maintain the first-parent worldview better to exist; thanks for working on the topic.

Hi Taylor,

Thank you so much for the review and feedback.

We use a mixed environment (Linux, MacOS and Windows) on multiple projects. And some members can work some days from home and some days from the office (on different PC).
So, using of scripts a bit uncomfortable.

Better to apply once for everybody in the domain
git config --global merge.ff one-only

I'll try to fill the proper test a bit later.

--
Ruslan