Re: [PATCH/RFC 1/2] pull: pass the --no-ff-only flag through to merge, not fetch

From: Samuel Bronson <naesten@gmail.com>
Date: Dec 1, 2011, 17:18 UTC
Message-ID: <CAJYzjmep7sKxiSNhMzAX2DRYJhANDQkPL5pX4HOZ9CssJxcWbw@mail.gmail.com>
In-Reply-To: <7vborsq45x.fsf@alter.siamese.dyndns.org>

On Wed, Nov 30, 2011 at 11:58 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 19 quoted lines

> Samuel Bronson <naesten@gmail.com> writes:
>
>> Without this, pull becomes unusable for merging branches when the config
>> option `merge.ff` is set to `only`.
>>
>> Signed-off-by: Samuel Bronson <naesten@gmail.com>
>
> I wonder why you need this. We have "git config --unset merge.ff" after
> all. From purely mechanstic point of view, being able to temporarily
> defeat a configuration variable makes perfect sense, but from the point of
> view of workflow, I am not sure if it is a good thing to even allow it in
> the first place in this particular case.
>
> Setting merge.ff to 'only' means you are following along other people's
> work and making nothing new on your own in this particular repository, no?
> Hence you won't be asking the upstream to pull from this repository, which
> in turn means that even if you made a merge by temporarily defeating the
> configuration setting with this patch, your future pulls will no longer
> fast forward, until somehow the upstream gets hold of your merge commit.

Actually, I wanted to set merge.ff to only avoid *accidentally* merging origin/master onto my local master, when I would likely to prefer to either rebase my work onto it, or retroactively put my work in a branch and merge that *into* the real master branch; I would then override only when I explicitly did want a merge.

(I actually have commit access to the repository in question, but wish to avoid making others' commits to master look as if they were on side branches.)

Show 16 quoted lines

> By the way (this is a digression), I also have to say --no-ff-only is too
> *ugly* as a UI element, even though I know "git merge" already allows it
> by accident.
>
> "ff" is a tristate. By default, fast-forward is done when appropriate, and
> people can _refuse_ to fast-forward and force Git to create an extra merge
> commit. Or if you are strictly following along, you can say you _require_
> fast-forward and reject anything else. So it may make the UI saner if we
> updated the UI to allow users to say:
>
>        --ff=normal     the default
>        --ff=never      same as --no-ff that forces an extra merge commit
>        --ff=required   same as --ff-only
>
> while keeping the current --ff-only and --no-ff as backward compatibility
> wart.

Hmm, yes, I had noticed that it was a tristate (merge.ff clearly is), and I guess --no-ff-only is a pretty ugly flag. I do have to ask, though: why give --ff these new values? Wouldn't it make more sense to reuse the values accepted by merge.ff; namely, 'true' (the implied default), 'false', and 'only'?

Otherwise, this looks like a very nice way to implement what I want: I guess it is probably a mistake that the existing (documented) flags do not behave in this way?