Re: [PATCH] push: Alias pushurl from push rewrites

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Mar 18, 2013, 23:10 UTC
Message-ID: <20130318231043.GD5062@elie.Belkin>
In-Reply-To: <20130318220224.3b23a381@hoelz.ro>

Hi,

Rob Hoelz wrote:

Show 21 quoted lines

> [url "git://github.com/"]
>     insteadOf = github:
> [url "git://github.com/myuser/"]
>     insteadOf = mygithub:
> [url "git@github.com:myuser/"]
>     pushInsteadOf = mygithub:
> [remote "origin"]
>     url     = github:organization/project
>     pushurl = mygithub:project
>
> With the above configuration, the following occurs:
>
> $ git push origin master
> fatal: remote error:
>   You can't push to git://github.com/myuser/project.git
>   Use git@github.com:myuser/project.git
>
> So you can see that pushurl is being followed (it's not attempting to
> push to git://github.com/organization/project), but insteadOf values are
> being used as opposed to pushInsteadOf values for expanding the pushurl
> alias.

At first glance it is not always obvious how overlapping settings like these should interact. Thanks for an instructive example that makes the right behavior obvious.

Test nits:

[...]

Show 15 quoted lines

> --- a/t/t5516-fetch-push.sh
> +++ b/t/t5516-fetch-push.sh
> @@ -244,6 +244,87 @@ test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf
>  	)
>  '
>  
> +test_expect_success 'push with pushInsteadOf and explicit pushurl (pushurl + pushInsteadOf does rewrite in this case)' '
> +	mk_empty &&
> +	rm -rf ro rw &&
> +	TRASH="$(pwd)/" &&
> +	mkdir ro &&
> +	mkdir rw &&
> +	git init --bare rw/testrepo &&
> +	git config "url.file://$TRASH/ro/.insteadOf" ro: &&
> +	git config "url.file://$TRASH/rw/.pushInsteadOf" rw: &&

The surrounding tests don't do this, but I wonder if it would make sense to use test_config instead of 'git config' here.

That way, the test's settings wouldn't affect other tests, and in particular if someone later decides to refactor the file by reordering tests, she could be confident that that would not break anything.

In most of the surrounding tests it does not matter because 'git config' is run in a subdirectory that is not reused later. Patches fixing the exceptions below.

Show 10 quoted lines

> +	git config remote.r.url ro:wrong &&
> +	git config remote.r.pushurl rw:testrepo &&
> +	git push r refs/heads/master:refs/remotes/origin/master &&
> +	(
> +		cd rw/testrepo &&
> +		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
> +		test "z$r" = "z$the_commit" &&
> +
> +		test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
> +	)

To produce more useful "./t5516-fetch-push.sh -v -i" output when the comparison fails:

	echo "$the_commit commit refs/remotes/origin/master" >expect &&
	(
		cd rw/testrepo &&
		git for-each-ref refs/remotes/origin
	) >actual &&
	test_cmp expect actual

Hope that helps,

Jonathan Nieder (3):
  push test: use test_config where appropriate
  push test: simplify check of push result
  push test: rely on &&-chaining instead of 'if bad; then echo Oops; fi'

 t/t5516-fetch-push.sh | 156 +++++++++++++++++++++-----------------------------
 1 file changed, 65 insertions(+), 91 deletions(-)