[PATCH v2 00/14] Classify {branches,remotes}-file as fringe features

[PATCH v2 00/14] Classify {branches,remotes}-file as fringe features

[Ramkumar Ramachandra]

Hi,

Since I didn't explain the series properly last time: this series
reduces the burden of the branches-file/remotes-file cruft.  It does
not argue for deprecation, because it is still useful to some people
and harms nobody.  It removes dead code, tests the feature properly,
and doesn't mention it in obscure places.

In this iteration, [1/14] does a file-wide style cleanup (wasn't as
painful as I thought).  Also, one s/git config/test_config/ patch has
been dropped because it adds no value.  This time, I actually ran all
the tests to make sure that I made no silly mistakes (there were quite
a few last time).

Thanks to Junio and Eric Sunshine for reviews.

Ramkumar Ramachandra (14):
  t/t5505-remote: modernize style
  t/t5505-remote: test push-refspec in branches-file
  t/t5505-remote: use test_path_is_missing
  t/t5505-remote: remove dependency on $origin_url
  remote: remove dead code in read_branches_file()
  t/t5505-remote: test url-with-# in branches-file
  t/t5516-fetch-push: don't use branches-file
  ls-remote doc: fix example invocation on git.git
  ls-remote doc: rewrite <repository> paragraph
  ls-remote doc: don't encourage use of branches-file
  t/t5505-remote: test multiple push/pull in remotes-file
  t/t5510-fetch: don't use remotes-file
  t/t5515-fetch-merge-logic: don't use {branches,remotes}-file
  remote: add comment about read_{branches,remotes}_file

 Documentation/git-ls-remote.txt |  11 +-
 remote.c                        |  34 +-
 t/t5505-remote.sh               | 852 ++++++++++++++++++++++------------------
 t/t5510-fetch.sh                |   9 +-
 t/t5515-fetch-merge-logic.sh    |  28 +-
 t/t5516-fetch-push.sh           |  16 +-
 6 files changed, 516 insertions(+), 434 deletions(-)

-- 
1.8.3.1.498.gacf2885

[PATCH v2 01/14] t/t5505-remote: modernize style

[Ramkumar Ramachandra]

Modernize the style of all tests throughout the file:
- Remove spurious blank lines.
- Indent the test body.
- Make sure that all lines end with &&, to make it easier to spot breaks
  in the chain.
- When executing something in a subshell, put the parenthesis on
  separate lines and indent the body.  Also make sure that the first
  statement in the subshell is a 'cd'.
- When redirecting output, replace the > output forms with >output.
- Use the <<-\EOF and <<-EOF forms of heredoc, not <<EOF.  Also, don't
  de-indent the heredoc body.
- When creating an empty file, use : >output form over >output for
  clarity.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 t/t5505-remote.sh | 813 +++++++++++++++++++++++++++++-------------------------
 1 file changed, 440 insertions(+), 373 deletions(-)

diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index dd10ff0..f679ded 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -42,107 +42,104 @@ check_tracking_branch () {
 }
 
 test_expect_success setup '
-
 	setup_repository one &&
 	setup_repository two &&
 	(
-		cd two && git branch another
+		cd two &&
+		git branch another
 	) &&
 	git clone one test
-
 '
 
 test_expect_success C_LOCALE_OUTPUT 'remote information for the origin' '
-(
-	cd test &&
-	tokens_match origin "$(git remote)" &&
-	check_remote_track origin master side &&
-	check_tracking_branch origin HEAD master side
-)
+	(
+		cd test &&
+		tokens_match origin "$(git remote)" &&
+		check_remote_track origin master side &&
+		check_tracking_branch origin HEAD master side
+	)
 '
 
 test_expect_success 'add another remote' '
-(
-	cd test &&
-	git remote add -f second ../two &&
-	tokens_match "origin second" "$(git remote)" &&
-	check_tracking_branch second master side another &&
-	git for-each-ref "--format=%(refname)" refs/remotes |
-	sed -e "/^refs\/remotes\/origin\//d" \
-	    -e "/^refs\/remotes\/second\//d" >actual &&
-	>expect &&
-	test_cmp expect actual
-)
+	(
+		cd test &&
+		git remote add -f second ../two &&
+		tokens_match "origin second" "$(git remote)" &&
+		check_tracking_branch second master side another &&
+		git for-each-ref "--format=%(refname)" refs/remotes |
+		sed -e "/^refs\/remotes\/origin\//d" \
+		    -e "/^refs\/remotes\/second\//d" >actual &&
+		>expect &&
+		test_cmp expect actual
+	)
 '
 
 test_expect_success C_LOCALE_OUTPUT 'check remote tracking' '
-(
-	cd test &&
-	check_remote_track origin master side &&
-	check_remote_track second master side another
-)
+	(
+		cd test &&
+		check_remote_track origin master side &&
+		check_remote_track second master side another
+	)
 '
 
 test_expect_success 'remote forces tracking branches' '
-(
-	cd test &&
-	case `git config remote.second.fetch` in
-	+*) true ;;
-	 *) false ;;
-	esac
-)
+	(
+		cd test &&
+		case `git config remote.second.fetch` in
+		+*) true ;;
+		 *) false ;;
+		esac
+	)
 '
 
 test_expect_success 'remove remote' '
-(
-	cd test &&
-	git symbolic-ref refs/remotes/second/HEAD refs/remotes/second/master &&
-	git remote rm second
-)
+	(
+		cd test &&
+		git symbolic-ref refs/remotes/second/HEAD refs/remotes/second/master &&
+		git remote rm second
+	)
 '
 
 test_expect_success C_LOCALE_OUTPUT 'remove remote' '
-(
-	cd test &&
-	tokens_match origin "$(git remote)" &&
-	check_remote_track origin master side &&
-	git for-each-ref "--format=%(refname)" refs/remotes |
-	sed -e "/^refs\/remotes\/origin\//d" >actual &&
-	>expect &&
-	test_cmp expect actual
-)
+	(
+		cd test &&
+		tokens_match origin "$(git remote)" &&
+		check_remote_track origin master side &&
+		git for-each-ref "--format=%(refname)" refs/remotes |
+		sed -e "/^refs\/remotes\/origin\//d" >actual &&
+		: >expect &&
+		test_cmp expect actual
+	)
 '
 
 test_expect_success 'remove remote protects local branches' '
-(
-	cd test &&
-	{ cat >expect1 <<EOF
-Note: A branch outside the refs/remotes/ hierarchy was not removed;
-to delete it, use:
-  git branch -d master
-EOF
-	} &&
-	{ cat >expect2 <<EOF
-Note: Some branches outside the refs/remotes/ hierarchy were not removed;
-to delete them, use:
-  git branch -d foobranch
-  git branch -d master
-EOF
-	} &&
-	git tag footag &&
-	git config --add remote.oops.fetch "+refs/*:refs/*" &&
-	git remote remove oops 2>actual1 &&
-	git branch foobranch &&
-	git config --add remote.oops.fetch "+refs/*:refs/*" &&
-	git remote rm oops 2>actual2 &&
-	git branch -d foobranch &&
-	git tag -d footag &&
-	test_i18ncmp expect1 actual1 &&
-	test_i18ncmp expect2 actual2
-)
-'
-
-cat > test/expect << EOF
+	(
+		cd test &&
+		cat >expect1 <<-\EOF &&
+		Note: A branch outside the refs/remotes/ hierarchy was not removed;
+		to delete it, use:
+		  git branch -d master
+		EOF
+		cat >expect2 <<-\EOF &&
+		Note: Some branches outside the refs/remotes/ hierarchy were not removed;
+		to delete them, use:
+		  git branch -d foobranch
+		  git branch -d master
+		EOF
+		git tag footag &&
+		git config --add remote.oops.fetch "+refs/*:refs/*" &&
+		git remote remove oops 2>actual1 &&
+		git branch foobranch &&
+		git config --add remote.oops.fetch "+refs/*:refs/*" &&
+		git remote rm oops 2>actual2 &&
+		git branch -d foobranch &&
+		git tag -d footag &&
+		test_i18ncmp expect1 actual1 &&
+		test_i18ncmp expect2 actual2
+	)
+'
+
+cat >test/expect <<-EOF
 * remote origin
   Fetch URL: $(pwd)/one
   Push  URL: $(pwd)/one
@@ -172,36 +169,40 @@ cat > test/expect << EOF
 EOF
 
 test_expect_success 'show' '
-	(cd test &&
-	 git config --add remote.origin.fetch refs/heads/master:refs/heads/upstream &&
-	 git fetch &&
-	 git checkout -b ahead origin/master &&
-	 echo 1 >> file &&
-	 test_tick &&
-	 git commit -m update file &&
-	 git checkout master &&
-	 git branch --track octopus origin/master &&
-	 git branch --track rebase origin/master &&
-	 git branch -d -r origin/master &&
-	 git config --add remote.two.url ../two &&
-	 git config --add remote.two.pushurl ../three &&
-	 git config branch.rebase.rebase true &&
-	 git config branch.octopus.merge "topic-a topic-b topic-c" &&
-	 (cd ../one &&
-	  echo 1 > file &&
-	  test_tick &&
-	  git commit -m update file) &&
-	 git config --add remote.origin.push : &&
-	 git config --add remote.origin.push refs/heads/master:refs/heads/upstream &&
-	 git config --add remote.origin.push +refs/tags/lastbackup &&
-	 git config --add remote.two.push +refs/heads/ahead:refs/heads/master &&
-	 git config --add remote.two.push refs/heads/master:refs/heads/another &&
-	 git remote show origin two > output &&
-	 git branch -d rebase octopus &&
-	 test_i18ncmp expect output)
-'
-
-cat > test/expect << EOF
+	(
+		cd test &&
+		git config --add remote.origin.fetch refs/heads/master:refs/heads/upstream &&
+		git fetch &&
+		git checkout -b ahead origin/master &&
+		echo 1 >>file &&
+		test_tick &&
+		git commit -m update file &&
+		git checkout master &&
+		git branch --track octopus origin/master &&
+		git branch --track rebase origin/master &&
+		git branch -d -r origin/master &&
+		git config --add remote.two.url ../two &&
+		git config --add remote.two.pushurl ../three &&
+		git config branch.rebase.rebase true &&
+		git config branch.octopus.merge "topic-a topic-b topic-c" &&
+		(
+			cd ../one &&
+			echo 1 >file &&
+			test_tick &&
+			git commit -m update file
+		) &&
+		git config --add remote.origin.push : &&
+		git config --add remote.origin.push refs/heads/master:refs/heads/upstream &&
+		git config --add remote.origin.push +refs/tags/lastbackup &&
+		git config --add remote.two.push +refs/heads/ahead:refs/heads/master &&
+		git config --add remote.two.push refs/heads/master:refs/heads/another &&
+		git remote show origin two >output &&
+		git branch -d rebase octopus &&
+		test_i18ncmp expect output
+	)
+'
+
+cat >test/expect <<-EOF
 * remote origin
   Fetch URL: $(pwd)/one
   Push  URL: $(pwd)/one
@@ -219,152 +220,187 @@ cat > test/expect << EOF
 EOF
 
 test_expect_success 'show -n' '
-	(mv one one.unreachable &&
-	 cd test &&
-	 git remote show -n origin > output &&
-	 mv ../one.unreachable ../one &&
-	 test_i18ncmp expect output)
+	mv one one.unreachable &&
+	(
+		cd test &&
+		git remote show -n origin >output &&
+		mv ../one.unreachable ../one &&
+		test_i18ncmp expect output
+	)
 '
 
 test_expect_success 'prune' '
-	(cd one &&
-	 git branch -m side side2) &&
-	(cd test &&
-	 git fetch origin &&
-	 git remote prune origin &&
-	 git rev-parse refs/remotes/origin/side2 &&
-	 test_must_fail git rev-parse refs/remotes/origin/side)
+	(
+		cd one &&
+		git branch -m side side2
+	) &&
+	(
+		cd test &&
+		git fetch origin &&
+		git remote prune origin &&
+		git rev-parse refs/remotes/origin/side2 &&
+		test_must_fail git rev-parse refs/remotes/origin/side
+	)
 '
 
 test_expect_success 'set-head --delete' '
-	(cd test &&
-	 git symbolic-ref refs/remotes/origin/HEAD &&
-	 git remote set-head --delete origin &&
-	 test_must_fail git symbolic-ref refs/remotes/origin/HEAD)
+	(
+		cd test &&
+		git symbolic-ref refs/remotes/origin/HEAD &&
+		git remote set-head --delete origin &&
+		test_must_fail git symbolic-ref refs/remotes/origin/HEAD
+	)
 '
 
 test_expect_success 'set-head --auto' '
-	(cd test &&
-	 git remote set-head --auto origin &&
-	 echo refs/remotes/origin/master >expect &&
-	 git symbolic-ref refs/remotes/origin/HEAD >output &&
-	 test_cmp expect output
+	(
+		cd test &&
+		git remote set-head --auto origin &&
+		echo refs/remotes/origin/master >expect &&
+		git symbolic-ref refs/remotes/origin/HEAD >output &&
+		test_cmp expect output
 	)
 '
 
-cat >test/expect <<EOF
+cat >test/expect <<-\EOF
 error: Multiple remote HEAD branches. Please choose one explicitly with:
   git remote set-head two another
   git remote set-head two master
 EOF
 
 test_expect_success 'set-head --auto fails w/multiple HEADs' '
-	(cd test &&
-	 test_must_fail git remote set-head --auto two >output 2>&1 &&
-	test_i18ncmp expect output)
+	(
+		cd test &&
+		test_must_fail git remote set-head --auto two >output 2>&1 &&
+		test_i18ncmp expect output
+	)
 '
 
-cat >test/expect <<EOF
+cat >test/expect <<-\EOF
 refs/remotes/origin/side2
 EOF
 
 test_expect_success 'set-head explicit' '
-	(cd test &&
-	 git remote set-head origin side2 &&
-	 git symbolic-ref refs/remotes/origin/HEAD >output &&
-	 git remote set-head origin master &&
-	 test_cmp expect output)
+	(
+		cd test &&
+		git remote set-head origin side2 &&
+		git symbolic-ref refs/remotes/origin/HEAD >output &&
+		git remote set-head origin master &&
+		test_cmp expect output
+	)
 '
 
-cat > test/expect << EOF
+cat >test/expect <<-EOF
 Pruning origin
 URL: $(pwd)/one
  * [would prune] origin/side2
 EOF
 
 test_expect_success 'prune --dry-run' '
-	(cd one &&
-	 git branch -m side2 side) &&
-	(cd test &&
-	 git remote prune --dry-run origin > output &&
-	 git rev-parse refs/remotes/origin/side2 &&
-	 test_must_fail git rev-parse refs/remotes/origin/side &&
-	(cd ../one &&
-	 git branch -m side side2) &&
-	 test_i18ncmp expect output)
+	(
+		cd one &&
+		git branch -m side2 side) &&
+	(
+		cd test &&
+		git remote prune --dry-run origin >output &&
+		git rev-parse refs/remotes/origin/side2 &&
+		test_must_fail git rev-parse refs/remotes/origin/side &&
+	(
+		cd ../one &&
+		git branch -m side side2) &&
+		test_i18ncmp expect output
+	)
 '
 
 test_expect_success 'add --mirror && prune' '
-	(mkdir mirror &&
-	 cd mirror &&
-	 git init --bare &&
-	 git remote add --mirror -f origin ../one) &&
-	(cd one &&
-	 git branch -m side2 side) &&
-	(cd mirror &&
-	 git rev-parse --verify refs/heads/side2 &&
-	 test_must_fail git rev-parse --verify refs/heads/side &&
-	 git fetch origin &&
-	 git remote prune origin &&
-	 test_must_fail git rev-parse --verify refs/heads/side2 &&
-	 git rev-parse --verify refs/heads/side)
+	mkdir mirror &&
+	(
+		cd mirror &&
+		git init --bare &&
+		git remote add --mirror -f origin ../one
+	) &&
+	(
+		cd one &&
+		git branch -m side2 side
+	) &&
+	(
+		cd mirror &&
+		git rev-parse --verify refs/heads/side2 &&
+		test_must_fail git rev-parse --verify refs/heads/side &&
+		git fetch origin &&
+		git remote prune origin &&
+		test_must_fail git rev-parse --verify refs/heads/side2 &&
+		git rev-parse --verify refs/heads/side
+	)
 '
 
 test_expect_success 'add --mirror=fetch' '
 	mkdir mirror-fetch &&
 	git init mirror-fetch/parent &&
-	(cd mirror-fetch/parent &&
-	 test_commit one) &&
+	(
+		cd mirror-fetch/parent &&
+		test_commit one
+	) &&
 	git init --bare mirror-fetch/child &&
-	(cd mirror-fetch/child &&
-	 git remote add --mirror=fetch -f parent ../parent)
+	(
+		cd mirror-fetch/child &&
+		git remote add --mirror=fetch -f parent ../parent
+	)
 '
 
 test_expect_success 'fetch mirrors act as mirrors during fetch' '
-	(cd mirror-fetch/parent &&
-	 git branch new &&
-	 git branch -m master renamed
+	(
+		cd mirror-fetch/parent &&
+		git branch new &&
+		git branch -m master renamed
 	) &&
-	(cd mirror-fetch/child &&
-	 git fetch parent &&
-	 git rev-parse --verify refs/heads/new &&
-	 git rev-parse --verify refs/heads/renamed
+	(
+		cd mirror-fetch/child &&
+		git fetch parent &&
+		git rev-parse --verify refs/heads/new &&
+		git rev-parse --verify refs/heads/renamed
 	)
 '
 
 test_expect_success 'fetch mirrors can prune' '
-	(cd mirror-fetch/child &&
-	 git remote prune parent &&
-	 test_must_fail git rev-parse --verify refs/heads/master
+	(
+		cd mirror-fetch/child &&
+		git remote prune parent &&
+		test_must_fail git rev-parse --verify refs/heads/master
 	)
 '
 
 test_expect_success 'fetch mirrors do not act as mirrors during push' '
-	(cd mirror-fetch/parent &&
-	 git checkout HEAD^0
+	(
+		cd mirror-fetch/parent &&
+		git checkout HEAD^0
 	) &&
-	(cd mirror-fetch/child &&
-	 git branch -m renamed renamed2 &&
-	 git push parent :
+	(
+		cd mirror-fetch/child &&
+		git branch -m renamed renamed2 &&
+		git push parent :
 	) &&
-	(cd mirror-fetch/parent &&
-	 git rev-parse --verify renamed &&
-	 test_must_fail git rev-parse --verify refs/heads/renamed2
+	(
+		cd mirror-fetch/parent &&
+		git rev-parse --verify renamed &&
+		test_must_fail git rev-parse --verify refs/heads/renamed2
 	)
 '
 
 test_expect_success 'add fetch mirror with specific branches' '
 	git init --bare mirror-fetch/track &&
-	(cd mirror-fetch/track &&
-	 git remote add --mirror=fetch -t heads/new parent ../parent
+	(
+		cd mirror-fetch/track &&
+		git remote add --mirror=fetch -t heads/new parent ../parent
 	)
 '
 
 test_expect_success 'fetch mirror respects specific branches' '
-	(cd mirror-fetch/track &&
-	 git fetch parent &&
-	 git rev-parse --verify refs/heads/new &&
-	 test_must_fail git rev-parse --verify refs/heads/renamed
+	(
+		cd mirror-fetch/track &&
+		git fetch parent &&
+		git rev-parse --verify refs/heads/new &&
+		test_must_fail git rev-parse --verify refs/heads/renamed
 	)
 '
 
@@ -372,60 +408,72 @@ test_expect_success 'add --mirror=push' '
 	mkdir mirror-push &&
 	git init --bare mirror-push/public &&
 	git init mirror-push/private &&
-	(cd mirror-push/private &&
-	 test_commit one &&
-	 git remote add --mirror=push public ../public
+	(
+		cd mirror-push/private &&
+		test_commit one &&
+		git remote add --mirror=push public ../public
 	)
 '
 
 test_expect_success 'push mirrors act as mirrors during push' '
-	(cd mirror-push/private &&
-	 git branch new &&
-	 git branch -m master renamed &&
-	 git push public
+	(
+		cd mirror-push/private &&
+		git branch new &&
+		git branch -m master renamed &&
+		git push public
 	) &&
-	(cd mirror-push/private &&
-	 git rev-parse --verify refs/heads/new &&
-	 git rev-parse --verify refs/heads/renamed &&
-	 test_must_fail git rev-parse --verify refs/heads/master
+	(
+		cd mirror-push/private &&
+		git rev-parse --verify refs/heads/new &&
+		git rev-parse --verify refs/heads/renamed &&
+		test_must_fail git rev-parse --verify refs/heads/master
 	)
 '
 
 test_expect_success 'push mirrors do not act as mirrors during fetch' '
-	(cd mirror-push/public &&
-	 git branch -m renamed renamed2 &&
-	 git symbolic-ref HEAD refs/heads/renamed2
+	(
+		cd mirror-push/public &&
+		git branch -m renamed renamed2 &&
+		git symbolic-ref HEAD refs/heads/renamed2
 	) &&
-	(cd mirror-push/private &&
-	 git fetch public &&
-	 git rev-parse --verify refs/heads/renamed &&
-	 test_must_fail git rev-parse --verify refs/heads/renamed2
+	(
+		cd mirror-push/private &&
+		git fetch public &&
+		git rev-parse --verify refs/heads/renamed &&
+		test_must_fail git rev-parse --verify refs/heads/renamed2
 	)
 '
 
 test_expect_success 'push mirrors do not allow you to specify refs' '
 	git init mirror-push/track &&
-	(cd mirror-push/track &&
-	 test_must_fail git remote add --mirror=push -t new public ../public
+	(
+		cd mirror-push/track &&
+		test_must_fail git remote add --mirror=push -t new public ../public
 	)
 '
 
 test_expect_success 'add alt && prune' '
-	(mkdir alttst &&
-	 cd alttst &&
-	 git init &&
-	 git remote add -f origin ../one &&
-	 git config remote.alt.url ../one &&
-	 git config remote.alt.fetch "+refs/heads/*:refs/remotes/origin/*") &&
-	(cd one &&
-	 git branch -m side side2) &&
-	(cd alttst &&
-	 git rev-parse --verify refs/remotes/origin/side &&
-	 test_must_fail git rev-parse --verify refs/remotes/origin/side2 &&
-	 git fetch alt &&
-	 git remote prune alt &&
-	 test_must_fail git rev-parse --verify refs/remotes/origin/side &&
-	 git rev-parse --verify refs/remotes/origin/side2)
+	mkdir alttst &&
+	(
+		cd alttst &&
+		git init &&
+		git remote add -f origin ../one &&
+		git config remote.alt.url ../one &&
+		git config remote.alt.fetch "+refs/heads/*:refs/remotes/origin/*"
+	) &&
+	(
+		cd one &&
+		git branch -m side side2
+	) &&
+	(
+		cd alttst &&
+		git rev-parse --verify refs/remotes/origin/side &&
+		test_must_fail git rev-parse --verify refs/remotes/origin/side2 &&
+		git fetch alt &&
+		git remote prune alt &&
+		test_must_fail git rev-parse --verify refs/remotes/origin/side &&
+		git rev-parse --verify refs/remotes/origin/side2
+	)
 '
 
 cat >test/expect <<\EOF
@@ -433,20 +481,24 @@ some-tag
 EOF
 
 test_expect_success 'add with reachable tags (default)' '
-	(cd one &&
-	 >foobar &&
-	 git add foobar &&
-	 git commit -m "Foobar" &&
-	 git tag -a -m "Foobar tag" foobar-tag &&
-	 git reset --hard HEAD~1 &&
-	 git tag -a -m "Some tag" some-tag) &&
-	(mkdir add-tags &&
-	 cd add-tags &&
-	 git init &&
-	 git remote add -f origin ../one &&
-	 git tag -l some-tag >../test/output &&
-	 git tag -l foobar-tag >>../test/output &&
-	 test_must_fail git config remote.origin.tagopt) &&
+	(
+		cd one &&
+		: >foobar &&
+		git add foobar &&
+		git commit -m "Foobar" &&
+		git tag -a -m "Foobar tag" foobar-tag &&
+		git reset --hard HEAD~1 &&
+		git tag -a -m "Some tag" some-tag
+	) &&
+	mkdir add-tags &&
+	(
+		cd add-tags &&
+		git init &&
+		git remote add -f origin ../one &&
+		git tag -l some-tag >../test/output &&
+		git tag -l foobar-tag >>../test/output &&
+		test_must_fail git config remote.origin.tagopt
+	) &&
 	test_cmp test/expect test/output
 '
 
@@ -457,14 +509,16 @@ foobar-tag
 EOF
 
 test_expect_success 'add --tags' '
-	(rm -rf add-tags &&
-	 mkdir add-tags &&
-	 cd add-tags &&
-	 git init &&
-	 git remote add -f --tags origin ../one &&
-	 git tag -l some-tag >../test/output &&
-	 git tag -l foobar-tag >>../test/output &&
-	 git config remote.origin.tagopt >>../test/output) &&
+	rm -rf add-tags &&
+	(
+		mkdir add-tags &&
+		cd add-tags &&
+		git init &&
+		git remote add -f --tags origin ../one &&
+		git tag -l some-tag >../test/output &&
+		git tag -l foobar-tag >>../test/output &&
+		git config remote.origin.tagopt >>../test/output
+	) &&
 	test_cmp test/expect test/output
 '
 
@@ -473,25 +527,31 @@ cat >test/expect <<\EOF
 EOF
 
 test_expect_success 'add --no-tags' '
-	(rm -rf add-tags &&
-	 mkdir add-no-tags &&
-	 cd add-no-tags &&
-	 git init &&
-	 git remote add -f --no-tags origin ../one &&
-	 git tag -l some-tag >../test/output &&
-	 git tag -l foobar-tag >../test/output &&
-	 git config remote.origin.tagopt >>../test/output) &&
-	(cd one &&
-	 git tag -d some-tag foobar-tag) &&
+	rm -rf add-tags &&
+	(
+		mkdir add-no-tags &&
+		cd add-no-tags &&
+		git init &&
+		git remote add -f --no-tags origin ../one &&
+		git tag -l some-tag >../test/output &&
+		git tag -l foobar-tag >../test/output &&
+		git config remote.origin.tagopt >>../test/output
+	) &&
+	(
+		cd one &&
+		git tag -d some-tag foobar-tag
+	) &&
 	test_cmp test/expect test/output
 '
 
 test_expect_success 'reject --no-no-tags' '
-	(cd add-no-tags &&
-	 test_must_fail git remote add -f --no-no-tags neworigin ../one)
+	(
+		cd add-no-tags &&
+		test_must_fail git remote add -f --no-no-tags neworigin ../one
+	)
 '
 
-cat > one/expect << EOF
+cat >one/expect <<-\EOF
   apis/master
   apis/side
   drosophila/another
@@ -500,17 +560,17 @@ cat > one/expect << EOF
 EOF
 
 test_expect_success 'update' '
-
-	(cd one &&
-	 git remote add drosophila ../two &&
-	 git remote add apis ../mirror &&
-	 git remote update &&
-	 git branch -r > output &&
-	 test_cmp expect output)
-
+	(
+		cd one &&
+		git remote add drosophila ../two &&
+		git remote add apis ../mirror &&
+		git remote update &&
+		git branch -r >output &&
+		test_cmp expect output
+	)
 '
 
-cat > one/expect << EOF
+cat >one/expect <<-\EOF
   drosophila/another
   drosophila/master
   drosophila/side
@@ -521,34 +581,40 @@ cat > one/expect << EOF
 EOF
 
 test_expect_success 'update with arguments' '
-
-	(cd one &&
-	 for b in $(git branch -r)
-	 do
+	(
+		cd one &&
+		for b in $(git branch -r)
+		do
 		git branch -r -d $b || break
-	 done &&
-	 git remote add manduca ../mirror &&
-	 git remote add megaloprepus ../mirror &&
-	 git config remotes.phobaeticus "drosophila megaloprepus" &&
-	 git config remotes.titanus manduca &&
-	 git remote update phobaeticus titanus &&
-	 git branch -r > output &&
-	 test_cmp expect output)
-
+		done &&
+		git remote add manduca ../mirror &&
+		git remote add megaloprepus ../mirror &&
+		git config remotes.phobaeticus "drosophila megaloprepus" &&
+		git config remotes.titanus manduca &&
+		git remote update phobaeticus titanus &&
+		git branch -r >output &&
+		test_cmp expect output
+	)
 '
 
 test_expect_success 'update --prune' '
-
-	(cd one &&
-	 git branch -m side2 side3) &&
-	(cd test &&
-	 git remote update --prune &&
-	 (cd ../one && git branch -m side3 side2) &&
-	 git rev-parse refs/remotes/origin/side3 &&
-	 test_must_fail git rev-parse refs/remotes/origin/side2)
+	(
+		cd one &&
+		git branch -m side2 side3
+	) &&
+	(
+		cd test &&
+		git remote update --prune &&
+		(
+			cd ../one &&
+			git branch -m side3 side2
+		) &&
+		git rev-parse refs/remotes/origin/side3 &&
+		test_must_fail git rev-parse refs/remotes/origin/side2
+	)
 '
 
-cat > one/expect << EOF
+cat >one/expect <<-\EOF
   apis/master
   apis/side
   manduca/master
@@ -558,117 +624,115 @@ cat > one/expect << EOF
 EOF
 
 test_expect_success 'update default' '
-
-	(cd one &&
-	 for b in $(git branch -r)
-	 do
+	(
+		cd one &&
+		for b in $(git branch -r)
+		do
 		git branch -r -d $b || break
-	 done &&
-	 git config remote.drosophila.skipDefaultUpdate true &&
-	 git remote update default &&
-	 git branch -r > output &&
-	 test_cmp expect output)
-
+		done &&
+		git config remote.drosophila.skipDefaultUpdate true &&
+		git remote update default &&
+		git branch -r >output &&
+		test_cmp expect output
+	)
 '
 
-cat > one/expect << EOF
+cat >one/expect <<-\EOF
   drosophila/another
   drosophila/master
   drosophila/side
 EOF
 
 test_expect_success 'update default (overridden, with funny whitespace)' '
-
-	(cd one &&
-	 for b in $(git branch -r)
-	 do
+	(
+		cd one &&
+		for b in $(git branch -r)
+		do
 		git branch -r -d $b || break
-	 done &&
-	 git config remotes.default "$(printf "\t drosophila  \n")" &&
-	 git remote update default &&
-	 git branch -r > output &&
-	 test_cmp expect output)
-
+		done &&
+		git config remotes.default "$(printf "\t drosophila  \n")" &&
+		git remote update default &&
+		git branch -r >output &&
+		test_cmp expect output
+	)
 '
 
 test_expect_success 'update (with remotes.default defined)' '
-
-	(cd one &&
-	 for b in $(git branch -r)
-	 do
+	(
+		cd one &&
+		for b in $(git branch -r)
+		do
 		git branch -r -d $b || break
-	 done &&
-	 git config remotes.default "drosophila" &&
-	 git remote update &&
-	 git branch -r > output &&
-	 test_cmp expect output)
-
+		done &&
+		git config remotes.default "drosophila" &&
+		git remote update &&
+		git branch -r >output &&
+		test_cmp expect output
+	)
 '
 
 test_expect_success '"remote show" does not show symbolic refs' '
-
 	git clone one three &&
-	(cd three &&
-	 git remote show origin > output &&
-	 ! grep "^ *HEAD$" < output &&
-	 ! grep -i stale < output)
-
+	(
+		cd three &&
+		git remote show origin >output &&
+		! grep "^ *HEAD$" < output &&
+		! grep -i stale < output
+	)
 '
 
 test_expect_success 'reject adding remote with an invalid name' '
-
 	test_must_fail git remote add some:url desired-name
-
 '
 
 # The first three test if the tracking branches are properly renamed,
 # the last two ones check if the config is updated.
 
 test_expect_success 'rename a remote' '
-
 	git clone one four &&
-	(cd four &&
-	 git remote rename origin upstream &&
-	 rmdir .git/refs/remotes/origin &&
-	 test "$(git symbolic-ref refs/remotes/upstream/HEAD)" = "refs/remotes/upstream/master" &&
-	 test "$(git rev-parse upstream/master)" = "$(git rev-parse master)" &&
-	 test "$(git config remote.upstream.fetch)" = "+refs/heads/*:refs/remotes/upstream/*" &&
-	 test "$(git config branch.master.remote)" = "upstream")
-
+	(
+		cd four &&
+		git remote rename origin upstream &&
+		rmdir .git/refs/remotes/origin &&
+		test "$(git symbolic-ref refs/remotes/upstream/HEAD)" = "refs/remotes/upstream/master" &&
+		test "$(git rev-parse upstream/master)" = "$(git rev-parse master)" &&
+		test "$(git config remote.upstream.fetch)" = "+refs/heads/*:refs/remotes/upstream/*" &&
+		test "$(git config branch.master.remote)" = "upstream"
+	)
 '
 
 test_expect_success 'rename does not update a non-default fetch refspec' '
-
 	git clone one four.one &&
-	(cd four.one &&
-	 git config remote.origin.fetch +refs/heads/*:refs/heads/origin/* &&
-	 git remote rename origin upstream &&
-	 test "$(git config remote.upstream.fetch)" = "+refs/heads/*:refs/heads/origin/*" &&
-	 git rev-parse -q origin/master)
-
+	(
+		cd four.one &&
+		git config remote.origin.fetch +refs/heads/*:refs/heads/origin/* &&
+		git remote rename origin upstream &&
+		test "$(git config remote.upstream.fetch)" = "+refs/heads/*:refs/heads/origin/*" &&
+		git rev-parse -q origin/master
+	)
 '
 
 test_expect_success 'rename a remote with name part of fetch spec' '
-
 	git clone one four.two &&
-	(cd four.two &&
-	 git remote rename origin remote &&
-	 git remote rename remote upstream &&
-	 test "$(git config remote.upstream.fetch)" = "+refs/heads/*:refs/remotes/upstream/*")
-
+	(
+		cd four.two &&
+		git remote rename origin remote &&
+		git remote rename remote upstream &&
+		test "$(git config remote.upstream.fetch)" = "+refs/heads/*:refs/remotes/upstream/*"
+	)
 '
 
 test_expect_success 'rename a remote with name prefix of other remote' '
-
 	git clone one four.three &&
-	(cd four.three &&
-	 git remote add o git://example.com/repo.git &&
-	 git remote rename o upstream &&
-	 test "$(git rev-parse origin/master)" = "$(git rev-parse master)")
-
+	(
+		cd four.three &&
+		git remote add o git://example.com/repo.git &&
+		git remote rename o upstream &&
+		test "$(git rev-parse origin/master)" = "$(git rev-parse master)"
+	)
 '
 
-cat > remotes_origin << EOF
+cat >remotes_origin <<-EOF
 URL: $(pwd)/one
 Push: refs/heads/master:refs/heads/upstream
 Pull: refs/heads/master:refs/heads/origin
@@ -677,27 +741,31 @@ EOF
 test_expect_success 'migrate a remote from named file in $GIT_DIR/remotes' '
 	git clone one five &&
 	origin_url=$(pwd)/one &&
-	(cd five &&
-	 git remote remove origin &&
-	 mkdir -p .git/remotes &&
-	 cat ../remotes_origin > .git/remotes/origin &&
-	 git remote rename origin origin &&
-	 ! test -f .git/remotes/origin &&
-	 test "$(git config remote.origin.url)" = "$origin_url" &&
-	 test "$(git config remote.origin.push)" = "refs/heads/master:refs/heads/upstream" &&
-	 test "$(git config remote.origin.fetch)" = "refs/heads/master:refs/heads/origin")
+	(
+		cd five &&
+		git remote remove origin &&
+		mkdir -p .git/remotes &&
+		cat ../remotes_origin >.git/remotes/origin &&
+		git remote rename origin origin &&
+		! test -f .git/remotes/origin &&
+		test "$(git config remote.origin.url)" = "$origin_url" &&
+		test "$(git config remote.origin.push)" = "refs/heads/master:refs/heads/upstream" &&
+		test "$(git config remote.origin.fetch)" = "refs/heads/master:refs/heads/origin"
+	)
 '
 
 test_expect_success 'migrate a remote from named file in $GIT_DIR/branches' '
 	git clone one six &&
 	origin_url=$(pwd)/one &&
-	(cd six &&
-	 git remote rm origin &&
-	 echo "$origin_url" > .git/branches/origin &&
-	 git remote rename origin origin &&
-	 ! test -f .git/branches/origin &&
-	 test "$(git config remote.origin.url)" = "$origin_url" &&
-	 test "$(git config remote.origin.fetch)" = "refs/heads/master:refs/heads/origin")
+	(
+		cd six &&
+		git remote rm origin &&
+		echo "$origin_url" >.git/branches/origin &&
+		git remote rename origin origin &&
+		! test -f .git/branches/origin &&
+		test "$(git config remote.origin.url)" = "$origin_url" &&
+		test "$(git config remote.origin.fetch)" = "refs/heads/master:refs/heads/origin"
+	)
 '
 
 test_expect_success 'remote prune to cause a dangling symref' '
@@ -727,7 +795,6 @@ test_expect_success 'remote prune to cause a dangling symref' '
 '
 
 test_expect_success 'show empty remote' '
-
 	test_create_repo empty &&
 	git clone empty empty-clone &&
 	(
-- 
1.8.3.1.498.gacf2885

[PATCH v2 02/14] t/t5505-remote: test push-refspec in branches-file

[Ramkumar Ramachandra]

The test "migrate a remote from named file in $GIT_DIR/branches" reads
the branches-file, but only checks that the url and fetch-refspec are
set correctly.  Check that the push-refspec is also set correctly.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 t/t5505-remote.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index f679ded..74ddd77 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -764,7 +764,8 @@ test_expect_success 'migrate a remote from named file in $GIT_DIR/branches' '
 		git remote rename origin origin &&
 		! test -f .git/branches/origin &&
 		test "$(git config remote.origin.url)" = "$origin_url" &&
-		test "$(git config remote.origin.fetch)" = "refs/heads/master:refs/heads/origin"
+		test "$(git config remote.origin.fetch)" = "refs/heads/master:refs/heads/origin" &&
+		test "$(git config remote.origin.push)" = "HEAD:refs/heads/master"
 	)
 '
 
-- 
1.8.3.1.498.gacf2885

[PATCH v2 03/14] t/t5505-remote: use test_path_is_missing

[Ramkumar Ramachandra]

Replace instances of ! test -f with test_path_is_missing.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 t/t5505-remote.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 74ddd77..d688662 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -747,7 +747,7 @@ test_expect_success 'migrate a remote from named file in $GIT_DIR/remotes' '
 		mkdir -p .git/remotes &&
 		cat ../remotes_origin >.git/remotes/origin &&
 		git remote rename origin origin &&
-		! test -f .git/remotes/origin &&
+		test_path_is_missing .git/remotes/origin &&
 		test "$(git config remote.origin.url)" = "$origin_url" &&
 		test "$(git config remote.origin.push)" = "refs/heads/master:refs/heads/upstream" &&
 		test "$(git config remote.origin.fetch)" = "refs/heads/master:refs/heads/origin"
@@ -762,7 +762,7 @@ test_expect_success 'migrate a remote from named file in $GIT_DIR/branches' '
 		git remote rm origin &&
 		echo "$origin_url" >.git/branches/origin &&
 		git remote rename origin origin &&
-		! test -f .git/branches/origin &&
+		test_path_is_missing .git/branches/origin &&
 		test "$(git config remote.origin.url)" = "$origin_url" &&
 		test "$(git config remote.origin.fetch)" = "refs/heads/master:refs/heads/origin" &&
 		test "$(git config remote.origin.push)" = "HEAD:refs/heads/master"
-- 
1.8.3.1.498.gacf2885

[PATCH v2 04/14] t/t5505-remote: remove dependency on $origin_url

[Ramkumar Ramachandra]

In the tests "migrate a remote from named file in
$GIT_DIR/{remotes,branches}", we are only checking that a configuration
is migrated successfully; it has no correspondence with whether or not
those values do something sensible with other git
operations (fetch/push).  Therefore, there is no need to determine
$origin_url: just substitute it with the constant value "quux".

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 t/t5505-remote.sh | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index d688662..13199d1 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -733,14 +733,13 @@ test_expect_success 'rename a remote with name prefix of other remote' '
 '
 
 cat >remotes_origin <<-EOF
-URL: $(pwd)/one
+URL: quux
 Push: refs/heads/master:refs/heads/upstream
 Pull: refs/heads/master:refs/heads/origin
 EOF
 
 test_expect_success 'migrate a remote from named file in $GIT_DIR/remotes' '
 	git clone one five &&
-	origin_url=$(pwd)/one &&
 	(
 		cd five &&
 		git remote remove origin &&
@@ -748,7 +747,7 @@ test_expect_success 'migrate a remote from named file in $GIT_DIR/remotes' '
 		cat ../remotes_origin >.git/remotes/origin &&
 		git remote rename origin origin &&
 		test_path_is_missing .git/remotes/origin &&
-		test "$(git config remote.origin.url)" = "$origin_url" &&
+		test "$(git config remote.origin.url)" = "quux" &&
 		test "$(git config remote.origin.push)" = "refs/heads/master:refs/heads/upstream" &&
 		test "$(git config remote.origin.fetch)" = "refs/heads/master:refs/heads/origin"
 	)
@@ -756,14 +755,13 @@ test_expect_success 'migrate a remote from named file in $GIT_DIR/remotes' '
 
 test_expect_success 'migrate a remote from named file in $GIT_DIR/branches' '
 	git clone one six &&
-	origin_url=$(pwd)/one &&
 	(
 		cd six &&
 		git remote rm origin &&
-		echo "$origin_url" >.git/branches/origin &&
+		echo "quux" >.git/branches/origin &&
 		git remote rename origin origin &&
 		test_path_is_missing .git/branches/origin &&
-		test "$(git config remote.origin.url)" = "$origin_url" &&
+		test "$(git config remote.origin.url)" = "quux" &&
 		test "$(git config remote.origin.fetch)" = "refs/heads/master:refs/heads/origin" &&
 		test "$(git config remote.origin.push)" = "HEAD:refs/heads/master"
 	)
-- 
1.8.3.1.498.gacf2885

[PATCH v2 05/14] remote: remove dead code in read_branches_file()

[Ramkumar Ramachandra]

The first line of the function checks that the remote-name contains a
slash ('/'), and sets the "slash" variable accordingly.  The only caller
of read_branches_file() is remote_get_1(); the calling codepath is
guarded by valid_remote_nick(), which checks that the remote does not
contain a slash.  Therefore, the "slash" variable can never be set:
remove the dead code that assumes otherwise.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 remote.c | 26 +++-----------------------
 1 file changed, 3 insertions(+), 23 deletions(-)

diff --git a/remote.c b/remote.c
index e71f66d..128b210 100644
--- a/remote.c
+++ b/remote.c
@@ -276,10 +276,9 @@ static void read_remotes_file(struct remote *remote)
 
 static void read_branches_file(struct remote *remote)
 {
-	const char *slash = strchr(remote->name, '/');
 	char *frag;
 	struct strbuf branch = STRBUF_INIT;
-	int n = slash ? slash - remote->name : 1000;
+	int n = 1000;
 	FILE *f = fopen(git_path("branches/%.*s", n, remote->name), "r");
 	char *s, *p;
 	int len;
@@ -299,36 +298,17 @@ static void read_branches_file(struct remote *remote)
 	while (isspace(p[-1]))
 		*--p = 0;
 	len = p - s;
-	if (slash)
-		len += strlen(slash);
 	p = xmalloc(len + 1);
 	strcpy(p, s);
-	if (slash)
-		strcat(p, slash);
 
-	/*
-	 * With "slash", e.g. "git fetch jgarzik/netdev-2.6" when
-	 * reading from $GIT_DIR/branches/jgarzik fetches "HEAD" from
-	 * the partial URL obtained from the branches file plus
-	 * "/netdev-2.6" and does not store it in any tracking ref.
-	 * #branch specifier in the file is ignored.
-	 *
-	 * Otherwise, the branches file would have URL and optionally
-	 * #branch specified.  The "master" (or specified) branch is
-	 * fetched and stored in the local branch of the same name.
-	 */
 	frag = strchr(p, '#');
 	if (frag) {
 		*(frag++) = '\0';
 		strbuf_addf(&branch, "refs/heads/%s", frag);
 	} else
 		strbuf_addstr(&branch, "refs/heads/master");
-	if (!slash) {
-		strbuf_addf(&branch, ":refs/heads/%s", remote->name);
-	} else {
-		strbuf_reset(&branch);
-		strbuf_addstr(&branch, "HEAD:");
-	}
+
+	strbuf_addf(&branch, ":refs/heads/%s", remote->name);
 	add_url_alias(remote, p);
 	add_fetch_refspec(remote, strbuf_detach(&branch, NULL));
 	/*
-- 
1.8.3.1.498.gacf2885

[PATCH v2 06/14] t/t5505-remote: test url-with-# in branches-file

[Ramkumar Ramachandra]

Add one more test similar to "migrate a remote from named file in
$GIT_DIR/branches" to check that a url with a # can be used to specify
the branch name (as opposed to the constant "master").

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 t/t5505-remote.sh | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 13199d1..229a89c 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -767,27 +767,41 @@ test_expect_success 'migrate a remote from named file in $GIT_DIR/branches' '
 	)
 '
 
-test_expect_success 'remote prune to cause a dangling symref' '
+test_expect_success 'migrate a remote from named file in $GIT_DIR/branches (2)' '
 	git clone one seven &&
 	(
+		cd seven &&
+		git remote rm origin &&
+		echo "quux#foom" > .git/branches/origin &&
+		git remote rename origin origin &&
+		test_path_is_missing .git/branches/origin &&
+		test "$(git config remote.origin.url)" = "quux" &&
+		test "$(git config remote.origin.fetch)" = "refs/heads/foom:refs/heads/origin"
+		test "$(git config remote.origin.push)" = "HEAD:refs/heads/foom"
+	)
+'
+
+test_expect_success 'remote prune to cause a dangling symref' '
+	git clone one eight &&
+	(
 		cd one &&
 		git checkout side2 &&
 		git branch -D master
 	) &&
 	(
-		cd seven &&
+		cd eight &&
 		git remote prune origin
 	) >err 2>&1 &&
 	test_i18ngrep "has become dangling" err &&
 
 	: And the dangling symref will not cause other annoying errors &&
 	(
-		cd seven &&
+		cd eight &&
 		git branch -a
 	) 2>err &&
 	! grep "points nowhere" err &&
 	(
-		cd seven &&
+		cd eight &&
 		test_must_fail git branch nomore origin
 	) 2>err &&
 	grep "dangling symref" err
-- 
1.8.3.1.498.gacf2885

[PATCH v2 07/14] t/t5516-fetch-push: don't use branches-file

[Ramkumar Ramachandra]

Four tests exercising fetch and push functionality unnecessarily depend
on $GIT_DIR/branches files.  Modern Git does not encourage the use of
those files, and the parser remote.c:read_branches_file() is only
provided for backward compatibility with older repositories.  We already
have tests in t/t5505-remote to verify that the parser works: so,
substitute the $GIT_DIR/branches configuration with an equivalent
gitconfig-style configuration, using the results of those tests.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 t/t5516-fetch-push.sh | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 4691d51..6e9fa84 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -852,9 +852,11 @@ test_expect_success 'fetch with branches' '
 	mk_empty testrepo &&
 	git branch second $the_first_commit &&
 	git checkout second &&
-	echo ".." > testrepo/.git/branches/branch1 &&
 	(
 		cd testrepo &&
+		test_config remote.branch1.url ".."  &&
+		test_config remote.branch1.fetch "refs/heads/master:refs/heads/branch1"  &&
+		test_config remote.branch1.push "HEAD:refs/heads/master"  &&
 		git fetch branch1 &&
 		echo "$the_commit commit	refs/heads/branch1" >expect &&
 		git for-each-ref refs/heads >actual &&
@@ -865,9 +867,11 @@ test_expect_success 'fetch with branches' '
 
 test_expect_success 'fetch with branches containing #' '
 	mk_empty testrepo &&
-	echo "..#second" > testrepo/.git/branches/branch2 &&
 	(
 		cd testrepo &&
+		test_config remote.branch2.url ".."  &&
+		test_config remote.branch2.fetch "refs/heads/second:refs/heads/branch2"  &&
+		test_config remote.branch2.push "HEAD:refs/heads/second"  &&
 		git fetch branch2 &&
 		echo "$the_first_commit commit	refs/heads/branch2" >expect &&
 		git for-each-ref refs/heads >actual &&
@@ -879,7 +883,9 @@ test_expect_success 'fetch with branches containing #' '
 test_expect_success 'push with branches' '
 	mk_empty testrepo &&
 	git checkout second &&
-	echo "testrepo" > .git/branches/branch1 &&
+	test_config remote.branch1.url testrepo &&
+	test_config remote.branch1.fetch "refs/heads/master:refs/heads/branch1" &&
+	test_config remote.branch1.push "HEAD:refs/heads/master" &&
 	git push branch1 &&
 	(
 		cd testrepo &&
@@ -891,7 +897,9 @@ test_expect_success 'push with branches' '
 
 test_expect_success 'push with branches containing #' '
 	mk_empty testrepo &&
-	echo "testrepo#branch3" > .git/branches/branch2 &&
+	test_config remote.branch2.url testrepo &&
+	test_config remote.branch2.fetch "refs/heads/branch3:refs/heads/branch2" &&
+	test_config remote.branch2.push "HEAD:refs/heads/branch3" &&
 	git push branch2 &&
 	(
 		cd testrepo &&
-- 
1.8.3.1.498.gacf2885

[PATCH v2 08/14] ls-remote doc: fix example invocation on git.git

[Ramkumar Ramachandra]

Under the EXAMPLES section, there is one invocation on the git.git
repository that attempts to list the refs master, pu, and rc.  The ref
rc does not exist in today's repository, so remove it.  Among other
things, this example demonstrates that the "<refs>..." argument is
simply a filter; requesting a non-existent ref is not an error.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Documentation/git-ls-remote.txt | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 774de5e..cce4f93 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -70,7 +70,6 @@ EXAMPLES
 	$ git ls-remote http://www.kernel.org/pub/scm/git/git.git master pu rc
 	5fe978a5381f1fbad26a80e682ddd2a401966740	refs/heads/master
 	c781a84b5204fb294c9ccc79f8b3baceeb32c061	refs/heads/pu
-	b1d096f2926c4e37c9c0b6a7bf2119bedaa277cb	refs/heads/rc
 	$ echo http://www.kernel.org/pub/scm/git/git.git >.git/branches/public
 	$ git ls-remote --tags public v\*
 	d6602ec5194c87b0fc87103ca4d67251c76f233a	refs/tags/v0.99
-- 
1.8.3.1.498.gacf2885

[PATCH v2 09/14] ls-remote doc: rewrite <repository> paragraph

[Ramkumar Ramachandra]

Replace the <repository> paragraph containing specific references to
$GIT_DIR/branches and "." with a generic urls-or-remotes paragraph
referencing the relevant sections in the git-fetch(1) manpage.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Documentation/git-ls-remote.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index cce4f93..283fc0c 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -48,9 +48,9 @@ OPTIONS
 	exit without talking to the remote.
 
 <repository>::
-	Location of the repository.  The shorthand defined in
-	$GIT_DIR/branches/ can be used. Use "." (dot) to list references in
-	the local repository.
+	The "remote" repository to query.  This parameter can be
+	either a URL or the name of a remote (see the GIT URLS and
+	REMOTES sections of linkgit:git-fetch[1]).
 
 <refs>...::
 	When unspecified, all references, after filtering done
-- 
1.8.3.1.498.gacf2885

[PATCH v2 10/14] ls-remote doc: don't encourage use of branches-file

[Ramkumar Ramachandra]

One outdated example encourages the use of $GIT_DIR/branches files.
Replace it with an equivalent example using a remote.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Documentation/git-ls-remote.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 283fc0c..2e22915 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -70,8 +70,8 @@ EXAMPLES
 	$ git ls-remote http://www.kernel.org/pub/scm/git/git.git master pu rc
 	5fe978a5381f1fbad26a80e682ddd2a401966740	refs/heads/master
 	c781a84b5204fb294c9ccc79f8b3baceeb32c061	refs/heads/pu
-	$ echo http://www.kernel.org/pub/scm/git/git.git >.git/branches/public
-	$ git ls-remote --tags public v\*
+	$ git remote add korg http://www.kernel.org/pub/scm/git/git.git
+	$ git ls-remote --tags korg v\*
 	d6602ec5194c87b0fc87103ca4d67251c76f233a	refs/tags/v0.99
 	f25a265a342aed6041ab0cc484224d9ca54b6f41	refs/tags/v0.99.1
 	c5db5456ae3b0873fc659c19fafdde22313cc441	refs/tags/v0.99.2
-- 
1.8.3.1.498.gacf2885

[PATCH v2 11/14] t/t5505-remote: test multiple push/pull in remotes-file

[Ramkumar Ramachandra]

Extend the test "migrate a remote from named file in $GIT_DIR/remotes"
to test that multiple "Push:" and "Pull:" lines in the remotes-file
works as expected.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 t/t5505-remote.sh | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 229a89c..6a622fc 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -735,7 +735,9 @@ test_expect_success 'rename a remote with name prefix of other remote' '
 cat >remotes_origin <<-EOF
 URL: quux
 Push: refs/heads/master:refs/heads/upstream
+Push: refs/heads/master:refs/heads/upstream2
 Pull: refs/heads/master:refs/heads/origin
+Pull: refs/heads/master:refs/heads/origin2
 EOF
 
 test_expect_success 'migrate a remote from named file in $GIT_DIR/remotes' '
@@ -748,8 +750,18 @@ test_expect_success 'migrate a remote from named file in $GIT_DIR/remotes' '
 		git remote rename origin origin &&
 		test_path_is_missing .git/remotes/origin &&
 		test "$(git config remote.origin.url)" = "quux" &&
-		test "$(git config remote.origin.push)" = "refs/heads/master:refs/heads/upstream" &&
-		test "$(git config remote.origin.fetch)" = "refs/heads/master:refs/heads/origin"
+		cat >push_expected <<-\EOF &&
+		refs/heads/master:refs/heads/upstream
+		refs/heads/master:refs/heads/upstream2
+		EOF
+		cat >fetch_expected <<-\EOF &&
+		refs/heads/master:refs/heads/origin
+		refs/heads/master:refs/heads/origin2
+		EOF
+		git config --get-all remote.origin.push >push_actual &&
+		git config --get-all remote.origin.fetch >fetch_actual &&
+		test_cmp push_expected push_actual &&
+		test_cmp fetch_expected fetch_actual
 	)
 '
 
-- 
1.8.3.1.498.gacf2885

[PATCH v2 12/14] t/t5510-fetch: don't use remotes-file

[Ramkumar Ramachandra]

Replace it with the equivalent gitconfig configuration, using the
results of a test in t/t5505-remote.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 t/t5510-fetch.sh | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index fde6891..47aeac2 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -46,12 +46,9 @@ test_expect_success "clone and setup child repos" '
 		cd three &&
 		git config branch.master.remote two &&
 		git config branch.master.merge refs/heads/one &&
-		mkdir -p .git/remotes &&
-		{
-			echo "URL: ../two/.git/"
-			echo "Pull: refs/heads/master:refs/heads/two"
-			echo "Pull: refs/heads/one:refs/heads/one"
-		} >.git/remotes/two
+		git config remote.two.url "../two/.git/" &&
+		git config remote.two.fetch "refs/heads/master:refs/heads/two" &&
+		git config --add remote.two.fetch "refs/heads/one:refs/heads/one"
 	) &&
 	git clone . bundle &&
 	git clone . seven
-- 
1.8.3.1.498.gacf2885

[PATCH v2 13/14] t/t5515-fetch-merge-logic: don't use {branches,remotes}-file

[Ramkumar Ramachandra]

Replace it with the equivalent gitconfig configuration, using the
results of the corresponding tests in t/t5505-remote.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 t/t5515-fetch-merge-logic.sh | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/t/t5515-fetch-merge-logic.sh b/t/t5515-fetch-merge-logic.sh
index dbb927d..cde44e0 100755
--- a/t/t5515-fetch-merge-logic.sh
+++ b/t/t5515-fetch-merge-logic.sh
@@ -55,27 +55,25 @@ test_expect_success setup '
 	git config remote.config-glob.fetch refs/heads/*:refs/remotes/rem/* &&
 	remotes="$remotes config-glob" &&
 
-	mkdir -p .git/remotes &&
-	{
-		echo "URL: ../.git/"
-		echo "Pull: refs/heads/master:remotes/rem/master"
-		echo "Pull: refs/heads/one:remotes/rem/one"
-		echo "Pull: two:remotes/rem/two"
-		echo "Pull: refs/heads/three:remotes/rem/three"
-	} >.git/remotes/remote-explicit &&
+	git config remote.remote-explicit.url ../.git/ &&
+	git config remote.remote-explicit.fetch refs/heads/master:remotes/rem/master &&
+	git config --add remote.remote-explicit.fetch refs/heads/one:remotes/rem/one &&
+	git config --add remote.remote-explicit.fetch two:remotes/rem/two &&
+	git config --add remote.remote-explicit.fetch refs/heads/three:remotes/rem/three &&
 	remotes="$remotes remote-explicit" &&
 
-	{
-		echo "URL: ../.git/"
-		echo "Pull: refs/heads/*:refs/remotes/rem/*"
-	} >.git/remotes/remote-glob &&
+	git config remote.remote-glob.url ../.git/ &&
+	git config remote.remote-glob.fetch refs/heads/*:refs/remotes/rem/* &&
 	remotes="$remotes remote-glob" &&
 
-	mkdir -p .git/branches &&
-	echo "../.git" > .git/branches/branches-default &&
+	git config remote.branches-default.url ../.git/ &&
+	git config remote.branches-default.fetch refs/heads/master:refs/heads/branches-default &&
+	git config remote.branches-default.push HEAD:refs/heads/master &&
 	remotes="$remotes branches-default" &&
 
-	echo "../.git#one" > .git/branches/branches-one &&
+	git config remote.branches-one.url ../.git/ &&
+	git config remote.branches-one.fetch refs/heads/one:refs/heads/branches-one &&
+	git config remote.branches-one.push HEAD:refs/heads/one &&
 	remotes="$remotes branches-one" &&
 
 	for remote in $remotes ; do
-- 
1.8.3.1.498.gacf2885

[PATCH v2 14/14] remote: add comment about read_{branches,remotes}_file

[Ramkumar Ramachandra]

Explain that they are fringe features.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 remote.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/remote.c b/remote.c
index 128b210..f980928 100644
--- a/remote.c
+++ b/remote.c
@@ -227,6 +227,10 @@ static void add_instead_of(struct rewrite *rewrite, const char *instead_of)
 	rewrite->instead_of_nr++;
 }
 
+/*
+ * Fringe feature.  Modern Git encourages the use of gitconfig instead
+ * of remotes-file.
+ */
 static void read_remotes_file(struct remote *remote)
 {
 	FILE *f = fopen(git_path("remotes/%s", remote->name), "r");
@@ -274,6 +278,10 @@ static void read_remotes_file(struct remote *remote)
 	fclose(f);
 }
 
+/*
+ * Fringe feature.  Modern Git encourages the use of gitconfig instead
+ * of branches-file.
+ */
 static void read_branches_file(struct remote *remote)
 {
 	char *frag;
-- 
1.8.3.1.498.gacf2885

Re: [PATCH v2 05/14] remote: remove dead code in read_branches_file()

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> -	/*
> -	 * With "slash", e.g. "git fetch jgarzik/netdev-2.6" when
> -	 * reading from $GIT_DIR/branches/jgarzik fetches "HEAD" from
> -	 * the partial URL obtained from the branches file plus
> -	 * "/netdev-2.6" and does not store it in any tracking ref.
> -	 * #branch specifier in the file is ignored.
> -	 *
> -	 * Otherwise, the branches file would have URL and optionally
> -	 * #branch specified.  The "master" (or specified) branch is
> -	 * fetched and stored in the local branch of the same name.
> -	 */

Removal of the second paratraph is wrong, isn't it?

>  	frag = strchr(p, '#');
>  	if (frag) {
>  		*(frag++) = '\0';
>  		strbuf_addf(&branch, "refs/heads/%s", frag);
>  	} else
>  		strbuf_addstr(&branch, "refs/heads/master");
> -	if (!slash) {
> -		strbuf_addf(&branch, ":refs/heads/%s", remote->name);
> -	} else {
> -		strbuf_reset(&branch);
> -		strbuf_addstr(&branch, "HEAD:");
> -	}
> +
> +	strbuf_addf(&branch, ":refs/heads/%s", remote->name);
>  	add_url_alias(remote, p);
>  	add_fetch_refspec(remote, strbuf_detach(&branch, NULL));
>  	/*

Re: [PATCH v2 08/14] ls-remote doc: fix example invocation on git.git

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Under the EXAMPLES section, there is one invocation on the git.git
> repository that attempts to list the refs master, pu, and rc.  The ref
> rc does not exist in today's repository, so remove it.  Among other
> things, this example demonstrates that the "<refs>..." argument is
> simply a filter; requesting a non-existent ref is not an error.

These last two lines are better be in the documentation, no?

>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  Documentation/git-ls-remote.txt | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
> index 774de5e..cce4f93 100644
> --- a/Documentation/git-ls-remote.txt
> +++ b/Documentation/git-ls-remote.txt
> @@ -70,7 +70,6 @@ EXAMPLES
>  	$ git ls-remote http://www.kernel.org/pub/scm/git/git.git master pu rc
>  	5fe978a5381f1fbad26a80e682ddd2a401966740	refs/heads/master
>  	c781a84b5204fb294c9ccc79f8b3baceeb32c061	refs/heads/pu
> -	b1d096f2926c4e37c9c0b6a7bf2119bedaa277cb	refs/heads/rc
>  	$ echo http://www.kernel.org/pub/scm/git/git.git >.git/branches/public
>  	$ git ls-remote --tags public v\*
>  	d6602ec5194c87b0fc87103ca4d67251c76f233a	refs/tags/v0.99

Re: [PATCH v2 01/14] t/t5505-remote: modernize style

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Modernize the style of all tests throughout the file:
> - Remove spurious blank lines.
> - Indent the test body.
> - Make sure that all lines end with &&, to make it easier to spot breaks
>   in the chain.
> - When executing something in a subshell, put the parenthesis on
>   separate lines and indent the body.  Also make sure that the first
>   statement in the subshell is a 'cd'.
> - When redirecting output, replace the > output forms with >output.
> - Use the <<-\EOF and <<-EOF forms of heredoc, not <<EOF.  Also, don't
>   de-indent the heredoc body.
> - When creating an empty file, use : >output form over >output for
>   clarity.

Everything except the last one is good, I think.

For the last one, I prefer ">output" moderately over ": >output", as
both are equally clear to people who write shell scripts, but this
preference is not strong enough to make me change ": >output" that
is originally in the file to ">output" in a patch.  In the very old
days, there were some implementations of shells that mishandled
">output" but those days are long gone.

> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  t/t5505-remote.sh | 813 +++++++++++++++++++++++++++++-------------------------
>  1 file changed, 440 insertions(+), 373 deletions(-)
>
> -cat > test/expect << EOF
> ...
> +cat >test/expect <<-EOF
>  * remote origin
>    Fetch URL: $(pwd)/one
>    Push  URL: $(pwd)/one

This one is questionable; if it is not indented with HT, there is no
point using the -EOF form.

> -cat > test/expect << EOF
> ...
> +cat >test/expect <<-EOF

Likewise.

> @@ -219,152 +220,187 @@ cat > test/expect << EOF
>  EOF
>  
>  test_expect_success 'show -n' '
> -	(mv one one.unreachable &&
> -	 cd test &&
> ...
> +	mv one one.unreachable &&
> +	(
> +		cd test &&

This is more than an indentation change, but I think it is a good
one.  I saw some others that moved "mkdir" out of the subshell to
chdir into that directory, which are also good changes.

>  cat >test/expect <<\EOF

Funny that you did not touch this one.

> -cat > one/expect << EOF
> +cat >one/expect <<-\EOF
>    apis/master

But this again is an unnecessary use of <<-

This smells like a largely blind conversion done with a script,
which needs a human proof-reader other than the submitter;
everything else looked good.


I'll queue with this fix-up squashed in.


 t/t5505-remote.sh | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index f679ded..0e7dfa2 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -107,7 +107,7 @@ test_expect_success C_LOCALE_OUTPUT 'remove remote' '
 		check_remote_track origin master side &&
 		git for-each-ref "--format=%(refname)" refs/remotes |
 		sed -e "/^refs\/remotes\/origin\//d" >actual &&
-		: >expect &&
+		>expect &&
 		test_cmp expect actual
 	)
 '
@@ -139,7 +139,7 @@ test_expect_success 'remove remote protects local branches' '
 	)
 '
 
-cat >test/expect <<-EOF
+cat >test/expect <<EOF
 * remote origin
   Fetch URL: $(pwd)/one
   Push  URL: $(pwd)/one
@@ -202,7 +202,7 @@ test_expect_success 'show' '
 	)
 '
 
-cat >test/expect <<-EOF
+cat >test/expect <<EOF
 * remote origin
   Fetch URL: $(pwd)/one
   Push  URL: $(pwd)/one
@@ -262,7 +262,7 @@ test_expect_success 'set-head --auto' '
 	)
 '
 
-cat >test/expect <<-\EOF
+cat >test/expect <<\EOF
 error: Multiple remote HEAD branches. Please choose one explicitly with:
   git remote set-head two another
   git remote set-head two master
@@ -276,7 +276,7 @@ test_expect_success 'set-head --auto fails w/multiple HEADs' '
 	)
 '
 
-cat >test/expect <<-\EOF
+cat >test/expect <<\EOF
 refs/remotes/origin/side2
 EOF
 
@@ -290,7 +290,7 @@ test_expect_success 'set-head explicit' '
 	)
 '
 
-cat >test/expect <<-EOF
+cat >test/expect <<EOF
 Pruning origin
 URL: $(pwd)/one
  * [would prune] origin/side2
@@ -483,7 +483,7 @@ EOF
 test_expect_success 'add with reachable tags (default)' '
 	(
 		cd one &&
-		: >foobar &&
+		>foobar &&
 		git add foobar &&
 		git commit -m "Foobar" &&
 		git tag -a -m "Foobar tag" foobar-tag &&
@@ -551,7 +551,7 @@ test_expect_success 'reject --no-no-tags' '
 	)
 '
 
-cat >one/expect <<-\EOF
+cat >one/expect <<\EOF
   apis/master
   apis/side
   drosophila/another
@@ -570,7 +570,7 @@ test_expect_success 'update' '
 	)
 '
 
-cat >one/expect <<-\EOF
+cat >one/expect <<\EOF
   drosophila/another
   drosophila/master
   drosophila/side
@@ -637,7 +637,7 @@ test_expect_success 'update default' '
 	)
 '
 
-cat >one/expect <<-\EOF
+cat >one/expect <<\EOF
   drosophila/another
   drosophila/master
   drosophila/side
@@ -732,7 +732,7 @@ test_expect_success 'rename a remote with name prefix of other remote' '
 	)
 '
 
-cat >remotes_origin <<-EOF
+cat >remotes_origin <<EOF
 URL: $(pwd)/one
 Push: refs/heads/master:refs/heads/upstream
 Pull: refs/heads/master:refs/heads/origin

Re: [PATCH v2 01/14] t/t5505-remote: modernize style

[Ramkumar Ramachandra]

Junio C Hamano wrote:
> This smells like a largely blind conversion done with a script,

It is :)

> I'll queue with this fix-up squashed in.

Thanks; I really appreciate it.

Re: [PATCH v2 08/14] ls-remote doc: fix example invocation on git.git

[Ramkumar Ramachandra]

Junio C Hamano wrote:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>
>> Under the EXAMPLES section, there is one invocation on the git.git
>> repository that attempts to list the refs master, pu, and rc.  The ref
>> rc does not exist in today's repository, so remove it.  Among other
>> things, this example demonstrates that the "<refs>..." argument is
>> simply a filter; requesting a non-existent ref is not an error.
>
> These last two lines are better be in the documentation, no?

You're no happy with the existing documentation?

<refs>...::
	When unspecified, all references, after filtering done
	with --heads and --tags, are shown.  When <refs>... are
	specified, only references matching the given patterns
	are displayed.

Re: [PATCH v2 08/14] ls-remote doc: fix example invocation on git.git

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Junio C Hamano wrote:
>> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>>
>>> Under the EXAMPLES section, there is one invocation on the git.git
>>> repository that attempts to list the refs master, pu, and rc.  The ref
>>> rc does not exist in today's repository, so remove it.  Among other
>>> things, this example demonstrates that the "<refs>..." argument is
>>> simply a filter; requesting a non-existent ref is not an error.
>>
>> These last two lines are better be in the documentation, no?
>
> You're no happy with the existing documentation?

I am not so happy with the existing EXAMPLE.  Lazy people (myself
included when trying to figure out other people's software) jump
straight to the EXAMPLES section without reading the main body.

Especially for these people, it makes tons of difference if a sample
command is accompanied by an explanation of key points the example
tries to illustrate.  In this particular manual page, there are a
couple of commands and its sample output listed but nothing else.

Re: [PATCH v2 11/14] t/t5505-remote: test multiple push/pull in remotes-file

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Extend the test "migrate a remote from named file in $GIT_DIR/remotes"
> to test that multiple "Push:" and "Pull:" lines in the remotes-file
> works as expected.
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  t/t5505-remote.sh | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
> index 229a89c..6a622fc 100755
> --- a/t/t5505-remote.sh
> +++ b/t/t5505-remote.sh
> @@ -735,7 +735,9 @@ test_expect_success 'rename a remote with name prefix of other remote' '
>  cat >remotes_origin <<-EOF
>  URL: quux
>  Push: refs/heads/master:refs/heads/upstream
> +Push: refs/heads/master:refs/heads/upstream2
>  Pull: refs/heads/master:refs/heads/origin
> +Pull: refs/heads/master:refs/heads/origin2
>  EOF

I do not think we ever designed this to get 'master' pushed to
update two separate destination branches or their 'master' to update
our two separate tracking branches.

If you want to make things more realistic like you did in 08/14, I
do not see a point to change the tests that is already done for a
useful and realistic case by making it deliberately less realistic.

The same comment applies to the bogus quux URL from the patch 04/14.

I'll reject 04/14 and tweak this patch to use 'next' for the new ref
mappings, not duplicated 'master'.

Patches 07/14, 12/14, 13/14, and 14/14 are bad idea (these will not
be queued on tonight's pushout of 'pu'; neither 04/14 will be).  We
may not be encouraging, but that is very different from deprecating
the original mechanisms.  The tests that depend on them to work
should be kept.  Otherwise, we will never know when we break them
like we did at df93e33c accidenally.

If we want to have tests that exercise the equivalents spelled with
the modern in-config mechanism, they should be added as new tests,
not by replacing the existing ones.

Re: [PATCH v2 11/14] t/t5505-remote: test multiple push/pull in remotes-file

[Ramkumar Ramachandra]

Junio C Hamano wrote:
> I'll reject 04/14 and tweak this patch to use 'next' for the new ref
> mappings, not duplicated 'master'.

It's a matter of taste: I don't like "realistic" (albeit misleading)
values when I'm testing configuration variables, while you think they
make the tests more readable.  Fine.

> Patches 07/14, 12/14, 13/14, and 14/14 are bad idea (these will not
> be queued on tonight's pushout of 'pu'; neither 04/14 will be).  We
> may not be encouraging, but that is very different from deprecating
> the original mechanisms.  The tests that depend on them to work
> should be kept.  Otherwise, we will never know when we break them
> like we did at df93e33c accidenally.
>
> If we want to have tests that exercise the equivalents spelled with
> the modern in-config mechanism, they should be added as new tests,
> not by replacing the existing ones.

I disagree.  It is trivial to prove that the tests in t/remote will
break if this fringe feature breaks: I don't know where "we will never
know when we break them" is coming from.  Why should I know about this
fringe feature when I'm reading/writing tests for fetch-merge-logic?
And what is _advantage_ of depending on this fringe feature when
testing fetch-merge-logic?  More tests break?

But whatever.  I've already spent more time discussing (bikeshedding?)
this series than writing it.  I regret having written a stupid cleanup
series with no real consequence now; I'll be less likely to make the
same mistake again in the future.

Thanks.

Re: [PATCH v2 11/14] t/t5505-remote: test multiple push/pull in remotes-file

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> I disagree.  It is trivial to prove that the tests in t/remote will
> break if this fringe feature breaks: I don't know where "we will never
> know when we break them" is coming from.  

If you remove tests that configure the remotes via the .git/branches
and the .git/remotes mechanism to lose test coverage, and then
remove the call to read_branches_file() or read_remotes_file() from
remote_get_1(), how would you make sure these tests that originally
expected these configuration mechanisms to work notice the change in
behaviour?

> Why should I know about this
> fringe feature when I'm reading/writing tests for fetch-merge-logic?

That is exactly why I asked you at the very beginning:

    I say "minor" only because I think the cost of keeping these old
    mechanisms alive is very low (if it is a heavy burden on the
    maintenance, please tell me and how).

If we are not deprecating and keeping them alive, anybody who is
touching the codepath to deal with "struct remote" should be at
least aware of these mechanisms to keep them working.  If it is a
heavy burden to do so, then keeping them alive is not "minor" at
all.

I've been operating under the assumption, from your response to the
message in the original discussion in v1, that we are in agreement
that we are not deprecating or removing them.

And if we are deliberately keeping them alive, there is no need for
you to paint them "fringe".  You may not use it, and you can write
in your blog or e-mail or whategver that you do not use it, but that
label does not belong to our codebase and documentation as long as
we are keeping them as supported feature.

Of course, .git/branches, being a single liner format, will not get
extended to support richer features that in-config remotes will
learn, so you cannot build on existing tests that use the mechanism
to show new features.  You would need to introduce tests based on
the in-config remotes to demonstrate new features such as the
support for triangular workflow.

Because the .git/remotes/ files are designed to be extensible by
introducing new fields, theoretically it may be possible to extend
it to support new features, but we will not.  We will keep existing
users' existing use cases before new features are introduced in
recent Git alive, without adding anything new.  People will be
migrated out of these old mechanisms over time that way.

Re: [PATCH v2 13/14] t/t5515-fetch-merge-logic: don't use {branches,remotes}-file

[Ramkumar Ramachandra]

Santi Béjar wrote:
> If you think it's not worth testing the merge logic with remotes in
> {branches,remotes}-files you can consider just removing these remotes and
> the associated result files.

Thanks, but Junio has already decided to keep them and dropped these patches.