Re: [PATCH] push: Alias pushurl from push rewrites
- From
Rob Hoelz <rob@hoelz.ro>
- Date
- Mar 18, 2013, 10:01 UTC
- Message-ID
- <5146E608.9070506@hoelz.ro>
- In-Reply-To
- <7vd2uxppk0.fsf@alter.siamese.dyndns.org>
On 3/18/13 12:35 AM, Junio C Hamano wrote:
Show 34 quoted lines
> Rob Hoelz <rob@hoelz.ro> writes: > >> git push currently doesn't consider pushInsteadOf when >> using pushurl; this tests and fixes that. >> >> If you use pushurl with an alias that has a pushInsteadOf configuration >> value, Git does not take advantage of it. For example: >> >> [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 > Incomplete sentence? For example [this is an example configuration] > and then what happens? Something like "with the sample > configuration, 'git push origin' should follow pushurl and then turn > it into X, but instead it ends up accessing Y". > > If there is no pushInsteadOf, does it still follow insteadOf? Is it > tested already? > > Wouldn't you want to cover all the combinations to negative cases > (i.e. making sure the codepath to support a new case does not affect > behaviour of the code outside the new case)? A remote with and > without pushurl (two combinations) and a pseudo URL scheme with and > without pushInsteadOf (again, two combinations) will give you four > cases. > > > Thanks.
Sorry; that's a good point. With the above configuration, the following fails:
$ git push origin master
With the following message:
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.
I haven't tried without pushInsteadOf; I will add a test for it later today. I assume that if pushInsteadOf isn't found, insteadOf should be used? I will also consider the other test cases you described.
Thanks again for the feedback!
Show 51 quoted lines
>
>> Signed-off-by: Rob Hoelz <rob@hoelz.ro>
>> ---
>> remote.c | 2 +-
>> t/t5516-fetch-push.sh | 20 ++++++++++++++++++++
>> 2 files changed, 21 insertions(+), 1 deletion(-)
>>
>> diff --git a/remote.c b/remote.c
>> index ca1f8f2..de7a915 100644
>> --- a/remote.c
>> +++ b/remote.c
>> @@ -465,7 +465,7 @@ 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);
>> + remotes[i]->pushurl[j] = alias_url(remotes[i]->pushurl[j], &rewrites_push);
>> }
>> add_pushurl_aliases = remotes[i]->pushurl_nr == 0;
>> for (j = 0; j < remotes[i]->url_nr; j++) {
>> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
>> index b5417cc..272e225 100755
>> --- a/t/t5516-fetch-push.sh
>> +++ b/t/t5516-fetch-push.sh
>> @@ -244,6 +244,26 @@ test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf
>> )
>> '
>>
>> +test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf does rewrite in this case)' '
>> + mk_empty &&
>> + 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: &&
>> + 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)
>> + )
>> +'
>> +
>> test_expect_success 'push with matching heads' '
>>
>> mk_test heads/master &&