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
> 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).
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/ &&
> 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