git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
Mar 28, 2013, 15:37 UTC
Message-ID
<7vfvzfwn55.fsf@alter.siamese.dyndns.org>
In-Reply-To
<20130327230943.GA5204@jtriplet-mobl1>
Josh Triplett <josh@joshtriplett.org> writes:
Show 10 quoted lines
> 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 &&
Previous: Josh TriplettNext: Josh Triplett
Message 13 of 24 in “push: Alias pushurl from push rewrites”
  1. push: Alias pushurl from push rewritesRob Hoelz, Mar 27, 2013
  2. Jonathan NiederMar 27, 2013
  3. Jonathan NiederMar 27, 2013
  4. Junio C HamanoMar 27, 2013
  5. Rob HoelzMar 27, 2013
  6. Josh TriplettMar 27, 2013
  7. Josh TriplettMar 27, 2013
  8. Jonathan NiederMar 27, 2013
  9. Junio C HamanoMar 28, 2013
  10. Josh TriplettMar 28, 2013
  11. Junio C HamanoMar 28, 2013
  12. Josh TriplettMar 28, 2013
  13. Junio C HamanoMar 28, 2013
  14. Josh TriplettMar 28, 2013
  15. Junio C HamanoMar 28, 2013
  16. Josh TriplettMar 28, 2013
  17. Jonathan NiederMar 28, 2013
  18. Rob HoelzMar 29, 2013
  19. Junio C HamanoMar 29, 2013
  20. Rob HoelzMar 27, 2013
  21. Jonathan NiederMar 27, 2013
  22. Rob HoelzMar 27, 2013
  23. Jonathan NiederMar 27, 2013
  24. Rob HoelzMar 27, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.