Re: [PATCH] merge: --ff-one-only to apply FF if commit is one

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 1, 2023, 23:05 UTC
Message-ID: <xmqqv8aloz0q.fsf@gitster.g>
In-Reply-To: <a457e24c-3375-49c6-8cf7-d2dd945827fe@app.fastmail.com>

"Kristoffer Haugsbakk" <code@khaugsbakk.name> writes:

Show 13 quoted lines

> On Wed, Nov 1, 2023, at 02:42, Junio C Hamano wrote:
>> 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).
>
> Yes. If you have multiple release/maintenance branches which you need to
> apply something to then you can’t use this .

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.