Skip to contentSkip to conversation

Fetch: branch properties for fetch.

PATCH In review Santi Béjar 2 participants 6 messages Jun 15, 2016 Documentation

lore
Share & tools
lore
#0 lore raw

cover Fetch: branch properties for fetch.

Hi *,

This patch series implements the following branch properties for fetch:

- default repository to fetch
- default remotes branches to merge from the default repository

and adds some tests.

It passes all the tests.

Santi

---
 Documentation/config.txt |    7 +++++
 git-fetch.sh             |    9 +++---
 git-parse-remote.sh      |   43 ++++++++++++++++++++++++------
 t/t5510-fetch.sh         |   67 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 113 insertions(+), 13 deletions(-)
#1 lore raw

PATCH 1/4 fetch: default remote repository from branch properties

If in branch "foo" and this in config:

[branch "foo"]
remote=bar

"git fetch" = "git fetch bar"
"git pull" = "git pull bar"

---
 Documentation/config.txt |    3 +++
 git-fetch.sh             |    9 ++++-----
 git-parse-remote.sh      |    6 ++++++
 3 files changed, 13 insertions(+), 5 deletions(-)
Documentation/config.txt +3 -0
index bb2fbc3..04c5094 100644
@@ -119,6 +119,9 @@ apply.whitespace::
 	Tells `git-apply` how to handle whitespaces, in the same way
 	as the '--whitespace' option. See gitlink:git-apply[1].
 
+branch.<name>.remote::
+	When in branch <name>, it tells `git fetch` which remote to fetch.
+
 pager.color::
 	A boolean to enable/disable colored output when the pager is in
 	use (default is true).
git-fetch.sh +4 -5
index 09a5d6c..50ad101 100755
@@ -68,11 +68,10 @@ done
 
 case "$#" in
 0)
-	test -f "$GIT_DIR/branches/origin" ||
-		test -f "$GIT_DIR/remotes/origin" ||
-			git-repo-config --get remote.origin.url >/dev/null ||
-				die "Where do you want to fetch from today?"
-	set origin ;;
+	origin=$(get_default_remote)
+	test -n "$(get_remote_url ${origin})" ||
+		die "Where do you want to fetch from today?"
+	set x $origin ; shift ;;
 esac
 
 remote_nick="$1"
git-parse-remote.sh +6 -0
index 187f088..6999816 100755
@@ -68,6 +68,12 @@ get_remote_url () {
 	esac
 }
 
+get_default_remote () {
+	curr_branch=$(git-symbolic-ref HEAD | sed -e 's|^refs/heads/||')
+	origin=$(git-repo-config --get "branch.$curr_branch.remote")
+	echo ${origin:-origin}
+}
+
 get_remote_default_refs_for_push () {
 	data_source=$(get_data_source "$1")
 	case "$data_source" in
Signature · 1 line
1.4.2.1.g279b
#2 lore raw

PATCH 2/4 Add t5510 to test per branch configuration affecting git-fetch.

In reply to Santi Béjar

From: Junio C Hamano <[email protected]>

---
I hope the attribution is ok.

 t/t5510-fetch.sh |   44 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 44 insertions(+), 0 deletions(-)
t/t5510-fetch.sh new file +44 -0
new file mode 100755
index 0000000..e71581a
@@ -0,0 +1,44 @@
+#!/bin/sh
+# Copyright (c) 2006, Junio C Hamano.
+
+test_description='Per branch config variables affects "git fetch".
+
+'
+
+. ./test-lib.sh
+
+D=`pwd`
+
+test_expect_success setup '
+	echo >file original &&
+	git add file &&
+	git commit -a -m original'
+
+test_expect_success "clone and setup child repos" '
+	git clone . one &&
+	cd one &&
+	echo >file updated by one &&
+	git commit -a -m "updated by one" &&
+	cd .. &&
+	git clone . two &&
+	cd two &&
+	git repo-config branch.master.remote one &&
+	{
+		echo "URL: ../one/.git/"
+		echo "Pull: refs/heads/master:refs/heads/one"
+	} >.git/remotes/one
+'
+
+test_expect_success "fetch test" '
+	cd "$D" &&
+	echo >file updated by origin &&
+	git commit -a -m "updated by origin" &&
+	cd two &&
+	git fetch &&
+	test -f .git/refs/heads/one &&
+	mine=`git rev-parse refs/heads/one` &&
+	his=`cd ../one && git rev-parse refs/heads/master` &&
+	test "z$mine" = "z$his"
+'
+
+test_done
Signature · 1 line
1.4.2.1.g279b
#3 lore raw

PATCH 3/4 fetch: get the remote branches to merge from the branch properties

In reply to Santi Béjar

If in branch "foo" and this in config:

[branch "foo"]
merge=bar

"git fetch": fetch from the default repository and program the "bar"
branch to be merged with pull.

---
 Documentation/config.txt |    4 ++++
 git-parse-remote.sh      |   37 +++++++++++++++++++++++++++++--------
 2 files changed, 33 insertions(+), 8 deletions(-)
Documentation/config.txt +4 -0
index 04c5094..98c1f3e 100644
@@ -122,6 +122,10 @@ apply.whitespace::
 branch.<name>.remote::
 	When in branch <name>, it tells `git fetch` which remote to fetch.
 
+branch.<name>.merge::
+	When in branch <name>, it tells `git fetch` the default remote branch
+	to be merged.
+
 pager.color::
 	A boolean to enable/disable colored output when the pager is in
 	use (default is true).
git-parse-remote.sh +29 -8
index 6999816..c325ef7 100755
@@ -92,9 +92,22 @@ get_remote_default_refs_for_push () {
 
 # Subroutine to canonicalize remote:local notation.
 canon_refs_list_for_fetch () {
-	# Leave only the first one alone; add prefix . to the rest
+	# If called from get_remote_default_refs_for_fetch
+	# leave the branches in branch.${curr_branch}.merge alone,
+	# or the first one otherwise; add prefix . to the rest
 	# to prevent the secondary branches to be merged by default.
-	dot_prefix=
+	merge_branches=
+	if test "$1" = "-d"
+	then
+		shift ; remote="$1" ; shift
+		if test "$remote" = "$(get_default_remote)"
+		then
+			curr_branch=$(git-symbolic-ref HEAD | \
+			    sed -e 's|^refs/heads/||')
+			merge_branches=$(git-repo-config \
+			    --get-all "branch.${curr_branch}.merge")
+		fi
+	fi
 	for ref
 	do
 		force=
@@ -107,6 +120,18 @@ canon_refs_list_for_fetch () {
 		expr "z$ref" : 'z.*:' >/dev/null || ref="${ref}:"
 		remote=$(expr "z$ref" : 'z\([^:]*\):')
 		local=$(expr "z$ref" : 'z[^:]*:\(.*\)')
+		dot_prefix=.
+		if test -z "$merge_branches"
+		then
+			merge_branches=$remote
+			dot_prefix=
+		else
+			for merge_branch in $merge_branches
+			do
+			    [ "$remote" = "$merge_branch" ] &&
+			    dot_prefix= && break
+			done
+		fi
 		case "$remote" in
 		'') remote=HEAD ;;
 		refs/heads/* | refs/tags/* | refs/remotes/*) ;;
@@ -126,7 +151,6 @@ canon_refs_list_for_fetch () {
 		   die "* refusing to create funny ref '$local_ref_name' locally"
 		fi
 		echo "${dot_prefix}${force}${remote}:${local}"
-		dot_prefix=.
 	done
 }
 
@@ -137,7 +161,7 @@ get_remote_default_refs_for_fetch () {
 	'' | config-partial | branches-partial)
 		echo "HEAD:" ;;
 	config)
-		canon_refs_list_for_fetch \
+		canon_refs_list_for_fetch -d "$1" \
 			$(git-repo-config --get-all "remote.$1.fetch") ;;
 	branches)
 		remote_branch=$(sed -ne '/#/s/.*#//p' "$GIT_DIR/branches/$1")
@@ -145,10 +169,7 @@ get_remote_default_refs_for_fetch () {
 		echo "refs/heads/${remote_branch}:refs/heads/$1"
 		;;
 	remotes)
-		# This prefixes the second and later default refspecs
-		# with a '.', to signal git-fetch to mark them
-		# not-for-merge.
-		canon_refs_list_for_fetch $(sed -ne '/^Pull: */{
+		canon_refs_list_for_fetch -d "$1" $(sed -ne '/^Pull: */{
 						s///p
 					}' "$GIT_DIR/remotes/$1")
 		;;
Signature · 1 line
1.4.2.1.g279b
#4 lore raw

PATCH 4/4 Add test for the default merges in fetch.

In reply to Santi Béjar
---
 t/t5510-fetch.sh |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)
t/t5510-fetch.sh +23 -0
index e71581a..c7faffe
@@ -27,6 +27,16 @@ test_expect_success "clone and setup chi
 		echo "URL: ../one/.git/"
 		echo "Pull: refs/heads/master:refs/heads/one"
 	} >.git/remotes/one
+	cd .. &&
+	git clone . three &&
+	cd three &&
+	git repo-config branch.master.remote two
+	git repo-config branch.master.merge refs/heads/one
+	{
+		echo "URL: ../two/.git/"
+		echo "Pull: refs/heads/master:refs/heads/two"
+		echo "Pull: refs/heads/one:refs/heads/one"
+	} >.git/remotes/two
 '
 
 test_expect_success "fetch test" '
@@ -41,4 +51,17 @@ test_expect_success "fetch test" '
 	test "z$mine" = "z$his"
 '
 
+test_expect_success "fetch test for-merge" '
+	cd "$D" &&
+	cd three &&
+	git fetch &&
+	test -f .git/refs/heads/two &&
+	test -f .git/refs/heads/one &&
+	{
+		echo -e "not-for-merge	branch \047master\047 of ../two/"
+		echo -e "	branch \047one\047 of ../two/"
+	} > expected &&
+	cut -f 2- .git/FETCH_HEAD > actual &&
+	diff expected actual'
+
 test_done
Signature · 1 line
1.4.2.1.g279b
#5 lore raw

Re: [PATCH 4/4 (take 4)] Add test for the default merges in fetch.

Review

Quoted 11 lines · Santi Béjar
(quoted diff) +10 -0
+	cd .. &&
+	git clone . three &&
+	cd three &&
+	git repo-config branch.master.remote two
+	git repo-config branch.master.merge refs/heads/one
+	{
+		echo "URL: ../two/.git/"
+		echo "Pull: refs/heads/master:refs/heads/two"
+		echo "Pull: refs/heads/one:refs/heads/one"
+	} >.git/remotes/two
 '

Please string them with && unless there is a compelling reason not to.

That would catch a potential error exit from repo-config, although it is not a focus of this test. We have caught breakage of parts of the system by tests meant for unrelated parts of the system number of times.

Review

Quoted 12 lines
(quoted diff) +12 -0
+test_expect_success "fetch test for-merge" '
+	cd "$D" &&
+	cd three &&
+	git fetch &&
+	test -f .git/refs/heads/two &&
+	test -f .git/refs/heads/one &&
+	{
+		echo -e "not-for-merge	branch \047master\047 of ../two/"
+		echo -e "	branch \047one\047 of ../two/"
+	} > expected &&
+	cut -f 2- .git/FETCH_HEAD > actual &&
+	diff expected actual'

Testing for the explicit implementation detail (namely, the human readable part of the merge log message per branch) makes me feel uneasy. Also I've stayed away from "echo -e" for some inexplicable fear of portability issues and I do not see a reason to use it here. Time to learn to use '\'' perhaps?

I would have:

- arranged branch tips of all the repos that are potentially involved to have different object names;

- checked for the commit object names at the beginning of the resulting FETCH_HEAD and not-for-merge markers;

- ignored the "branch 'foo' of repo" log message pieces;.

In other words, this on top of yours, which I am going to commit.

t/t5510-fetch.sh +8 -6
index 2d8da59..df0ae48 100755
@@ -30,8 +30,8 @@ test_expect_success "clone and setup chi
 	cd .. &&
 	git clone . three &&
 	cd three &&
-	git repo-config branch.master.remote two
-	git repo-config branch.master.merge refs/heads/one
+	git repo-config branch.master.remote two &&
+	git repo-config branch.master.merge refs/heads/one &&
 	{
 		echo "URL: ../two/.git/"
 		echo "Pull: refs/heads/master:refs/heads/two"
@@ -57,11 +57,13 @@ test_expect_success "fetch test for-merg
 	git fetch &&
 	test -f .git/refs/heads/two &&
 	test -f .git/refs/heads/one &&
+	master_in_two=`cd ../two && git rev-parse master` &&
+	one_in_two=`cd ../two && git rev-parse one` &&
 	{
-		echo -e "not-for-merge	branch \047master\047 of ../two/"
-		echo -e "	branch \047one\047 of ../two/"
-	} > expected &&
-	cut -f 2- .git/FETCH_HEAD > actual &&
+		echo "$master_in_two	not-for-merge"
+		echo "$one_in_two	"
+	} >expected &&
+	cut -f -2 .git/FETCH_HEAD >actual &&
 	diff expected actual'
 
 test_done