[PATCH 0/4] make pull.ff=true override merge.ff

[PATCH 0/4] make pull.ff=true override merge.ff

[Paul Tan]

Since b814da8 (pull: add pull.ff configuration, 2014-01-15), running
git-pull with the configuration pull.ff=false or pull.ff=only is
equivalent to passing --no-ff and --ff-only to git-merge. However, if
pull.ff=true, no switch is passed to git-merge. This leads to the
confusing behavior where pull.ff=false or pull.ff=only is able to
override merge.ff, while pull.ff=true is unable to.

This patch series adds a failing test to demonstrates the above, and fixes it.

The documentation is also tweaked to clarify that pull.ff is meant to override
merge.ff.

The last patch makes pull.ff consistent with merge.ff by supporting the config
aliases of true and false (on, off, 0, 1).

Paul Tan (4):
  t7601: test for pull.ff=true overrides merge.ff=false
  pull: make pull.ff=true override merge.ff
  Documentation/config.txt: clarify that pull.ff overrides merge.ff
  pull: parse pull.ff as a bool or string

 Documentation/config.txt     | 2 +-
 git-pull.sh                  | 5 ++++-
 t/t7601-merge-pull-config.sh | 8 ++++++++
 3 files changed, 13 insertions(+), 2 deletions(-)

-- 
2.1.4

[PATCH 1/4] t7601: test for pull.ff=true overrides merge.ff=false

[Paul Tan]

Since b814da8 (pull: add pull.ff configuration, 2014-01-15), running
git-pull with the configuration pull.ff=false or pull.ff=only is
equivalent to passing --no-ff and --ff-only to git-merge. However, if
pull.ff=true, no switch is passed to git-merge. This leads to the
confusing behavior where pull.ff=false or pull.ff=only is able to
override merge.ff, while pull.ff=true is unable to.

Add a failing test that demonstrates this case.

Signed-off-by: Paul Tan <pyokagan@gmail.com>
---

 t/t7601-merge-pull-config.sh | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
index f768c90..cef94e6 100755
--- a/t/t7601-merge-pull-config.sh
+++ b/t/t7601-merge-pull-config.sh
@@ -45,6 +45,14 @@ test_expect_success 'fast-forward pull succeeds with "true" in pull.ff' '
 	test "$(git rev-parse HEAD)" = "$(git rev-parse c1)"
 '
 
+test_expect_failure 'pull.ff=true overrides merge.ff=false' '
+	git reset --hard c0 &&
+	test_config merge.ff false &&
+	test_config pull.ff true &&
+	git pull . c1 &&
+	verbose test "$(git rev-parse HEAD)" = "$(git rev-parse c1)"
+'
+
 test_expect_success 'fast-forward pull creates merge with "false" in pull.ff' '
 	git reset --hard c0 &&
 	test_config pull.ff false &&
-- 
2.1.4

Re: [PATCH 1/4] t7601: test for pull.ff=true overrides merge.ff=false

[Johannes Schindelin]

Hi,

On 2015-05-13 11:52, Paul Tan wrote:
> diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
> index f768c90..cef94e6 100755
> --- a/t/t7601-merge-pull-config.sh
> +++ b/t/t7601-merge-pull-config.sh
> @@ -45,6 +45,14 @@ test_expect_success 'fast-forward pull succeeds
> with "true" in pull.ff' '
>  	test "$(git rev-parse HEAD)" = "$(git rev-parse c1)"
>  '
>  
> +test_expect_failure 'pull.ff=true overrides merge.ff=false' '
> +	git reset --hard c0 &&
> +	test_config merge.ff false &&
> +	test_config pull.ff true &&
> +	git pull . c1 &&
> +	verbose test "$(git rev-parse HEAD)" = "$(git rev-parse c1)"

Given that Junio objected to this "verbose test", maybe you want to remove the "verbose"? Or introduce a `test_assert_equal` of the form

```sh
test_assert_equal () {
    test "a$1" = "a$2" || {
        echo "$1 != $2" >&2
        false
    }
}
```

Hmm. Now that I think about it, `test_eq` is probably a better name, still.

For the sake of having better reporting, say, in Continuous Integration (where re-running tests via `sh -x t????-*.sh -i -v` -- as Junio suggested -- is not an option), I agree that it would be good to report the non-matching strings.

Ciao,
Dscho

Re: [PATCH 1/4] t7601: test for pull.ff=true overrides merge.ff=false

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

>> +	verbose test "$(git rev-parse HEAD)" = "$(git rev-parse c1)"
> 
> Given that Junio objected to this "verbose test", maybe you want to remove the "verbose"? Or introduce a `test_assert_equal` of the form
>
> ```sh
> test_assert_equal () {
>     test "a$1" = "a$2" || {
>         echo "$1 != $2" >&2
>         false
>     }
> }
> ```
>
> Hmm. Now that I think about it, `test_eq` is probably a better name, still.

I do not think using "two strings" check for that one is very
useful.  You cannot tell where two 40-HEX came from.

And that is why I suggested "-i -v -x".

By the way, I cannot think of a reason why your CI cannot always run
the tests with GIT_TEST_OPTS="-v -x" exported.

> For the sake of having better reporting, say, in Continuous Integration (where re-running tests via `sh -x t????-*.sh -i -v` -- as Junio suggested -- is not an option), I agree that it would be good to report the non-matching strings.
>
> Ciao,
> Dscho

sh -x -i -v with continuous integration, was Re: [PATCH 1/4] t7601: test for pull.ff=true overrides merge.ff=false

[Johannes Schindelin]

Hi Junio,

On 2015-05-14 18:45, Junio C Hamano wrote:

> By the way, I cannot think of a reason why your CI cannot always run
> the tests with GIT_TEST_OPTS="-v -x" exported.

Asketh and ye shall be given: without running the tests in parallel, our Jenkins would take *even longer* than the three hours per test suite run (which is really painful, still, by the way). And as you know, running the tests with "-v -x" is awfully useless if you run the test suite in parallel.

Come to think of it, the proposed "verbose test" will not help this use case one bit, neither would test_eq.

Ciao,
Dscho

Re: sh -x -i -v with continuous integration, was Re: [PATCH 1/4] t7601: test for pull.ff=true overrides merge.ff=false

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> Asketh and ye shall be given: without running the tests in parallel,
> our Jenkins would take *even longer* than the three hours per test
> suite run (which is really painful, still, by the way). And as you
> know, running the tests with "-v -x" is awfully useless if you run the
> test suite in parallel.
>
> Come to think of it, the proposed "verbose test" will not help this
> use case one bit, neither would test_eq.

Yeah, I'd agree.  So let's forget about adding a way to make the
test output more verbose for CI's sake.

And I would say that "verbose test" is not a useful way to make the
test output more verbose for Human's sake; GIT_TEST_OPTS="-v -x -i"
on the other hand is.

I am very tempted to suggest doing this ;-)

 t/test-lib-functions.sh | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 0698ce7..c6c67e8 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -638,8 +638,14 @@ test_cmp_bin() {
 }
 
 # Call any command "$@" but be more verbose about its
-# failure. This is handy for commands like "test" which do
-# not output anything when they fail.
+# failure. This may seem handy for commands like "test" which do
+# not output anything when they fail, but in practice not
+# very useful for things like 'test "$actual" = "$expect"',
+# as this only shows the actual values (i.e. after $actual and
+# $expect are turned into the values in these variables).
+#
+# In short, do not add use of this; this is kept only to
+# avoid having to remove its use (for now).
 verbose () {
 	"$@" && return 0
 	echo >&2 "command failed: $(git rev-parse --sq-quote "$@")"

Re: sh -x -i -v with continuous integration, was Re: [PATCH 1/4] t7601: test for pull.ff=true overrides merge.ff=false

[Jeff King]

On Thu, May 14, 2015 at 10:06:57AM -0700, Junio C Hamano wrote:

> And I would say that "verbose test" is not a useful way to make the
> test output more verbose for Human's sake; GIT_TEST_OPTS="-v -x -i"
> on the other hand is.
> 
> I am very tempted to suggest doing this ;-)
> 
>  t/test-lib-functions.sh | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 0698ce7..c6c67e8 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -638,8 +638,14 @@ test_cmp_bin() {
>  }
>  
>  # Call any command "$@" but be more verbose about its
> -# failure. This is handy for commands like "test" which do
> -# not output anything when they fail.
> +# failure. This may seem handy for commands like "test" which do
> +# not output anything when they fail, but in practice not
> +# very useful for things like 'test "$actual" = "$expect"',
> +# as this only shows the actual values (i.e. after $actual and
> +# $expect are turned into the values in these variables).
> +#
> +# In short, do not add use of this; this is kept only to
> +# avoid having to remove its use (for now).

Sorry, but I completely disagree here (no surprise, as I was the one who
added "verbose").

It's true that it does not tell you the original commands that were
handed to the shell. But neither does the output of "test_cmp". However,
combined with the output of the test code snippet that we already
provide, it is often clear.

E.g., let's "break" a verbose test from the test suite and break it:

diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 1b2e981..f3206a3 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -113,7 +113,7 @@ test_expect_success 'diff-filter=M' '
 
 	actual=$(git log --pretty="format:%s" --diff-filter=M HEAD) &&
 	expect=$(echo second) &&
-	verbose test "$actual" = "$expect"
+	verbose test "$actual" = "foo-$expect"
 
 '
 

Right now the output is:

    expecting success: 
    
            actual=$(git log --pretty="format:%s" --diff-filter=M HEAD) &&
            expect=$(echo second) &&
            verbose test "$actual" = "foo-$expect"
    
    
    command failed:  'test' 'second' '=' 'foo-second'
    not ok 10 - diff-filter=M

which IMHO is pretty helpful. If we take away the "verbose", we get:

    expecting success: 
    
            actual=$(git log --pretty="format:%s" --diff-filter=M HEAD) &&
            expect=$(echo second) &&
            test "$actual" = "foo-$expect"
    
    
    not ok 10 - diff-filter=M

which leaves you with head-scratching (did log exit non-zero, or did the
comparison fail?). If we add in "-x", we get:

    expecting success: 

            actual=$(git log --pretty="format:%s" --diff-filter=M HEAD) &&
            expect=$(echo second) &&
            test "$actual" = "foo-$expect"
    
    
    + git log --pretty=format:%s --diff-filter=M HEAD
    + actual=second
    + echo second
    + expect=second
    + test second = foo-second
    error: last command exited with $?=1
    not ok 10 - diff-filter=M

That essentially gives us the same data, but with a lot of other cruft.
In this case, I don't think it's too bad (the test isn't that long), but
some of the "-x" cases can get pretty verbose. I guess things are a bit
better since I added the "last command exited..." in red, but I still
think the "verbose" version is the most straightforward.

Do we object to having to sprinkle the "verbose" throughout the test
scripts? IMHO it is not any worse than test_cmp. But if that is a
problem, would it be too magical to do something like:

  test () {
	verbose command test "$@"
  }

(but only inside the test snippets)?

-Peff

Re: sh -x -i -v with continuous integration, was Re: [PATCH 1/4] t7601: test for pull.ff=true overrides merge.ff=false

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Do we object to having to sprinkle the "verbose" throughout the test
> scripts?

Yes.  

An unconstrained "verbose" that applies to anything would make
people less careful to come up with more useful abstractions,
e.g. test_line_count, which I view as a bigger problem.

"verbose test", regardless of how it is spelled, either as two words
"verbose test", a "verbose_test" helper, or a helper function that
overrides "test", share the same issue, as their unconstrained-ness
comes from the fact that test is too broad a command.  I'd rather
want to see us move more in the direction of encouraging things like
test_line_count, which makes it easier to write new tests with less
mistakes (e.g. by hiding the BSD wc pitfall "$(wc -l <file)", in
test_line_count's case).

Re: sh -x -i -v with continuous integration, was Re: [PATCH 1/4] t7601: test for pull.ff=true overrides merge.ff=false

[Jeff King]

On Sat, May 16, 2015 at 12:07:23PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > Do we object to having to sprinkle the "verbose" throughout the test
> > scripts?
> 
> Yes.  
> 
> An unconstrained "verbose" that applies to anything would make
> people less careful to come up with more useful abstractions,
> e.g. test_line_count, which I view as a bigger problem.

I don't think that is true at all. They serve two different purposes.

We need "test_line_count" because of portability considerations. But we
do not need "test_eq" for that; the only reason to add it is for
debuggable output. We _could_ add a bunch of little helpers to cover
each situation, but empirically we have not[1], because it's annoying to
do so. Instead we just write a bare "test". I had hoped that "verbose"
would make it sufficiently easy to improve upon the status quo that
people would actually use it[2].

So I don't really buy the argument that "verbose" makes things worse. I
think I _do_ buy the argument that it is not worth the extra effort
given that I implemented "-x" around the same time. I think the
"verbose" output is a little nicer, as I said earlier, but having the
test-writer do literally zero work may be worth it.

-Peff

[1] The exception, AFAIK, is test_path_is_*.

[2] For the most part, actually, I think the status quo is using
    test_cmp, and most of the uses of "test" are old. Using test_cmp is
    more verbose, but it does have the advantage of checking the exit
    code of the sub-programs (and also its output is much saner for
    multi-line comparisons, but that isn't relevant to Paul's patches).

    I'd be in favor of helpers that made it shorter to use test_cmp.
    E.g.:

      # "actual" contains "foo\n"; we save one line of "echo"
      git foo >actual &&
      test_cmp_str foo actual

      # same, but take multiline input on stdin. Saves one line of "cat"
      git foo >actual &&
      test_cmp_str - actual <<-\EOF
      foo
      EOF

      # similar, but it runs the command for us and compares its
      # stdout, saving another line. Note that we take stdout as-is,
      # so you get no opportunity to post-process it (but you
      # can still do it the "long" way as above if you need to).
      test_cmp_cmd foo git foo

      # same, with stdin; obviously this doesn't work if you need to
      # send something over stdin to the command, but again, you can
      # fallback to the old way.
      test_cmp_cmd - git foo <<-\EOF
      foo
      EOF

   These also go in the direction of "verbose", by providing generic
   solutions rather than specific ones. So by the argument you made
   above, this is a bad thing.

Re: sh -x -i -v with continuous integration, was Re: [PATCH 1/4] t7601: test for pull.ff=true overrides merge.ff=false

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> [2] For the most part, actually, I think the status quo is using
>     test_cmp, and most of the uses of "test" are old. Using test_cmp is
>     more verbose, but it does have the advantage of checking the exit
>     code of the sub-programs (and also its output is much saner for
>     multi-line comparisons, but that isn't relevant to Paul's patches).
>
>     I'd be in favor of helpers that made it shorter to use test_cmp.
>     E.g.:
>
>       # "actual" contains "foo\n"; we save one line of "echo"
>       git foo >actual &&
>       test_cmp_str foo actual
>
>       # same, but take multiline input on stdin. Saves one line of "cat"
>       git foo >actual &&
>       test_cmp_str - actual <<-\EOF
>       foo
>       EOF
>
>       # similar, but it runs the command for us and compares its
>       # stdout, saving another line. Note that we take stdout as-is,
>       # so you get no opportunity to post-process it (but you
>       # can still do it the "long" way as above if you need to).
>       test_cmp_cmd foo git foo
>
>       # same, with stdin; obviously this doesn't work if you need to
>       # send something over stdin to the command, but again, you can
>       # fallback to the old way.
>       test_cmp_cmd - git foo <<-\EOF
>       foo
>       EOF
>
>    These also go in the direction of "verbose", by providing generic
>    solutions rather than specific ones. So by the argument you made
>    above, this is a bad thing.

I do not agree with that.  Perhaps "unconstained" was an unfit
phrase.

"test_cmp_cmd expected-string git foo" may be "test $a = $b" inside,
but its debug error message could be made to say "output from 'git
foo' is different from 'expected-string'", which is a very good
thing.  Contrast that with:

	verbose test "$(git foo)" = "expected-string"

which would not give you that information (and you'd need to do the
"-x" thing to find out where the string that is different from the
expected string came from).

Ultimately that is because 'test' that an individual test script
writes in it does not constrain what is on each side of comparison
(for that matter, it does not even constrain that what is tested is
comparison between two things), which makes it very hard to give
useful diagnoses.  'test' itself operates at too low a level, and
wrapping it with 'verbose' does not recover the information already
lost when invoking 'test'.

So, I suspect we want more or less the same thing.

We agree that we need shorter, easier to use, and less error prone
idioms, like test_cmp (comparing two files and show the differences
as diagnosis), test_cmp_rev (comparing two extended SHA-1
expressions as object names), and test_line_count (checking the
number of lines in a file).  I just do not think "verbose" that can
apply to any command will help us move in that direction very much,
and I find that "verbose" applied to something overly versatile like
"test" is a prime example that it does not help us much.

Re: sh -x -i -v with continuous integration, was Re: [PATCH 1/4] t7601: test for pull.ff=true overrides merge.ff=false

[Jeff King]

On Mon, May 18, 2015 at 01:41:31PM -0700, Junio C Hamano wrote:

> So, I suspect we want more or less the same thing.
> 
> We agree that we need shorter, easier to use, and less error prone
> idioms, like test_cmp (comparing two files and show the differences
> as diagnosis), test_cmp_rev (comparing two extended SHA-1
> expressions as object names), and test_line_count (checking the
> number of lines in a file).  I just do not think "verbose" that can
> apply to any command will help us move in that direction very much,
> and I find that "verbose" applied to something overly versatile like
> "test" is a prime example that it does not help us much.

I'd agree with that if you replace "need" with "it would be nice, but it
clearly isn't high enough priority for people to actually write the
helpers, so...". :)

I dunno. Maybe there are only a few helpers needed, and we can just add
them and start using them. But if we start having to grow a lot of them,
it can be annoying.

What do you want to do with the "verbose" calls I have been sprinkling
through the test suite (and the function itself)? Leave them or remove
them? A grep for "verbose " (with the trailing space) shows some
examples.

-Peff

Re: sh -x -i -v with continuous integration, was Re: [PATCH 1/4] t7601: test for pull.ff=true overrides merge.ff=false

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> What do you want to do with the "verbose" calls I have been sprinkling
> through the test suite (and the function itself)? Leave them or remove
> them? A grep for "verbose " (with the trailing space) shows some
> examples.

That was one of the things I wanted to do next, as I suspected that
the small number of existing ones might be mostly "obviously good"
ones (I never said any 'verbose test' is automatically bad; it is
just 'verbose test "$a" = "$b"' that hides what $a and $b were are
as useful as without 'verbose'), but I didn't offhand think of
examples of obviously good ones.

Re: sh -x -i -v with continuous integration, was Re: [PATCH 1/4] t7601: test for pull.ff=true overrides merge.ff=false

[Paul Tan]

Hi Johannes,

On Fri, May 15, 2015 at 12:53 AM, Johannes Schindelin
<johannes.schindelin@gmx.de> wrote:
> Asketh and ye shall be given: without running the tests in parallel, our Jenkins would take *even longer* than the three hours per test suite run (which is really painful, still, by the way). And as you know, running the tests with "-v -x" is awfully useless if you run the test suite in parallel.

Would the --tee option work for this case?

    --tee::
        In addition to printing the test output to the terminal,
        write it to files named 't/test-results/$TEST_NAME.out'.
        As the names depend on the tests' file names, it is safe to
        run the tests with this option in parallel.

Thanks,
Paul

[PATCH 2/4] pull: make pull.ff=true override merge.ff

[Paul Tan]

While since b814da8 (pull: add pull.ff configuration, 2014-01-15)
git-pull would set the --no-ff or --ff-only switch if pull.ff was false
and only respectively, it did not set the --ff switch if pull.ff was
true. This led to the inconsistent behavior that pull.ff=false and
pull.ff=only would override merge.ff, but pull.ff=true would not.

Fix this by adding the --ff switch if pull.ff=true.
---
 git-pull.sh                  | 3 +++
 t/t7601-merge-pull-config.sh | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/git-pull.sh b/git-pull.sh
index 9ed01fd..e51dd37 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -56,6 +56,9 @@ fi
 # Setup default fast-forward options via `pull.ff`
 pull_ff=$(git config pull.ff)
 case "$pull_ff" in
+true)
+	no_ff=--ff
+	;;
 false)
 	no_ff=--no-ff
 	;;
diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
index cef94e6..d16ef8b 100755
--- a/t/t7601-merge-pull-config.sh
+++ b/t/t7601-merge-pull-config.sh
@@ -45,7 +45,7 @@ test_expect_success 'fast-forward pull succeeds with "true" in pull.ff' '
 	test "$(git rev-parse HEAD)" = "$(git rev-parse c1)"
 '
 
-test_expect_failure 'pull.ff=true overrides merge.ff=false' '
+test_expect_success 'pull.ff=true overrides merge.ff=false' '
 	git reset --hard c0 &&
 	test_config merge.ff false &&
 	test_config pull.ff true &&
-- 
2.1.4

[PATCH 3/4] Documentation/config.txt: clarify that pull.ff overrides merge.ff

[Paul Tan]

Since the pull.ff setting was implemented in b814da8 (pull: add pull.ff
configuration, 2014-01-15) pull.ff has always overridden merge.ff,
although not mentioned in the documentation.

As such, clarify that this is so.

Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 Documentation/config.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 948b8b0..ec0f9ef 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2042,7 +2042,7 @@ pull.ff::
 	a case (equivalent to giving the `--no-ff` option from the command
 	line). When set to `only`, only such fast-forward merges are
 	allowed (equivalent to giving the `--ff-only` option from the
-	command line).
+	command line). This setting overrides `merge.ff` when pulling.
 
 pull.rebase::
 	When true, rebase branches on top of the fetched branch, instead
-- 
2.1.4

[PATCH 4/4] pull: parse pull.ff as a bool or string

[Paul Tan]

Since b814da8 (pull: add pull.ff configuration, 2014-01-15) git-pull
supported setting --(no-)ff via the pull.ff configuration value.
However, as it only matches the string values of "true" and "false", it
does not support other boolean aliases such as "on", "off", "1", "0".
This is inconsistent with the merge.ff setting, which supports these
aliases.

Fix this by using the bool_or_string_config function to retrieve the
value of pull.ff.

Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
 git-pull.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-pull.sh b/git-pull.sh
index e51dd37..09bc678 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -54,7 +54,7 @@ then
 fi
 
 # Setup default fast-forward options via `pull.ff`
-pull_ff=$(git config pull.ff)
+pull_ff=$(bool_or_string_config pull.ff)
 case "$pull_ff" in
 true)
 	no_ff=--ff
-- 
2.1.4

Re: [PATCH 0/4] make pull.ff=true override merge.ff

[Johannes Schindelin]

Hi Paul,

On 2015-05-13 11:52, Paul Tan wrote:
> Since b814da8 (pull: add pull.ff configuration, 2014-01-15), running
> git-pull with the configuration pull.ff=false or pull.ff=only is
> equivalent to passing --no-ff and --ff-only to git-merge. However, if
> pull.ff=true, no switch is passed to git-merge. This leads to the
> confusing behavior where pull.ff=false or pull.ff=only is able to
> override merge.ff, while pull.ff=true is unable to.
> 
> This patch series adds a failing test to demonstrates the above, and fixes it.
> 
> The documentation is also tweaked to clarify that pull.ff is meant to override
> merge.ff.
> 
> The last patch makes pull.ff consistent with merge.ff by supporting the config
> aliases of true and false (on, off, 0, 1).

Looks very good!

Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Ciao,
Dscho