Re: [PATCH] push: Alias pushurl from push rewrites
Josh Triplett j...@joshtriplett.org writes: On Wed, Mar 27, 2013 at 05:48:45PM -0500, Rob Hoelz wrote: ... The test that checked that pushInsteadOf + pushurl shouldn't work as I expect was actually broken; I have removed it, updated the documentation, and sent a new patch to the list. There's an argument for either behavior as valid. My original patch specifically documented and tested for the opposite behavior, namely that pushurl overrides any pushInsteadOf, because I intended pushInsteadOf as a fallback if you don't have an explicit pushurl set. For only this bit. I think the test in question is this one from t5516: test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf should not rewrite)' ' mk_empty TRASH=$(pwd)/ git config url.trash2/.pushInsteadOf trash/ git config remote.r.url trash/wrong git config remote.r.pushurl $TRASH/testrepo git push r refs/heads/master:refs/remotes/origin/master ( cd 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) ) ' It defines a remote r, with URL trash/wrong (used for fetch) and pushURL $(pwd)/testrepo (used for push). There is a pushInsteadOf rule to rewrite anything that goes to trash/* to be pushed to trash2/* instead but that shouldn't be used to rewrite an explicit pushURL. And then the test pushes to r and checks if testrepo gets updated; in other words, it wants to make sure remote.r.pushURL defines the final destination, without pushInsteadOf getting in the way. But $(pwd)/testrepo does not match trash/* in the first place, so there is no chance for a pushInsteadOf to interfere; it looks to me that it is not testing what it wants to test. Perhaps we should do something like this? We tell it to push to testrepo/ with pushURL, and set up a pushInsteadOf to rewrite testrepo/ to trash2/, but because for this push it comes from an explicit pushURL, it still goes to testrepo/. As we do not have trash2/ repository, the test not just tests the push goes to testrepo/, but it also tests that it does not attempt to push to trash2/, checking both sides of the coin. t/t5516-fetch-push.sh | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index d3dc5df..b5ea32c 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -230,10 +230,9 @@ test_expect_success 'push with pushInsteadOf' ' test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf should not rewrite)' ' mk_empty - TRASH=$(pwd)/ - git config url.trash2/.pushInsteadOf trash/ + git config url.trash2/.pushInsteadOf testrepo/ git config remote.r.url trash/wrong - git config remote.r.pushurl $TRASH/testrepo + git config remote.r.pushurl testrepo/ git push r refs/heads/master:refs/remotes/origin/master ( cd testrepo -- 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
Re: [PATCH] push: Alias pushurl from push rewrites
On Thu, Mar 28, 2013 at 08:37:58AM -0700, Junio C Hamano wrote: Josh Triplett j...@joshtriplett.org writes: On Wed, Mar 27, 2013 at 05:48:45PM -0500, Rob Hoelz wrote: ... The test that checked that pushInsteadOf + pushurl shouldn't work as I expect was actually broken; I have removed it, updated the documentation, and sent a new patch to the list. There's an argument for either behavior as valid. My original patch specifically documented and tested for the opposite behavior, namely that pushurl overrides any pushInsteadOf, because I intended pushInsteadOf as a fallback if you don't have an explicit pushurl set. For only this bit. I think the test in question is this one from t5516: test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf should not rewrite)' ' mk_empty TRASH=$(pwd)/ git config url.trash2/.pushInsteadOf trash/ git config remote.r.url trash/wrong git config remote.r.pushurl $TRASH/testrepo git push r refs/heads/master:refs/remotes/origin/master ( cd 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) ) ' It defines a remote r, with URL trash/wrong (used for fetch) and pushURL $(pwd)/testrepo (used for push). There is a pushInsteadOf rule to rewrite anything that goes to trash/* to be pushed to trash2/* instead but that shouldn't be used to rewrite an explicit pushURL. And then the test pushes to r and checks if testrepo gets updated; in other words, it wants to make sure remote.r.pushURL defines the final destination, without pushInsteadOf getting in the way. But $(pwd)/testrepo does not match trash/* in the first place, so there is no chance for a pushInsteadOf to interfere; it looks to me that it is not testing what it wants to test. That test does actually test something important: it tests that the result of applying pushInsteadOf to url does *not* override pushurl. Not the same thing as testing that pushInsteadOf does or does not apply to pushurl. The relevant sentence of the git-config manpage (in the documentation for pushInsteadOf) says: If a remote has an explicit pushurl, git will ignore this setting for that remote. That really meant what I just said above: git will prefer an explicit pushurl over the pushInsteadOf rewrite of url. It says nothing about applying pushInsteadOf to rewrite pushurl. Perhaps we should do something like this? We tell it to push to testrepo/ with pushURL, and set up a pushInsteadOf to rewrite testrepo/ to trash2/, but because for this push it comes from an explicit pushURL, it still goes to testrepo/. As we do not have trash2/ repository, the test not just tests the push goes to testrepo/, but it also tests that it does not attempt to push to trash2/, checking both sides of the coin. Sensible test, assuming you want to enforce that behavior. I don't strongly care either way about that one, since it only applies if your pushInsteadOf rewrites could apply to your pushurl, and I only ever use pushInsteadOf to rewrite unpushable repos to pushable ones. However... t/t5516-fetch-push.sh | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index d3dc5df..b5ea32c 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -230,10 +230,9 @@ test_expect_success 'push with pushInsteadOf' ' test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf should not rewrite)' ' mk_empty - TRASH=$(pwd)/ - git config url.trash2/.pushInsteadOf trash/ + git config url.trash2/.pushInsteadOf testrepo/ git config remote.r.url trash/wrong - git config remote.r.pushurl $TRASH/testrepo + git config remote.r.pushurl testrepo/ git push r refs/heads/master:refs/remotes/origin/master ( cd testrepo ...the test you describe should appear in *addition* to this test, not replacing it, because as described above this test does test one critical bit of behavior, namely prefering pushurl over the pushInsteadOf rewrite of url. - Josh Triplett -- 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
Re: [PATCH] push: Alias pushurl from push rewrites
Josh Triplett j...@joshtriplett.org writes: OK, I take it back. I *can* imagine configurations that this change would break, since it does change intentional and documented behavior, but I don't have any such configuration. The only such configuration I can imagine involves directly counting on the non-rewriting of pushUrl, by using pushInsteadOf to rewrite urls and then sometimes using pushUrl to override that and point back at the un-rewritten URL. And while supported, that does seem *odd*. Objection withdrawn; if nobody can come up with a sensible configuration that relies on the documented behavior, I don't particularly care if it changes. I actually do. Given the popularity of the system, people involved in this thread cannot imagine a case that existing people may get hurt is very different from this is not a regression. After merging this change when people start complaining, you and Rob can hide and ignore them, but we collectively as the Git project have to have a way to help them when it happens. -- 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
Re: [PATCH] push: Alias pushurl from push rewrites
On Thu, Mar 28, 2013 at 09:10:59AM -0700, Junio C Hamano wrote: Josh Triplett j...@joshtriplett.org writes: OK, I take it back. I *can* imagine configurations that this change would break, since it does change intentional and documented behavior, but I don't have any such configuration. The only such configuration I can imagine involves directly counting on the non-rewriting of pushUrl, by using pushInsteadOf to rewrite urls and then sometimes using pushUrl to override that and point back at the un-rewritten URL. And while supported, that does seem *odd*. Objection withdrawn; if nobody can come up with a sensible configuration that relies on the documented behavior, I don't particularly care if it changes. I actually do. Given the popularity of the system, people involved in this thread cannot imagine a case that existing people may get hurt is very different from this is not a regression. After merging this change when people start complaining, you and Rob can hide and ignore them, but we collectively as the Git project have to have a way to help them when it happens. I entirely agree that it represents a regression from documented behavior; I just mean that it no longer matches a specific use case I had in mind with the original change. I agree that we should hesitate to change that documented behavior. - Josh Triplett -- 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
Re: [PATCH] push: Alias pushurl from push rewrites
Josh Triplett j...@joshtriplett.org writes: (on url.$base.pushInsteadOf) If a remote has an explicit pushurl, git will ignore this setting for that remote. That really meant what I just said above: git will prefer an explicit pushurl over the pushInsteadOf rewrite of url. Very correct. It says nothing about applying pushInsteadOf to rewrite pushurl. Incorrect, I think. If you have pushURL, pushInsteadOf is *ignored*. Of course, if you have both URL and pushURL, the ignored pushInsteadOf will not apply to _anything_. It will not apply to URL, and it will certainly not apply to pushURL. You are correct to point out that with the test we would want to make sure that for a remote with pushURL and URL, a push goes - to pushURL; - not to URL; - not to insteadOf(URL); - not to pushInsteadOf(URL); - not to insteadOf(pushURL); and - not to pushInsteadOf(pushURL). I do not think it is worth checking all of them, but I agree we should make sure it does not go to pushInsteadOf(URL) which you originally meant to check, and we should also make sure it does not go to pushInsteadOf(pushURL). test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf should not rewrite)' ' mk_empty -TRASH=$(pwd)/ -git config url.trash2/.pushInsteadOf trash/ +git config url.trash2/.pushInsteadOf testrepo/ Adding git config url.trash3/.pusnInsteadOf trash/wrong here should be sufficient for that, no? If we mistakenly used URL (i.e. trash/wrong) the push would fail. If we mistakenly used pushInsteadOf(URL), that is rewritten to trash3/ and again the push would fail. pushInsteadOf(pushURL) would go to trash2/ and that would also fail. We aren't checking insteadOf(URL) and insteadOf(pushURL) but everything else is checked, I think, so we can do without replacing anything. We can just extend it, no? git config remote.r.url trash/wrong -git config remote.r.pushurl $TRASH/testrepo +git config remote.r.pushurl testrepo/ git push r refs/heads/master:refs/remotes/origin/master ( cd testrepo ...the test you describe should appear in *addition* to this test, not replacing it, because as described above this test does test one critical bit of behavior, namely prefering pushurl over the pushInsteadOf rewrite of url. - Josh Triplett -- 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
Re: [PATCH] push: Alias pushurl from push rewrites
On Thu, Mar 28, 2013 at 11:50:08AM -0700, Junio C Hamano wrote: Josh Triplett j...@joshtriplett.org writes: (on url.$base.pushInsteadOf) If a remote has an explicit pushurl, git will ignore this setting for that remote. That really meant what I just said above: git will prefer an explicit pushurl over the pushInsteadOf rewrite of url. Very correct. It says nothing about applying pushInsteadOf to rewrite pushurl. Incorrect, I think. If you have pushURL, pushInsteadOf is *ignored*. Of course, if you have both URL and pushURL, the ignored pushInsteadOf will not apply to _anything_. It will not apply to URL, and it will certainly not apply to pushURL. Debatable whether the documentation sentence above really says that; it certainly doesn't say it very clearly if so. But that does match the actual behavior, making the proposed change a regression from the actual behavior, whether the documentation clearly guarantees that behavior or not. You are correct to point out that with the test we would want to make sure that for a remote with pushURL and URL, a push goes - to pushURL; - not to URL; - not to insteadOf(URL); - not to pushInsteadOf(URL); - not to insteadOf(pushURL); and - not to pushInsteadOf(pushURL). I do not think it is worth checking all of them, but I agree we should make sure it does not go to pushInsteadOf(URL) which you originally meant to check, and we should also make sure it does not go to pushInsteadOf(pushURL). Agreed. Related to this, as a path forward, I do think it makes sense to have a setting usable as an insteadOf that only applies to pushurl, even though pushInsteadOf won't end up serving that purpose. That way, pushInsteadOf covers the map read-only repo url to pushable repo url case, and insteadOfPushOnly covers the construct a magic prefix that maps to different urls when used in url or pushurl case. test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf should not rewrite)' ' mk_empty - TRASH=$(pwd)/ - git config url.trash2/.pushInsteadOf trash/ + git config url.trash2/.pushInsteadOf testrepo/ Adding git config url.trash3/.pusnInsteadOf trash/wrong here should be sufficient for that, no? If we mistakenly used URL (i.e. trash/wrong) the push would fail. If we mistakenly used pushInsteadOf(URL), that is rewritten to trash3/ and again the push would fail. pushInsteadOf(pushURL) would go to trash2/ and that would also fail. We aren't checking insteadOf(URL) and insteadOf(pushURL) but everything else is checked, I think, so we can do without replacing anything. We can just extend it, no? That sounds sensible. - Josh Triplett -- 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
Re: [PATCH] push: Alias pushurl from push rewrites
Josh Triplett wrote: Related to this, as a path forward, I do think it makes sense to have a setting usable as an insteadOf that only applies to pushurl, even though pushInsteadOf won't end up serving that purpose. That way, pushInsteadOf covers the map read-only repo url to pushable repo url case, and insteadOfPushOnly covers the construct a magic prefix that maps to different urls when used in url or pushurl case. I hope not. That would be making configuration even more complicated. I hope that we can fix the documentation, tests, and change description in the commit message enough to make Rob's patch a no-brainer. If that's not possible, I think the current state is livable, just confusing. I was happy to see Rob's patch because it brings git's behavior closer to following the principle of least surprise. I am not actually that excited by the use case, except the avoiding surprise part of it. Hope that helps, Jonathan -- 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
Re: [PATCH] push: Alias pushurl from push rewrites
On Thu, 28 Mar 2013 12:25:07 -0700 Jonathan Nieder jrnie...@gmail.com wrote: Josh Triplett wrote: Related to this, as a path forward, I do think it makes sense to have a setting usable as an insteadOf that only applies to pushurl, even though pushInsteadOf won't end up serving that purpose. That way, pushInsteadOf covers the map read-only repo url to pushable repo url case, and insteadOfPushOnly covers the construct a magic prefix that maps to different urls when used in url or pushurl case. I hope not. That would be making configuration even more complicated. I hope that we can fix the documentation, tests, and change description in the commit message enough to make Rob's patch a no-brainer. If that's not possible, I think the current state is livable, just confusing. I was happy to see Rob's patch because it brings git's behavior closer to following the principle of least surprise. I am not actually that excited by the use case, except the avoiding surprise part of it. Hope that helps, Jonathan Thanks for all of the input, everyone. I personally agree with Jonathon's notion of principle of least surprise, as I was quite surprised when my configuration with pushInsteadOf + pushurl did not work. However, I also understand Junio's arguments about going back on documented behavior, as well as whether or not it's a good idea to have this triangular remote set up. Honestly, if my workflow here is stupid and not Git-like and someone has a better suggestion, I would happy to let my patch go. Using two remotes is an option, but to me, using this triangular setup is just easier. The only way I see this breaking an existing configuration is if a user has something like url.u...@server.com.pushInsteadOf = myserver:, and pushurl = myserver:repo/. If this behavior weren't documented, I would say that such a configuration works because it relies on a bug, and should use ssh://myserver:repo/ instead. I personally feel that the fact that insteadOf + url works and pushInsteadOf + pushurl doesn't is inconsistent and confusing. However, I am one user of many, and this is my first exposure to Git from a project contributor point of view. Therefore, I submit to the wisdom of more seasoned Git developers such as yourselves. =) -Rob -- 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
Re: [PATCH] push: Alias pushurl from push rewrites
Rob Hoelz r...@hoelz.ro writes: Honestly, if my workflow here is stupid and not Git-like and someone has a better suggestion, I would happy to let my patch go. Using two remotes is an option, but to me, using this triangular setup is just easier. I think you are conflating two unrelated things. Pulling from one repository to integrate others' work with yours (either by merging others' work to yours, or rebasing your work on others'), and pushing the result of your work to another repository to publish, i.e. triangular workflow, is no less Git-like than pulling from and pushing to the same repository. Both are valid workflows, and Git supports them. What is not correct in your set-up is that a single remote with URL and pushURL (or rewritten URL derived from them via pushInsteadOf and insteadOf) that point at two different repositories is *not* the way to express that triangular configuration. You name two remotes, pull from one and push to the other. If you look at Ram's triangular workflows series, cf. http://thread.gmane.org/gmane.comp.version-control.git/219387 you can see that a progress is being made to make the two remotes configuration easier to use. The discussion on the earliest iteration of the patch series, cf. http://thread.gmane.org/gmane.comp.version-control.git/215702 shows that even I initially made the same pointing two different repositories with URL and pushURL should be sufficient mistake, which was corrected by others. The primary issue is remote tracking branches are designed to keep track of the state of the branches at the named remote---for this reason alone, you must not name a logically different repository with URL and pushURL for a single remote. So that is one thing. tl;dr: Triangular workflow is valid. A single remote with URL and pushURL to point at the two remote repositories is not a valid way to express that workflow. The other thing is if it is worth risking to break the backward compatibility and hurting existing users in order to remove the strange To an explicit pushURL, insteadOf rewriting will not apply exception. The reason I didn't bring up the possible breakage of documented behaviour in the earlier review of this series is exactly because that special case was unintuitive, so you do not have to argue it is strange, unintuitive, and would not be a way we would have designed the system if we knew better. I already agree to that point, and I think others do, too. There is a gap between We would design it differently if we were building it now with what we know and We should change it and make it ideal and the gap is called existing users. These two are unrelated and independent. I suspect that Ram's triangular series, when it matures, will help your pull from there, push to another workflow in a different way. You will just define two remotes for these two places, and you may no longer need pushInsteadOf is not ignored when you have pushURL to solve your immediate issue. But removing the pushInsteadOf is ignored when explicit pushURL exists may still be a worthwhile thing to do, even if you do not need it. -- 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
Re: [PATCH] push: Alias pushurl from push rewrites
On Wed, 20 Mar 2013 07:35:58 -0700 Junio C Hamano gits...@pobox.com wrote: Rob Hoelz r...@hoelz.ro writes: On 3/19/13 7:08 PM, Junio C Hamano wrote: Jonathan Nieder jrnie...@gmail.com writes: Junio C Hamano wrote: Jonathan Nieder jrnie...@gmail.com writes: Test nits: ... 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' Are these supposed to be follow-up patches? Preparatory steps that Rob can reroll on top? Something else? Preparatory steps. OK, thanks. I'll queue these first then. Should I apply these to my patch to move things along? What's the next step for me? You would fetch from nearby git.git mirror, find the tip of Janathan's series and redo your patch on top. Perhaps the session would go like this: $ git fetch git://git.kernel.org/pub/scm/git/git.git/ pu $ git log --oneline --first-parent ..FETCH_HEAD | grep jn/push-tests 83a072a Merge branch 'jn/push-tests' into pu $ git checkout -b rh/push-pushurl-pushinsteadof 83a072a ... redo the work, perhaps with combinations of: $ git cherry-pick -n $your_original_branch $ edit t/t5516-fetch-push.sh ... and then $ make test $ git commit Ok, changes applied. New patch coming. -- 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
Re: [PATCH] push: Alias pushurl from push rewrites
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 commitrefs/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
--
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
Re: [PATCH] push: Alias pushurl from push rewrites
Jonathan Nieder jrnie...@gmail.com writes: Sorry, typo. The configuration in the example above should have been [url git://anongit.myserver.example.com/] insteadOf = myserver.example.com: [url myserver.example.com:] pushInsteadOf = myserver.example.com: In other words, suppose I set url.*.insteadof to point to a faster address for fetching alongside url.*.pushinsteadof requesting that the original address should still be used for pushing. I didn't know we were even shooting for supporting the identity mapping: url.X.pushinsteadof=X but that would certainly be nice. By the way, it seems that the original commit 1c2eafb89bca (Add url.base.pushInsteadOf: URL rewriting for push only, 2009-09-07) wanted to explicitly avoid use of pushInsteadOf aliasing for a pushURL and it is also documented in config.txt from day one. I think the intent is You have a normal URL, and a way to override it explicitly with pushURL, or a way to rewrite it via the aliasing the normal URL with pushInsteadOf. Either one or the other, but not both, as having many levels of indirection would be confusing. Which I can understand and sympathise. In anay case, the change proposed in this thread seems to change that, so the documentation would need to be updated. Also the tests the original commit adds explicitly checks that pushInsteadOf is ignored, which may have to be updated (unless that test is already broken). -- 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
Re: [PATCH] push: Alias pushurl from push rewrites
On Wed, 27 Mar 2013 15:07:18 -0700 Junio C Hamano gits...@pobox.com wrote: Jonathan Nieder jrnie...@gmail.com writes: Sorry, typo. The configuration in the example above should have been [url git://anongit.myserver.example.com/] insteadOf = myserver.example.com: [url myserver.example.com:] pushInsteadOf = myserver.example.com: In other words, suppose I set url.*.insteadof to point to a faster address for fetching alongside url.*.pushinsteadof requesting that the original address should still be used for pushing. I didn't know we were even shooting for supporting the identity mapping: url.X.pushinsteadof=X but that would certainly be nice. By the way, it seems that the original commit 1c2eafb89bca (Add url.base.pushInsteadOf: URL rewriting for push only, 2009-09-07) wanted to explicitly avoid use of pushInsteadOf aliasing for a pushURL and it is also documented in config.txt from day one. I think the intent is You have a normal URL, and a way to override it explicitly with pushURL, or a way to rewrite it via the aliasing the normal URL with pushInsteadOf. Either one or the other, but not both, as having many levels of indirection would be confusing. Which I can understand and sympathise. In anay case, the change proposed in this thread seems to change that, so the documentation would need to be updated. Also the tests the original commit adds explicitly checks that pushInsteadOf is ignored, which may have to be updated (unless that test is already broken). My use case is that I use Github for my personal development. I have a prefix for my personal repos (hoelzro: - git://github.com/hoelzro for fetch, g...@github.com:hoelzro/ for push) and one for all other Git repos (github: - git://github.com/) I have a few projects where I work in a fork, but I want to fetch updates from the original project. So my url for the origin remote is github:org/project, but my pushurl is hoelzro:project. This behavior in Git currently doesn't allow me to work that way. I used to work with two remotes; origin for my repo and base for the official one, but I've found that I prefer this other way. The test that checked that pushInsteadOf + pushurl shouldn't work as I expect was actually broken; I have removed it, updated the documentation, and sent a new patch to the list. -Rob -- 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
Re: [PATCH] push: Alias pushurl from push rewrites
On Wed, 27 Mar 2013 15:47:35 -0700 Jonathan Nieder jrnie...@gmail.com wrote: Hi, Rob Hoelz wrote: On Wed, 27 Mar 2013 11:23:45 -0700 Jonathan Nieder jrnie...@gmail.com wrote: 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? Are you sure? The message you are replying to is nonsense, due to a typo while editing. Did you see my followup? Sorry for the confusion, Jonathan My mistake; I had not seen it! I thought you may have found a bug in my implementation, so I wanted to double check. =) -Rob -- 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
Re: [PATCH] push: Alias pushurl from push rewrites
Rob Hoelz wrote: My mistake; I had not seen it! I thought you may have found a bug in my implementation, so I wanted to double check. =) Well, I had found an unfortunate consequence of the implementation that uses an unnecessary copy. :) Will follow up to the updated patch. -- 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
Re: [PATCH] push: Alias pushurl from push rewrites
On Wed, 27 Mar 2013 15:56:56 -0700 Jonathan Nieder jrnie...@gmail.com wrote: Rob Hoelz wrote: My mistake; I had not seen it! I thought you may have found a bug in my implementation, so I wanted to double check. =) Well, I had found an unfortunate consequence of the implementation that uses an unnecessary copy. :) Will follow up to the updated patch. I actually wanted to talk about the copy thing. I realize that this could have been avoided by simply saving a pointer to the old string and performing a comparison, but I figured if the implementation for alias_url were changed in the future to use realloc or something, it could potentially return the original char * with its contents altered. So, by copying the string, we can avoid strange bugs in the future. -Rob -- 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
Re: [PATCH] push: Alias pushurl from push rewrites
On Wed, Mar 27, 2013 at 05:48:45PM -0500, Rob Hoelz wrote: On Wed, 27 Mar 2013 15:07:18 -0700 Junio C Hamano gits...@pobox.com wrote: Jonathan Nieder jrnie...@gmail.com writes: Sorry, typo. The configuration in the example above should have been [url git://anongit.myserver.example.com/] insteadOf = myserver.example.com: [url myserver.example.com:] pushInsteadOf = myserver.example.com: In other words, suppose I set url.*.insteadof to point to a faster address for fetching alongside url.*.pushinsteadof requesting that the original address should still be used for pushing. I didn't know we were even shooting for supporting the identity mapping: url.X.pushinsteadof=X but that would certainly be nice. By the way, it seems that the original commit 1c2eafb89bca (Add url.base.pushInsteadOf: URL rewriting for push only, 2009-09-07) wanted to explicitly avoid use of pushInsteadOf aliasing for a pushURL and it is also documented in config.txt from day one. I think the intent is You have a normal URL, and a way to override it explicitly with pushURL, or a way to rewrite it via the aliasing the normal URL with pushInsteadOf. Either one or the other, but not both, as having many levels of indirection would be confusing. Which I can understand and sympathise. In anay case, the change proposed in this thread seems to change that, so the documentation would need to be updated. Also the tests the original commit adds explicitly checks that pushInsteadOf is ignored, which may have to be updated (unless that test is already broken). My use case is that I use Github for my personal development. I have a prefix for my personal repos (hoelzro: - git://github.com/hoelzro for fetch, g...@github.com:hoelzro/ for push) and one for all other Git repos (github: - git://github.com/) I have a few projects where I work in a fork, but I want to fetch updates from the original project. So my url for the origin remote is github:org/project, but my pushurl is hoelzro:project. This behavior in Git currently doesn't allow me to work that way. I used to work with two remotes; origin for my repo and base for the official one, but I've found that I prefer this other way. The test that checked that pushInsteadOf + pushurl shouldn't work as I expect was actually broken; I have removed it, updated the documentation, and sent a new patch to the list. There's an argument for either behavior as valid. My original patch specifically documented and tested for the opposite behavior, namely that pushurl overrides any pushInsteadOf, because I intended pushInsteadOf as a fallback if you don't have an explicit pushurl set. For instance, you could use pushInsteadOf to rewrite a family of anonymous git URLs to corresponding pushable repositories, but then use an explicit pushurl to override that for a specific repository. This change would break the ability to use pushurl for its original intended purpose, namely having a local repository where fetch comes from one remote repo and push goes to another. One use case of mine: I have a .gitconfig in my git-managed home directory which sets pushInsteadOf so that I can clone via git:// and immediately have working push. I work with a number of systems that don't have inbound access to each other but do have outbound access to the network; on some of these satellite boxes, I can't push changes directly to the server pushInsteadOf points to, so I can explicitly set pushurl in .git/config for that repository, which overrides the pushInsteadOf. This change would break that configuration. - Josh Triplett -- 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
Re: [PATCH] push: Alias pushurl from push rewrites
On Wed, Mar 27, 2013 at 04:09:43PM -0700, Josh Triplett wrote: On Wed, Mar 27, 2013 at 05:48:45PM -0500, Rob Hoelz wrote: On Wed, 27 Mar 2013 15:07:18 -0700 Junio C Hamano gits...@pobox.com wrote: Jonathan Nieder jrnie...@gmail.com writes: Sorry, typo. The configuration in the example above should have been [url git://anongit.myserver.example.com/] insteadOf = myserver.example.com: [url myserver.example.com:] pushInsteadOf = myserver.example.com: In other words, suppose I set url.*.insteadof to point to a faster address for fetching alongside url.*.pushinsteadof requesting that the original address should still be used for pushing. I didn't know we were even shooting for supporting the identity mapping: url.X.pushinsteadof=X but that would certainly be nice. By the way, it seems that the original commit 1c2eafb89bca (Add url.base.pushInsteadOf: URL rewriting for push only, 2009-09-07) wanted to explicitly avoid use of pushInsteadOf aliasing for a pushURL and it is also documented in config.txt from day one. I think the intent is You have a normal URL, and a way to override it explicitly with pushURL, or a way to rewrite it via the aliasing the normal URL with pushInsteadOf. Either one or the other, but not both, as having many levels of indirection would be confusing. Which I can understand and sympathise. In anay case, the change proposed in this thread seems to change that, so the documentation would need to be updated. Also the tests the original commit adds explicitly checks that pushInsteadOf is ignored, which may have to be updated (unless that test is already broken). My use case is that I use Github for my personal development. I have a prefix for my personal repos (hoelzro: - git://github.com/hoelzro for fetch, g...@github.com:hoelzro/ for push) and one for all other Git repos (github: - git://github.com/) I have a few projects where I work in a fork, but I want to fetch updates from the original project. So my url for the origin remote is github:org/project, but my pushurl is hoelzro:project. This behavior in Git currently doesn't allow me to work that way. I used to work with two remotes; origin for my repo and base for the official one, but I've found that I prefer this other way. The test that checked that pushInsteadOf + pushurl shouldn't work as I expect was actually broken; I have removed it, updated the documentation, and sent a new patch to the list. There's an argument for either behavior as valid. My original patch specifically documented and tested for the opposite behavior, namely that pushurl overrides any pushInsteadOf, because I intended pushInsteadOf as a fallback if you don't have an explicit pushurl set. For instance, you could use pushInsteadOf to rewrite a family of anonymous git URLs to corresponding pushable repositories, but then use an explicit pushurl to override that for a specific repository. This change would break the ability to use pushurl for its original intended purpose, namely having a local repository where fetch comes from one remote repo and push goes to another. One use case of mine: I have a .gitconfig in my git-managed home directory which sets pushInsteadOf so that I can clone via git:// and immediately have working push. I work with a number of systems that don't have inbound access to each other but do have outbound access to the network; on some of these satellite boxes, I can't push changes directly to the server pushInsteadOf points to, so I can explicitly set pushurl in .git/config for that repository, which overrides the pushInsteadOf. This change would break that configuration. Clarifying this use case a bit: note that it's been a while since I had many such boxes, so I don't actually have any systems currently using that pushurl configuration. Still a regression in defined behavior, though. Why not just use insteadOf for your personal github prefix hoelzro:, and both insteadOf and pushInsteadOf for github: in general? Then, a repository cloned via github: would work for pull and push (if you have push access), and you can change pushurl to your personal github alias if needed. Though, as Junio said, the modern push-updates-remote-heads behavior of git means that both of our configurations arguably seem wrong, and we should both just use separate remotes for separate repos. - Josh Triplett -- 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
Re: [PATCH] push: Alias pushurl from push rewrites
Josh Triplett wrote: I have a .gitconfig in my git-managed home directory which sets pushInsteadOf so that I can clone via git:// and immediately have working push. I work with a number of systems that don't have inbound access to each other but do have outbound access to the network; on some of these satellite boxes, I can't push changes directly to the server pushInsteadOf points to, so I can explicitly set pushurl in .git/config for that repository, which overrides the pushInsteadOf. This change would break that configuration. Would it? As long as your pushurl does not start with git://, I think your configuration would still work fine. After this patch, neither pushInsteadOf nor pushUrl overrides the other one. The rule is: 1. First, get the URL from the remote's configuration, based on whether you are fetching or pushing. (At this step, in your setup git chooses the URL specified with pushurl in your .git/config.) 2. Next, apply the most appropriate url.*.insteadOf or url.*.pushInsteadOf rule, based on whether you are fetching or pushing. (At this step, no rewrite rules apply, so the URL is used as is.) -- 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
Re: [PATCH] push: Alias pushurl from push rewrites
On 3/19/13 7:08 PM, Junio C Hamano wrote: Jonathan Nieder jrnie...@gmail.com writes: Junio C Hamano wrote: Jonathan Nieder jrnie...@gmail.com writes: Test nits: ... 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' Are these supposed to be follow-up patches? Preparatory steps that Rob can reroll on top? Something else? Preparatory steps. OK, thanks. I'll queue these first then. Should I apply these to my patch to move things along? What's the next step for me? Thanks, Rob -- 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
Re: [PATCH] push: Alias pushurl from push rewrites
Jonathan Nieder jrnie...@gmail.com writes: Junio C Hamano wrote: Jonathan Nieder jrnie...@gmail.com writes: Test nits: ... 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' Are these supposed to be follow-up patches? Preparatory steps that Rob can reroll on top? Something else? Preparatory steps. OK, thanks. I'll queue these first then. -- 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
Re: [PATCH] push: Alias pushurl from push rewrites
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
Re: [PATCH] push: Alias pushurl from push rewrites
On Sun, 17 Mar 2013 16:35:59 -0700
Junio C Hamano gits...@pobox.com 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.
I've taken your advice, and an amended patch follows.
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
Re: [PATCH] push: Alias pushurl from push rewrites
Jonathan Nieder jrnie...@gmail.com writes: Test nits: ... 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' Are these supposed to be follow-up patches? Preparatory steps that Rob can reroll on top? Something else? -- 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
Re: [PATCH] push: Alias pushurl from push rewrites
Junio C Hamano wrote: Jonathan Nieder jrnie...@gmail.com writes: Test nits: ... 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' Are these supposed to be follow-up patches? Preparatory steps that Rob can reroll on top? Something else? Preparatory steps. -- 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
Re: [PATCH] push: Alias pushurl from push rewrites
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.
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
