git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Bryan Turner <bturner@atlassian.com>
Cc: Elijah Newren <newren@gmail.com>,
	usbuser@mailbox.org, Git Mailing List <git@vger.kernel.org>
Subject: Re: Unexpected or wrong ff, no-ff and ff-only behaviour
Date: Wed, 10 Jul 2019 09:34:39 -0700	[thread overview]
Message-ID: <xmqqtvbtu9uo.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <CAGyf7-FW-_4AbWE735-=7WjZAaTLHOT_QuWOoHKAjOzZCbWhFA@mail.gmail.com> (Bryan Turner's message of "Tue, 9 Jul 2019 13:51:39 -0700")

Bryan Turner <bturner@atlassian.com> writes:

> I think this is something I've seen come up on the list before[1]
> (Roland can correct me if I'm wrong). What I've seen asked for before
> is the ability to pass the combination "--ff-only --no-ff" and have
> that:
> * Ensure the branch to be merged is fast-forward from the current
> branch (i.e., to ensure no merge commit is actually necessary), but
> * Create a redundant merge commit anyway
>
> This retains the ancestry (as in, it shows where the branches were
> merged), but the merge is always effectively a no-op (no risk of
> unintended interactions, the sort of subtle breakages where the merge
> succeeds but the code on each "side" isn't entirely compatible,
> resulting in broken compilation and/or tests and/or runtime).

Hmph.  I know that the above was totally outside of expected use
pattern back when --ff/--no-ff and --ff-only were invented.  As I
still cannot tell if the use pattern makes sense, or even is
internally consistent, I am not ready to say if the omission was a
bad thing, though.  We did not omit it deliberately, but we might
have been lucky to omit it, which may have prevented a wrong
workflow from spreading.  If it is a wrong workflow, that is, and I
am yet not sure it is not.

The "--no-ff" part is a way to ensure that the series of commits are
still grouped together after they hit the mainline of the history,
so it expresses a wish (by the project using the use pattern) that
they care about topic branch workflow.  A group of commits belonging
to one single topic is about doing one single thing, not a random
collection.  "Make sure we always create a merge to record which
ones are from the side branch and which ones are the mainline" is a
reasonable way to ensure that.

The "the tip being merged into the mainline must always be
fast-forwardable", however, is not consistent with the topic branch
workflow, and I do not mean this in the sense that you should never
rebase just before submitting (which is a bad practice).  For an
initial merge of the topic to the mainline, the project can keep
rebasing to the then-current tip of the mainline, and as long as
they can afford the cycle to test the result, "record the range of
the topic branch by making a redundant merge" would work.  

But that is true only when you have one mainline.  You cannot reuse
the topic to maintain an older mainline (i.e. a maintenance track),
because of the "topic must contain the then-current tip of the
mainline" rule.  A topic forked from the tip of master cannot be
merged to maint without dragging all the other crap happened since
maint to master, and a topic forked from the tip of maint, which can
be merged to maint without problem, cannot be merged to master due
to this "must contain the tip of master" rule.

It gets worse when you need to make a fix-up to the topic.  Has any
of those who advocate this workflow considered a case where a topic
later needs fixups?  Do they always produce a 100% bug-free code so
that they do not need to?

If you have a topic with initially 1 commit, merged to the mainline,
with the "record a redundant merge of fast-forwardable side branch",
you would get this:


    ----O---1--- mainline
         \ /
          A      topic


where the topic only has commit A.  How would you apply a fix this
commit?  "A merged branch must have forked from the then-current tip"
would mean you would do this:

    ----O---1-----2--- mainline
         \ / \   /
          A   \ /  topic
               B   topic-fix

i.e. you cannot touch 'topic' branch, which has 'A', that is sealed
by the fact that it was already merged to the mainline and the rule
that you reject a merge of any side branch that does not fork from
the then-current tip.  You are forced to create B forked from the
mainline to fix A and merge it back to mainline, losing the valuable
"these commits, together, achieves this thing" grouping you wanted
to achieve with the topic-branch workflow, which is what "--no-ff"
is asking.  This is the reason why I doubt the workflow that wants
to say "record a merge with a side branch that contains the current
tip of the mainline" is not even internally consistent.

So, I dunno.



  parent reply	other threads:[~2019-07-10 16:34 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-09  9:42 Unexpected or wrong ff, no-ff and ff-only behaviour usbuser
2019-07-09 14:51 ` Junio C Hamano
2019-07-09 16:15   ` Roland Jäger
2019-07-09 16:35     ` Elijah Newren
2019-07-09 17:00       ` usbuser
2019-07-09 20:33         ` Elijah Newren
2019-07-09 20:51           ` Bryan Turner
2019-07-10  7:49             ` usbuser
2019-07-10 16:34             ` Junio C Hamano [this message]
2019-07-11  5:13               ` Sergey Organov
2019-07-11 17:03                 ` Junio C Hamano
2019-07-12 13:50                   ` Sergey Organov
2019-07-12 16:24                     ` Elijah Newren
2019-07-15 12:08                       ` Sergey Organov
2019-07-12 18:33                     ` Junio C Hamano
2019-07-15 12:47                       ` Sergey Organov
2019-07-15 16:57                         ` Junio C Hamano
2019-07-19 11:00                           ` Sergey Organov
2019-07-11 15:46             ` brian m. carlson
2019-07-10 14:36       ` Sergey Organov

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqtvbtu9uo.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=bturner@atlassian.com \
    --cc=git@vger.kernel.org \
    --cc=newren@gmail.com \
    --cc=usbuser@mailbox.org \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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).