RFC patch, 2 partspull: pass the --no-ff-only flag through to merge, not fetch

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>
---
 git-pull.sh |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

Now that --no-ff-only will work with pull and not just with merge, we should probably tell the users about it.

Signed-off-by: Samuel Bronson <naesten@gmail.com>
---
 Documentation/merge-options.txt |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

Samuel Bronson <naesten@gmail.com> writes:

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.

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.

I dunno.

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

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

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?

Samuel Bronson <naesten@gmail.com> writes:

The 'true' and 'false' values to merge.ff are carry-over from the days when it was a boolean, _not_ a tristate. If we were to make the UI more rational by making it clear that this is not a boolean, it is a good time for us to aim a bit higher than merely repeating the mistakes we made in the past due to historical accident. In other words, we could add a synonym for the "default" mode in addition to "--ff=true" (and for the "always merge" mode in addition to "--ff=false") that makes it clear that the value is _not_ a boolean [*1*]. If we were to go the "--ff=<value>" route, we have to add support for other ways to spell boolean 'true' (e.g. 'yes', '1', and 'on') anyway, so it is not that much extra work to do so, I would think.

Yeah, right now if you say "merge --ff-only --no-ff", we say these are mutually exclusive (which is true), but if you think about the tristate nature of the 'ff' option and spell it differently in your head, i.e. "merge --ff=only --ff=never", it is reasonable to argue that we should apply the usual "last one overrides" rule and behave as if "merge --no-ff" were given (for the purpose of "last one overrides", the configured defaults can be treated as if they come very early on the command line). After all "merge --no-ff --ff" does seem to use the "last one overrides" rule.

[Footnote]

*1* Perhaps 'allowed' instead of 'normal' (which I wrote out of thin-air; I do not have any strong preference on the actual values) may be a better choice for such a "this is not a boolean" spelling for the default mode.

On Thu, Dec 1, 2011 at 1:06 PM, Junio C Hamano <gitster@pobox.com> wrote:

Sure, that makes sense. I was just a little worried that you might be (accidentally) proposing that --ff use a different set of names than merge.ff for a moment there...

Yes, I totally agree that it would make more sense that way; I certainly tried that before I even began to look at any of the code.