threads / patch / 39355

v2, 2 partsmake pull.ff=true override merge.ff

Subject: [PATCH v2 0/2] make pull.ff=true override merge.ff

## tl;dr

5 messages between May 18, 2015 and May 18, 2015. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Paul Tan· May 18, 2015, 13:45 UTC · lore

This is a re-roll of [1], primarily to remove the use of "verbose".

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

[1] http://thread.gmane.org/gmane.comp.version-control.git/268972

Paul Tan (2):
  pull: make pull.ff=true override 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

Paul Tan· May 18, 2015, 13:45 UTC · re: Paul Tan · lore

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

Fix this by adding the --ff switch if pull.ff=true, and add a test to catch future regressions.

Furthermore, clarify in the documentation that pull.ff overrides merge.ff.

Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
* Removed use of "verbose".

 Documentation/config.txt     | 2 +-
 git-pull.sh                  | 3 +++
 t/t7601-merge-pull-config.sh | 8 ++++++++
 3 files changed, 12 insertions(+), 1 deletion(-)

Show changes to 3 files +12 −1

Documentation/config.txt, git-pull.sh, t/t7601-merge-pull-config.sh

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
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 f768c90..c6c44ec 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_success 'pull.ff=true overrides merge.ff=false' '
+	git reset --hard c0 &&
+	test_config merge.ff false &&
+	test_config pull.ff true &&
+	git pull . c1 &&
+	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

Paul Tan· May 18, 2015, 13:45 UTC · re: Paul Tan · lore

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

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

Show changes to git-pull.sh +1 −1

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

Johannes Schindelin· May 18, 2015, 14:46 UTC · re: Paul Tan · lore

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

Hi Paul,

On 2015-05-18 15:45, Paul Tan wrote:

> This is a re-roll of [1], primarily to remove the use of "verbose".

I think these two patches are good to go now.

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

Ciao, Dscho

Junio C Hamano· May 18, 2015, 18:22 UTC · re: Johannes Schindelin · lore

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

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

Show 11 quoted lines

> Hi Paul,
>
> On 2015-05-18 15:45, Paul Tan wrote:
>> This is a re-roll of [1], primarily to remove the use of "verbose".
>
> I think these two patches are good to go now.
>
> Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> Ciao,
> Dscho

Yeah, looks good.  Thanks, both.