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

[Jonathan Nieder <jrnieder@gmail.com>]

Rob Hoelz wrote:

> --- a/remote.c
> +++ b/remote.c
> @@ -465,7 +465,11 @@ static void alias_all_urls(void)
>  		if (!remotes[i])
>  			continue;
>  		for (j = 0; j < remotes[i]->pushurl_nr; j++) {
> -			remotes[i]->pushurl[j] = alias_url(remotes[i]->pushurl[j], &rewrites);
> +			char *copy = xstrdup(remotes[i]->pushurl[j]);
> +			remotes[i]->pushurl[j] = alias_url(remotes[i]->pushurl[j], &rewrites_push);
> +			if (!strcmp(copy, remotes[i]->pushurl[j]))
> +				remotes[i]->pushurl[j] = alias_url(remotes[i]->pushurl[j], &rewrites);
> +			free(copy);

Interesting.

Suppose I configure

	[url "git://anongit.myserver.example.com/"]
		insteadOf = myserver.example.com:
	[url "myserver:"]
		pushInsteadOf = myserver.example.com:

The above code would make the insteadOf rule apply instead of
pushInsteadOf, even when pushing.  Perhaps something like the
following would work?

			const char *url = remotes[i]->pushurl[j];
			remotes[i]->pushurl[j] = alias_url(url, &rewrites_push);
			if (remotes[i]->pushurl[j] == url)
				/* No url.*.pushinsteadof configuration matched. */
				remotes[i]->pushurl[j] = alias_url(url, &rewrites);

> --- a/t/t5516-fetch-push.sh
> +++ b/t/t5516-fetch-push.sh
> @@ -244,6 +244,83 @@ 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 &&
> +	test_config "url.file://$TRASH/ro/.insteadOf" ro: &&
> +	test_config "url.file://$TRASH/rw/.pushInsteadOf" rw: &&
> +	test_config remote.r.url ro:wrong &&
> +	test_config remote.r.pushurl rw:testrepo &&
> +	git push r refs/heads/master:refs/remotes/origin/master &&
> +	(
> +		cd rw/testrepo &&
> +		echo "$the_commit commit	refs/remotes/origin/master" > expected &&
> +		git for-each-ref refs/remotes/origin > actual &&
> +		test_cmp expected actual
> +	)

Looks good.  The usual style in git tests is to include no space
after >redirection operators:

		git for-each-ref refs/remotes/origin >actual &&

Hope that helps,
Jonathan