On 3/18/13 12:35 AM, Junio C Hamano wrote:
> Rob Hoelz <r...@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 "g...@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 g...@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!
>
>> Signed-off-by: Rob Hoelz <r...@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 &&
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
