[PATCH 0/5] per branch options for git-merge incl. --no-ff

[PATCH 0/5] per branch options for git-merge incl. --no-ff

[Lars Hjemli]

This patch-series implements support for per branch configuration of
git-merge, using entries (branch.<name>.mergeoptions) in .git/config.

There is also a new option, --no-ff, which can be used to force git-merge
to create merge commits even when the merge was a fast-forward.

Finally, the new optiond --ff and --commit can be used to override
setting specified in .git/config.

Note: patch 1/5 is a resend of the testscript for git-merge, this time
with correct mode bits.

Shortlog:
     Add test-script for git-merge porcelain
     git-merge: refactor option parsing
     git-merge: add support for branch.<name>.mergeoptions
     git-merge: add support for --commit
     git-merge: add --ff and --no-ff options

Diffstat:
 Documentation/config.txt        |    6 +
 Documentation/git-merge.txt     |    4 +
 Documentation/merge-options.txt |   13 ++
 git-merge.sh                    |   56 +++++-
 t/t7600-merge.sh                |  414 +++++++++++++++++++++++++++++++++++++++

 5 files changed, 483 insertions(+), 10 deletions(-)

[PATCH 1/5] Add test-script for git-merge porcelain

[Lars Hjemli]

This test-script tries to excercise the porcelainish aspects of git-merge.

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
 t/t7600-merge.sh |  317 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 317 insertions(+), 0 deletions(-)
 create mode 100755 t/t7600-merge.sh

diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
new file mode 100755
index 0000000..9e58636
--- /dev/null
+++ b/t/t7600-merge.sh
@@ -0,0 +1,317 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Lars Hjemli
+#
+
+test_description='git-merge
+
+Testing basic merge operations/option parsing.'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo "
+1
+2
+3
+4
+5
+6
+7
+8
+9
+" > file &&
+	git add file &&
+	git commit -m "commit 0" &&
+	git tag c0 &&
+	c0=$(git rev-parse HEAD) &&
+	echo "
+1 X
+2
+3
+4
+5
+6
+7
+8
+9
+" > file &&
+	git add file &&
+	git commit -m "commit 1" &&
+	git tag c1 &&
+	c1=$(git rev-parse HEAD) &&
+	git reset --hard "$c0" &&
+	echo "
+1
+2
+3
+4
+5 X
+6
+7
+8
+9
+" > file &&
+	git add file &&
+	git commit -m "commit 2" &&
+	git tag c2 &&
+	c2=$(git rev-parse HEAD) &&
+	git reset --hard "$c0" &&
+	echo "
+1
+2
+3
+4
+5
+6
+7
+8
+9 X
+" > file &&
+	git add file &&
+	git commit -m "commit 3" &&
+	git tag c3 &&
+	c3=$(git rev-parse HEAD)
+	git reset --hard "$c0"
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'test option parsing' '
+	if git merge -$ c1
+	then
+		echo "[OOPS] -$ accepted"
+		false
+	fi &&
+	if git merge --no-such c1
+	then
+		echo "[OOPS] --no-such accepted"
+		false
+	fi &&
+	if git merge -s foobar c1
+	then
+		echo "[OOPS] -s foobar accepted"
+		false
+	fi &&
+	if git merge -s=foobar c1
+	then
+		echo "[OOPS] -s=foobar accepted"
+		false
+	fi &&
+	if git merge -m
+	then
+		echo "[OOPS] missing commit msg accepted"
+		false
+	fi &&
+	if git merge
+	then
+		echo "[OOPS] missing commit references accepted"
+		false
+	fi
+'
+
+test_expect_success 'merge c0 with c1' '
+	git reset --hard c0 &&
+	git merge c1 &&
+	test "$c1" = "$(git rev-parse HEAD)"
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'verify merge result' '
+	echo "
+1 X
+2
+3
+4
+5
+6
+7
+8
+9
+" > result.1 &&
+	cmp -s file result.1
+'
+
+test_expect_success 'merge c1 with c2' '
+	git reset --hard c1 &&
+	git merge c2 &&
+	test "$c1" = "$(git rev-parse HEAD^1)" &&
+	test "$c2" = "$(git rev-parse HEAD^2)"
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'verify merge result' '
+	echo "
+1 X
+2
+3
+4
+5 X
+6
+7
+8
+9
+" > result.1-5 &&
+	cmp -s file result.1-5
+'
+
+test_expect_success 'merge c1 with c2 and c3' '
+	git reset --hard c1 &&
+	git merge c2 c3 &&
+	test "$c1" = "$(git rev-parse HEAD^1)" &&
+	test "$c2" = "$(git rev-parse HEAD^2)" &&
+	test "$c3" = "$(git rev-parse HEAD^3)"
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'verify merge result' '
+	echo "
+1 X
+2
+3
+4
+5 X
+6
+7
+8
+9 X
+" > result.1-5-9 &&
+	cmp -s file result.1-5-9
+'
+
+test_expect_success 'merge c0 with c1 (no-commit)' '
+	git reset --hard c0 &&
+	git merge --no-commit c1 &&
+	if test "$c1" != "$(git rev-parse HEAD)"
+	then
+		echo "[OOPS] fast-forward not performed"
+		false
+	fi &&
+	if ! cmp -s file result.1
+	then
+		echo "[OOPS] merge result is wrong"
+		false
+	fi
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'merge c1 with c2 (no-commit)' '
+	git reset --hard c1 &&
+	git merge --no-commit c2 &&
+	if test "$c1" != "$(git rev-parse HEAD)"
+	then
+		echo "[OOPS] HEAD changed"
+		false
+	fi &&
+	if ! cmp -s file result.1-5
+	then
+		echo "[OOPS] merge result is wrong"
+		false
+	fi &&
+	if test "$c2" != "$(cat .git/MERGE_HEAD)"
+	then
+		echo "[OOPS] MERGE_HEAD is wrong"
+		false
+	fi
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'merge c1 with c2 and c3 (no-commit)' '
+	git reset --hard c1 &&
+	git merge --no-commit c2 c3 &&
+	if test "$c1" != "$(git rev-parse HEAD)"
+	then
+		echo "[OOPS] HEAD changed"
+		false
+	fi &&
+	if ! cmp -s file result.1-5-9
+	then
+		echo "[OOPS] merge result is wrong"
+		false
+	fi &&
+	if ! grep -q "$c2" .git/MERGE_HEAD
+	then
+		echo "[OOPS] c2 not in MERGE_HEAD"
+		false
+	fi &&
+	if ! grep -q "$c3" .git/MERGE_HEAD
+	then
+		echo "[OOPS] c3 not in MERGE_HEAD"
+		false
+	fi
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'merge c0 with c1 (squash)' '
+	git reset --hard c0 &&
+	git merge --squash c1 &&
+	if test "$c0" != "$(git rev-parse HEAD)"
+	then
+		echo "[OOPS] HEAD changed"
+		false
+	fi &&
+	if ! cmp -s file result.1
+	then
+		echo "[OOPS] merge result is wrong"
+		false
+	fi &&
+	if test -f .git/MERGE_HEAD
+	then
+		echo "[OOPS] MERGE_HEAD exists"
+		false
+	fi
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'merge c1 with c2 (squash)' '
+	git reset --hard c1 &&
+	git merge --squash c2 &&
+	if test "$c1" != "$(git rev-parse HEAD)"
+	then
+		echo "[OOPS] new commit created"
+		false
+	fi &&
+	if ! cmp -s file result.1-5
+	then
+		echo "[OOPS] merge result is wrong"
+		false
+	fi &&
+	if test -f .git/MERGE_HEAD
+	then
+		echo "[OOPS] MERGE_HEAD exists"
+		false
+	fi
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'merge c1 with c2 and c3 (squash)' '
+	git reset --hard c1 &&
+	git merge --squash c2 c3 &&
+	if test "$c1" != "$(git rev-parse HEAD)"
+	then
+		echo "[OOPS] HEAD changed"
+		false
+	fi &&
+	if ! cmp -s file result.1-5-9
+	then
+		echo "[OOPS] merge result is wrong"
+		false
+	fi &&
+	if test -f .git/MERGE_HEAD
+	then
+		echo "[OOPS] MERGE_HEAD exists"
+		false
+	fi
+'
+
+test_debug 'gitk --all'
+
+test_done
-- 
1.5.3.2.82.g75c8d

[PATCH 2/5] git-merge: refactor option parsing

[Lars Hjemli]

Move the option parsing into a separate function as preparation for reuse
by the next commit.

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
 git-merge.sh |   20 ++++++++++++++------
 1 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/git-merge.sh b/git-merge.sh
index 3a01db0..a3ef676 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -119,11 +119,7 @@ merge_name () {
 	fi
 }
 
-case "$#" in 0) usage ;; esac
-
-have_message=
-while case "$#" in 0) break ;; esac
-do
+parse_option () {
 	case "$1" in
 	-n|--n|--no|--no-|--no-s|--no-su|--no-sum|--no-summ|\
 		--no-summa|--no-summar|--no-summary)
@@ -166,9 +162,21 @@ do
 		have_message=t
 		;;
 	-*)	usage ;;
-	*)	break ;;
+	*)	return 1 ;;
 	esac
 	shift
+	args_left="$#"
+}
+
+case "$#" in 0) usage ;; esac
+
+have_message=
+while parse_option "$@"
+do
+	while test $args_left -lt $#
+	do
+		shift
+	done
 done
 
 if test -z "$show_diffstat"; then
-- 
1.5.3.2.82.g75c8d

[PATCH 3/5] git-merge: add support for branch.<name>.mergeoptions

[Lars Hjemli]

This enables per branch configuration of merge options. Currently, the most
useful options to specify per branch are --squash, --summary/--no-summary
and possibly --strategy, but all options are supported.

Note: Options containing whitespace will _not_ be handled correctly. Luckily,
the only option which can include whitespace is --message and it doesn't
make much sense to give that option a default value.

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
 Documentation/config.txt    |    6 ++++
 Documentation/git-merge.txt |    4 +++
 git-merge.sh                |   16 +++++++++++
 t/t7600-merge.sh            |   63 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 89 insertions(+), 0 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 015910f..d3c25f3 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -337,6 +337,12 @@ branch.<name>.merge::
 	branch.<name>.merge to the desired branch, and use the special setting
 	`.` (a period) for branch.<name>.remote.
 
+branch.<name>.mergeoptions::
+	Sets default options for merging into branch <name>. The syntax and
+	supported options are equal to that of gitlink:git-merge[1], but
+	option values containing whitespace characters are currently not
+	supported.
+
 clean.requireForce::
 	A boolean to make git-clean do nothing unless given -f or -n.  Defaults
 	to false.
diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 144bc16..b1771a1 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -58,6 +58,10 @@ merge.verbosity::
 	above outputs debugging information.  The default is level 2.
 	Can be overriden by 'GIT_MERGE_VERBOSITY' environment variable.
 
+branch.<name>.mergeoptions::
+	Sets default options for merging into branch <name>. The syntax and
+	supported options are equal to that of git-merge, but option values
+	containing whitespace characters are currently not supported.
 
 HOW MERGE WORKS
 ---------------
diff --git a/git-merge.sh b/git-merge.sh
index a3ef676..39a24ac 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -168,9 +168,25 @@ parse_option () {
 	args_left="$#"
 }
 
+parse_config () {
+	while test "$#" != "0"
+	do
+		parse_option "$@" || usage
+		while test $args_left -lt $#
+		do
+			shift
+		done
+	done
+}
+
 case "$#" in 0) usage ;; esac
 
 have_message=
+
+branch=$(git-symbolic-ref HEAD | sed -e 's|^refs/heads/||')
+mergeopts=$(git config "branch.$branch.mergeoptions")
+parse_config $mergeopts
+
 while parse_option "$@"
 do
 	while test $args_left -lt $#
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 9e58636..e99b5d9 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -314,4 +314,67 @@ test_expect_success 'merge c1 with c2 and c3 (squash)' '
 
 test_debug 'gitk --all'
 
+test_expect_success 'merge c1 with c2 (no-commit in config)' '
+	git reset --hard c1 &&
+	git config branch.master.mergeoptions "--no-commit" &&
+	git merge c2 &&
+	if test "$c1" != "$(git rev-parse HEAD)"
+	then
+		echo "[OOPS] HEAD changed"
+		false
+	fi &&
+	if ! cmp -s file result.1-5
+	then
+		echo "[OOPS] merge result is wrong"
+		false
+	fi &&
+	if test "$c2" != "$(cat .git/MERGE_HEAD)"
+	then
+		echo "[OOPS] MERGE_HEAD is wrong"
+		false
+	fi
+'
+
+test_expect_success 'merge c1 with c2 (squash in config)' '
+	git reset --hard c1 &&
+	git config branch.master.mergeoptions "--squash" &&
+	git merge c2 &&
+	if test "$c1" != "$(git rev-parse HEAD)"
+	then
+		echo "[OOPS] new commit created"
+		false
+	fi &&
+	if ! cmp -s file result.1-5
+	then
+		echo "[OOPS] merge result is wrong"
+		false
+	fi &&
+	if test -f .git/MERGE_HEAD
+	then
+		echo "[OOPS] MERGE_HEAD exists"
+		false
+	fi
+'
+
+test_expect_success 'override config option -n' '
+	git reset --hard c1 &&
+	git config branch.master.mergeoptions "-n" &&
+	git merge --summary c2 >diffstat.txt &&
+	if ! grep -e "^ file | \+2 +-$" diffstat.txt
+	then
+		echo "[OOPS] diffstat was not generated"
+	fi
+'
+
+test_expect_success 'override config option --summary' '
+	git reset --hard c1 &&
+	git config branch.master.mergeoptions "--summary" &&
+	git merge -n c2 >diffstat.txt &&
+	if grep -e "^ file | \+2 +-$" diffstat.txt
+	then
+		echo "[OOPS] diffstat was generated"
+		false
+	fi
+'
+
 test_done
-- 
1.5.3.2.82.g75c8d

[PATCH 4/5] git-merge: add support for --commit

[Lars Hjemli]

This option can be used to override --no-commit and --squash. The change
also introduces slightly different behavior for --no-commit: when specified,
it explicitly overrides --squash.

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
 Documentation/merge-options.txt |    4 ++++
 git-merge.sh                    |    6 ++++--
 t/t7600-merge.sh                |   16 ++++++++++++++++
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index d64c259..9be90b5 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -10,6 +10,10 @@
 	not autocommit, to give the user a chance to inspect and
 	further tweak the merge result before committing.
 
+--commit::
+	Perform the merge and commit the result. This option can
+	be used to override --no-commit and --squash.
+
 --squash::
 	Produce the working tree and index state as if a real
 	merge happened, but do not actually make a commit or
diff --git a/git-merge.sh b/git-merge.sh
index 39a24ac..73ff130 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -3,7 +3,7 @@
 # Copyright (c) 2005 Junio C Hamano
 #
 
-USAGE='[-n] [--summary] [--no-commit] [--squash] [-s <strategy>] [-m=<merge-message>] <commit>+'
+USAGE='[-n] [--summary] [--[no-]commit] [--squash] [-s <strategy>] [-m=<merge-message>] <commit>+'
 
 SUBDIRECTORY_OK=Yes
 . git-sh-setup
@@ -128,8 +128,10 @@ parse_option () {
 		show_diffstat=t ;;
 	--sq|--squ|--squa|--squas|--squash)
 		squash=t no_commit=t ;;
+	--c|--co|--com|--comm|--commi|--commit)
+		squash= no_commit= ;;
 	--no-c|--no-co|--no-com|--no-comm|--no-commi|--no-commit)
-		no_commit=t ;;
+		squash= no_commit=t ;;
 	-s=*|--s=*|--st=*|--str=*|--stra=*|--strat=*|--strate=*|\
 		--strateg=*|--strategy=*|\
 	-s|--s|--st|--str|--stra|--strat|--strate|--strateg|--strategy)
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index e99b5d9..75b0ee4 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -377,4 +377,20 @@ test_expect_success 'override config option --summary' '
 	fi
 '
 
+test_expect_success 'merge c1 with c2 (override --no-commit)' '
+	git reset --hard c1 &&
+	git config branch.master.mergeoptions "--no-commit" &&
+	git merge --commit c2 &&
+	test "$c1" = "$(git rev-parse HEAD^1)" &&
+	test "$c2" = "$(git rev-parse HEAD^2)"
+'
+
+test_expect_success 'merge c1 with c2 (override --squash)' '
+	git reset --hard c1 &&
+	git config branch.master.mergeoptions "--squash" &&
+	git merge --commit c2 &&
+	test "$c1" = "$(git rev-parse HEAD^1)" &&
+	test "$c2" = "$(git rev-parse HEAD^2)"
+'
+
 test_done
-- 
1.5.3.2.82.g75c8d

[PATCH 5/5] git-merge: add --ff and --no-ff options

[Lars Hjemli]

These new options can be used to control the policy for fast-forward
merges: --ff allows it (this is the default) while --no-ff will create
a merge commit.

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
 Documentation/merge-options.txt |    9 +++++++++
 git-merge.sh                    |   20 +++++++++++++++-----
 t/t7600-merge.sh                |   18 ++++++++++++++++++
 3 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 9be90b5..b6c4dc4 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -23,6 +23,15 @@
 	top of the current branch whose effect is the same as
 	merging another branch (or more in case of an octopus).
 
+--no-ff::
+	Generate a merge commit even if the merge resolved as a
+	fast-forward.
+
+--ff::
+	Do not generate a merge commit if the merge resolved as
+	a fast-forward, only update the branch pointer. This is
+	the default behavior of git-merge.
+
 -s <strategy>, \--strategy=<strategy>::
 	Use the given merge strategy; can be supplied more than
 	once to specify them in the order they should be tried.
diff --git a/git-merge.sh b/git-merge.sh
index 73ff130..daa95a1 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -3,7 +3,7 @@
 # Copyright (c) 2005 Junio C Hamano
 #
 
-USAGE='[-n] [--summary] [--[no-]commit] [--squash] [-s <strategy>] [-m=<merge-message>] <commit>+'
+USAGE='[-n] [--summary] [--[no-]commit] [--squash] [--[no-]ff] -s <strategy>] [-m=<merge-message>] <commit>+'
 
 SUBDIRECTORY_OK=Yes
 . git-sh-setup
@@ -127,11 +127,15 @@ parse_option () {
 	--summary)
 		show_diffstat=t ;;
 	--sq|--squ|--squa|--squas|--squash)
-		squash=t no_commit=t ;;
+		allow_fast_forward=t squash=t no_commit=t ;;
 	--c|--co|--com|--comm|--commi|--commit)
-		squash= no_commit= ;;
+		allow_fast_forward=t squash= no_commit= ;;
 	--no-c|--no-co|--no-com|--no-comm|--no-commi|--no-commit)
-		squash= no_commit=t ;;
+		allow_fast_forward=t squash= no_commit=t ;;
+	--ff)
+		allow_fast_forward=t squash= no_commit= ;;
+	--no-ff)
+		allow_fast_forward=false squash= no_commit= ;;
 	-s=*|--s=*|--st=*|--str=*|--stra=*|--strat=*|--strate=*|\
 		--strateg=*|--strategy=*|\
 	-s|--s|--st|--str|--stra|--strat|--strate|--strateg|--strategy)
@@ -470,7 +474,13 @@ done
 # auto resolved the merge cleanly.
 if test '' != "$result_tree"
 then
-    parents=$(git show-branch --independent "$head" "$@" | sed -e 's/^/-p /')
+    if test "$allow_fast_forward" = "t"
+    then
+        parents=$(git show-branch --independent "$head" "$@")
+    else
+        parents=$(git rev-parse "$head" "$@")
+    fi
+    parents=$(echo "$parents" | sed -e 's/^/-p /')
     result_commit=$(printf '%s\n' "$merge_msg" | git commit-tree $result_tree $parents) || exit
     finish "$result_commit" "Merge made by $wt_strategy."
     dropsave
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 75b0ee4..6e16a28 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -393,4 +393,22 @@ test_expect_success 'merge c1 with c2 (override --squash)' '
 	test "$c2" = "$(git rev-parse HEAD^2)"
 '
 
+test_expect_success 'merge c0 with c1 (no-ff)' '
+	git reset --hard c0 &&
+	git merge --no-ff c1 &&
+	test "$c0" = "$(git rev-parse HEAD^1)" &&
+	test "$c1" = "$(git rev-parse HEAD^2)"
+'
+
+test_debug 'gitk --all'
+
+test_expect_success 'merge c0 with c1 (ff overrides no-ff)' '
+	git reset --hard c0 &&
+	git config branch.master.mergeoptions "--no-ff" &&
+	git merge --ff c1 &&
+	test "$c1" = "$(git rev-parse HEAD)"
+'
+
+test_debug 'gitk --all'
+
 test_done
-- 
1.5.3.2.82.g75c8d

Re: [PATCH 1/5] Add test-script for git-merge porcelain

[Junio C Hamano]

Lars Hjemli <hjemli@gmail.com> writes:

> This test-script tries to excercise the porcelainish aspects of git-merge.

It does exercise; no need for "tries to" ;-).

> +test_expect_success 'verify merge result' '
> +	echo "
> +1 X
> +2
> +3
> +4
> +5
> +6
> +7
> +8
> +9
> +" > result.1 &&
> +	cmp -s file result.1
> +'

Unless there is a compelling reason otherwise, I'd prefer tests
with a test vector like this to be spelled like this:

	diff -u result.1 file

That is, compare to show the difference actual output might have
from the expected result.  It's easier to spot the difference
when you later break things this way.

> +test_expect_success 'merge c1 with c2' '
> +	git reset --hard c1 &&
> +	git merge c2 &&
> +	test "$c1" = "$(git rev-parse HEAD^1)" &&
> +	test "$c2" = "$(git rev-parse HEAD^2)"
> +'

We might also want to test:

 - the index is merged;

 - the working tree matches the index;

 - the merge message (e.g. "git show -s --pretty=format:%s
   HEAD") is as expected;

Otherwise I think it is a good idea to add these tests.

By the way, I think squash_message() leaves a wrong message
template for an Octopus, which might be worth fixing.

Re: [PATCH 3/5] git-merge: add support for branch.<name>.mergeoptions

[Junio C Hamano]

Lars Hjemli <hjemli@gmail.com> writes:

> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
> index 144bc16..b1771a1 100644
> --- a/Documentation/git-merge.txt
> +++ b/Documentation/git-merge.txt
> @@ -58,6 +58,10 @@ merge.verbosity::
>  	above outputs debugging information.  The default is level 2.
>  	Can be overriden by 'GIT_MERGE_VERBOSITY' environment variable.
>  
> +branch.<name>.mergeoptions::
> +	Sets default options for merging into branch <name>. The syntax and
> +	supported options are equal to that of git-merge, but option values
> +	containing whitespace characters are currently not supported.

I think this is a sensible thing to do in general.

> +branch=$(git-symbolic-ref HEAD | sed -e 's|^refs/heads/||')
> +mergeopts=$(git config "branch.$branch.mergeoptions")
> +parse_config $mergeopts

What should happen when your head is detached?

Re: [PATCH 4/5] git-merge: add support for --commit

[Junio C Hamano]

Lars Hjemli <hjemli@gmail.com> writes:

> This option can be used to override --no-commit and --squash. The change
> also introduces slightly different behavior for --no-commit: when specified,
> it explicitly overrides --squash.

Makes me wonder if --no-squash also make sense to override a --squash.
Is this really needed?  IOW, does it ever make sense to have --no-commit
in the configuration?

Re: [PATCH 1/5] Add test-script for git-merge porcelain

[Lars Hjemli]

On 9/23/07, Junio C Hamano <gitster@pobox.com> wrote:
> Lars Hjemli <hjemli@gmail.com> writes:
>
> > This test-script tries to excercise the porcelainish aspects of git-merge.
>
> It does exercise; no need for "tries to" ;-).

Heh, ok

> > +     cmp -s file result.1
> > +'
>
> Unless there is a compelling reason otherwise, I'd prefer tests
> with a test vector like this to be spelled like this:
>
>         diff -u result.1 file
>
> That is, compare to show the difference actual output might have
> from the expected result.  It's easier to spot the difference
> when you later break things this way.

Good point, I'll change it.

>
> > +test_expect_success 'merge c1 with c2' '
> > +     git reset --hard c1 &&
> > +     git merge c2 &&
> > +     test "$c1" = "$(git rev-parse HEAD^1)" &&
> > +     test "$c2" = "$(git rev-parse HEAD^2)"
> > +'
>
> We might also want to test:
>
>  - the index is merged;
>
>  - the working tree matches the index;
>
>  - the merge message (e.g. "git show -s --pretty=format:%s
>    HEAD") is as expected;

Yes, sorry for being lazy ;-)  I'll add more thorough verification

>
> Otherwise I think it is a good idea to add these tests.
>
> By the way, I think squash_message() leaves a wrong message
> template for an Octopus, which might be worth fixing.

Ok, I'll try to detect it in the test-script and send a proper fix.

Thanks for the review.

-- 
larsh

Re: [PATCH 3/5] git-merge: add support for branch.<name>.mergeoptions

[Lars Hjemli]

On 9/23/07, Junio C Hamano <gitster@pobox.com> wrote:
> Lars Hjemli <hjemli@gmail.com> writes:
> > +branch=$(git-symbolic-ref HEAD | sed -e 's|^refs/heads/||')
> > +mergeopts=$(git config "branch.$branch.mergeoptions")
> > +parse_config $mergeopts
>
> What should happen when your head is detached?
>

My plan was 'nothing', but I should have tested it (it does work, but
also prints an ugly "fatal: ref HEAD is not a symbolic ref").

-- 
larsh

Re: [PATCH 4/5] git-merge: add support for --commit

[Lars Hjemli]

On 9/23/07, Junio C Hamano <gitster@pobox.com> wrote:
> Lars Hjemli <hjemli@gmail.com> writes:
>
> > This option can be used to override --no-commit and --squash. The change
> > also introduces slightly different behavior for --no-commit: when specified,
> > it explicitly overrides --squash.
>
> Makes me wonder if --no-squash also make sense to override a --squash.
> Is this really needed?  IOW, does it ever make sense to have --no-commit
> in the configuration?
>

Actually, I did add --no-squash but the semantics (and implementation)
was identical to --commit so I decided it wasn't worth it (but maybe
it is - just to avoid user confusion?).

-- 
larsh

Re: [PATCH 3/5] git-merge: add support for branch.<name>.mergeoptions

[Junio C Hamano]

"Lars Hjemli" <hjemli@gmail.com> writes:

> On 9/23/07, Junio C Hamano <gitster@pobox.com> wrote:
>> Lars Hjemli <hjemli@gmail.com> writes:
>> > +branch=$(git-symbolic-ref HEAD | sed -e 's|^refs/heads/||')
>> > +mergeopts=$(git config "branch.$branch.mergeoptions")
>> > +parse_config $mergeopts
>>
>> What should happen when your head is detached?
>>
>
> My plan was 'nothing', but I should have tested it (it does work, but
> also prints an ugly "fatal: ref HEAD is not a symbolic ref").

That, and also running "git config branch..mergeoptions" and
expect it does not change behaviour to issue more strict
warning, are both not so good.  Perhaps the code needs to be
more defensive like:

    if branch=$(git symbolic-ref -q HEAD)
    then
        mergeopts=$(git config "branch.${branch#refs/heads/}.mergeoptions")
	if test -n "$mergeopts"
        then
	    parse_config $mergeopts
	fi
    fi


 

Re: [PATCH 3/5] git-merge: add support for branch.<name>.mergeoptions

[Lars Hjemli]

On 9/23/07, Junio C Hamano <gitster@pobox.com> wrote:
> "Lars Hjemli" <hjemli@gmail.com> writes:
>
> > On 9/23/07, Junio C Hamano <gitster@pobox.com> wrote:
> >> Lars Hjemli <hjemli@gmail.com> writes:
> >> > +branch=$(git-symbolic-ref HEAD | sed -e 's|^refs/heads/||')
> >> > +mergeopts=$(git config "branch.$branch.mergeoptions")
> >> > +parse_config $mergeopts
> >>
> >> What should happen when your head is detached?
> >>
> >
> > My plan was 'nothing', but I should have tested it (it does work, but
> > also prints an ugly "fatal: ref HEAD is not a symbolic ref").
>
> That, and also running "git config branch..mergeoptions" and
> expect it does not change behaviour to issue more strict
> warning, are both not so good.  Perhaps the code needs to be
> more defensive like:
>
>     if branch=$(git symbolic-ref -q HEAD)
>     then
>         mergeopts=$(git config "branch.${branch#refs/heads/}.mergeoptions")
>         if test -n "$mergeopts"
>         then
>             parse_config $mergeopts
>         fi
>     fi

Yes, this is much better (and the ${parameter#word} syntax was nice too).

Thanks.

--
larsh