Re: [PATCH] merge: allow using --no-ff and --ff-only at the same time

[Michael Haggerty <mhagger@alum.mit.edu>]

On 07/01/2013 09:01 AM, Miklos Vajna wrote:
> 1347483 (Teach 'git merge' and 'git pull' the option --ff-only,
> 2009-10-29) says this is not allowed, as they contradict each other.
> 
> However, --ff-only is about asserting the input of the merge, and
> --no-ff is about instructing merge to always create a merge commit, i.e.
> it makes sense to use these options together in some workflow, e.g. when
> branches are integrated by rebasing then merging, and the maintainer
> wants to be sure the branch is rebased.

That is one interpretation of what these options should mean, and I
agree that it is one way of reading the manpage (which says

--ff-only::
	Refuse to merge and exit with a non-zero status unless the
	current `HEAD` is already up-to-date or the merge can be
	resolved as a fast-forward.

).  However, I don't think that the manpage unambiguously demands this
interpretation, and that (more importantly) most users would be very
surprised if --ff-only and --no-ff were not opposites.

How does it hurt?  If I have configuration value merge.ff set to "only"
and run "git merge --no-ff" and then I merge a branch that *cannot* be
fast forwarded, the logic of your patch would require the merge to be
rejected, no?  But I think it is more plausible to expect that the
command line option takes precedence.

Hmmph.  I just tested and found out that (before your patch) a "--no-ff"
command-line option does *not* override a "git config merge.ff only" but
rather that the combination provokes the error that you are trying to
remove,

    fatal: You cannot combine --no-ff with --ff-only.

I find *that* surprising; usually command-line options override
configuration file settings.  OK, it's time for some more exhaustive
testing:

   situation      merge.ff  option      result
   -------------------------------------------------------------------
1  ff possible    false     --ff        works (ff)
2  ff impossible  false     --ff        works (non-ff)
3  ff possible    false     --ff-only   error "cannot combine options"
4  ff impossible  false     --ff-only   error "cannot combine options"
5  ff possible    false     --no-ff     works (non-ff)
6  ff impossible  false     --no-ff     works (non-ff)
7  ff possible    only      --ff        works (ff)
8  ff impossible  only      --ff        error "not possible to ff"
9  ff possible    only      --ff-only   works (ff)
10 ff impossible  only      --ff-only   error "not possible to ff"
11 ff possible    only      --no-ff     error "cannot combine options"
12 ff impossible  only      --no-ff     error "cannot combine options"

>From line 1 we see that "--ff" overrides "merge.ff=false".

>From lines 3 and 4 we see that "--ff-only" cannot be combined with
"merge.ff=false".

>From line 8 we see that "merge.ff=only" has its effect despite "--ff",
which would normally allow a non-ff merge.

>From lines 11 and 12 we see that "--no-ff" cannot be combined with
"merge.ff=only".

I find this inconsistent.  I think it would be more consistent to have
exactly three states,

* merge.ff unset == --ff == "do ff if possible, otherwise non-ff"

* merge.ff=false == --no-ff == "always create merge commit"

* merge.ff=only == --ff-only == "do ff if possible, otherwise fail"

and for the command-line option to always take precedence over the
config file option.

Moreover, isn't it the usual practice for later command-line options to
take precedence over earlier ones?  It is the case for at least one command:

    $ git log --oneline --no-walk --no-decorate --decorate
    cf71d9b (HEAD, master) 2
    $ git log --oneline --no-walk --decorate --no-decorate
    cf71d9b 2

So I think that command invocations with more than one of {"--ff",
"--no-ff", "--ff-only"} should respect the last option listed rather
than complaining about "cannot combine options".

If I find the time (unlikely) I might submit a patch to implement these
expectations.


In my opinion, your use case shouldn't be supported by the command
because (1) it is confusing, (2) it is not very common, and (3) it is
easy to work around:

    if git merge-base --is-ancestor HEAD $branch
    then
        git merge --no-ff $branch
    else
        echo "fatal: Not possible to fast-forward, aborting."
        exit 1
    fi

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/