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

On 07/01/2013 09:01 AM, Miklos Vajna wrote:

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"

"merge.ff=false".

which would normally allow a non-ff merge.

"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