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

Re: [PATCH v5 3/3] upload-pack: optionally allow fetching reachable sha1

From
Junio C Hamano <gitster@pobox.com>
Date
May 21, 2015, 22:15 UTC
Message-ID
<xmqqioblh91w.fsf@gitster.dls.corp.google.com>
In-Reply-To
<1432239819-21794-3-git-send-email-fredrik.medley@gmail.com>
Fredrik Medley <fredrik.medley@gmail.com> writes:
Show 8 quoted lines
> --- a/Documentation/technical/protocol-capabilities.txt
> +++ b/Documentation/technical/protocol-capabilities.txt
> @@ -260,6 +260,13 @@ If the upload-pack server advertises this capability, fetch-pack may
>  send "want" lines with SHA-1s that exist at the server but are not
>  advertised by upload-pack.
>  
> +allow-reachable-sha1-in-want
> +----------------------

This is an underline applied to one line prior, and their length must match. I'll amend while applying (attached at end), so there is no need to resend with correction unless you have other reasons to do so.

Show 7 quoted lines
> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
> index 8a5f236..fdcc114 100755
> --- a/t/t5516-fetch-push.sh
> +++ b/t/t5516-fetch-push.sh
> @@ -1120,6 +1120,61 @@ test_expect_success 'fetch exact SHA1' '
>  	)
>  '
It looks like this new set of tests are well thought out; good job.

I spotted a few minor nits, though. All I'll amend while applying so there is no need to resend only to correct them.

Show 15 quoted lines
> +for configallowtipsha1inwant in true false
> +do
> +	test_expect_success "shallow fetch reachable SHA1 (but not a ref), allowtipsha1inwant=$configallowtipsha1inwant" '
> +		mk_empty testrepo &&
> +		(
> +			cd testrepo &&
> +			git config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&
> +			git commit --allow-empty -m foo &&
> +			git commit --allow-empty -m bar
> +		) &&
> +		SHA1=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&
> +		mk_empty shallow &&
> +		(
> +			cd shallow &&
> +			test_must_fail git fetch --depth=1 ../testrepo/.git $SHA1 &&

This tries to fetch one before the tip with "allowTipSHA1InWant" set to true or false; either should fail. Good.

> +			git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
> +			git fetch --depth=1 ../testrepo/.git $SHA1 &&

And regardless of allowTip setting, with allowReachable set to true, fetching the reachable HEAD^ would succeed. Good.

> +			git cat-file commit $SHA1 >/dev/null

Minor nit; drop ">/dev/null", as test framework will squelch the output by default, and when the test is run with "-v" option, the output would help debugging the script.

Show 12 quoted lines
> +		)
> +	'
> +
> +	test_expect_success "deny fetch unreachable SHA1, allowtipsha1inwant=$configallowtipsha1inwant" '
> +		mk_empty testrepo &&
> +		(
> +			cd testrepo &&
> +			git config uploadpack.allowtipsha1inwant $configallowtipsha1inwant &&
> +			git commit --allow-empty -m foo &&
> +			git commit --allow-empty -m bar &&
> +			git commit --allow-empty -m xyz
> +		)
Broken && chain
Show 6 quoted lines
> +		SHA1_1=$(git --git-dir=testrepo/.git rev-parse HEAD^^) &&
> +		SHA1_2=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&
> +		SHA1_3=$(git --git-dir=testrepo/.git rev-parse HEAD) &&
> +		(
> +			cd testrepo &&
> +			git reset --hard $SHA1_2 &&

We have one before the tip (SHA1_1), one after the tip and no longer reachable (SHA1_3); SHA1_2 is sitting at the tip of a ref.

> +			git cat-file commit $SHA1_3 >/dev/null &&
> +			git cat-file commit $SHA1_3 >/dev/null

I think one of the latter two is $SHA1_1, i.e. you make sure SHA1_{1,2,3} are there in testrepo.

Show 6 quoted lines
> +		) &&
> +		mk_empty shallow &&
> +		(
> +			cd shallow &&
> +			test_must_fail git fetch ../testrepo/.git $SHA1_3 &&
> +			test_must_fail git fetch ../testrepo/.git $SHA1_1 &&

With allowTip only, whether it is set to true or false, fetching _1 or _3 that are not at tip will fail. Good.

> +			git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
> +			git fetch ../testrepo/.git $SHA1_1 &&
> +			git cat-file commit $SHA1_1 >/dev/null &&

With allowReachable, _1 which is reachable from tip is possible, regardless of the setting of allowTip. Good.

> +			test_must_fail git cat-file commit $SHA1_2 >/dev/null &&

And fetching _1 will not pull in _2, which is _1's child, that we did not ask for. Good (but it is probably not very relevant for the purpose of these tests).

> +			git fetch ../testrepo/.git $SHA1_2 &&
> +			git cat-file commit $SHA1_2 >/dev/null &&
And of course, _2 can be fetched.
> +			test_must_fail git fetch ../testrepo/.git $SHA1_3
And _3 that is not reachable cannot.
> +		)
> +	'
> +done
Again, very carefully covering combinations.  Good.
 Documentation/technical/protocol-capabilities.txt |  2 +-
 t/t5516-fetch-push.sh                             | 14 +++++++-------
 2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt
index 265fcab..eaab6b4 100644
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -261,7 +261,7 @@ send "want" lines with SHA-1s that exist at the server but are not
 advertised by upload-pack.
 
 allow-reachable-sha1-in-want
-----------------------
+----------------------------
 
 If the upload-pack server advertises this capability, fetch-pack may
 send "want" lines with SHA-1s that exist at the server but are not
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index fdcc114..ec22c98 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1137,7 +1137,7 @@ do
 			test_must_fail git fetch --depth=1 ../testrepo/.git $SHA1 &&
 			git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
 			git fetch --depth=1 ../testrepo/.git $SHA1 &&
-			git cat-file commit $SHA1 >/dev/null
+			git cat-file commit $SHA1
 		)
 	'
 
@@ -1149,15 +1149,15 @@ do
 			git commit --allow-empty -m foo &&
 			git commit --allow-empty -m bar &&
 			git commit --allow-empty -m xyz
-		)
+		) &&
 		SHA1_1=$(git --git-dir=testrepo/.git rev-parse HEAD^^) &&
 		SHA1_2=$(git --git-dir=testrepo/.git rev-parse HEAD^) &&
 		SHA1_3=$(git --git-dir=testrepo/.git rev-parse HEAD) &&
 		(
 			cd testrepo &&
 			git reset --hard $SHA1_2 &&
-			git cat-file commit $SHA1_3 >/dev/null &&
-			git cat-file commit $SHA1_3 >/dev/null
+			git cat-file commit $SHA1_1 &&
+			git cat-file commit $SHA1_3
 		) &&
 		mk_empty shallow &&
 		(
@@ -1166,10 +1166,10 @@ do
 			test_must_fail git fetch ../testrepo/.git $SHA1_1 &&
 			git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
 			git fetch ../testrepo/.git $SHA1_1 &&
-			git cat-file commit $SHA1_1 >/dev/null &&
-			test_must_fail git cat-file commit $SHA1_2 >/dev/null &&
+			git cat-file commit $SHA1_1 &&
+			test_must_fail git cat-file commit $SHA1_2 &&
 			git fetch ../testrepo/.git $SHA1_2 &&
-			git cat-file commit $SHA1_2 >/dev/null &&
+			git cat-file commit $SHA1_2 &&
 			test_must_fail git fetch ../testrepo/.git $SHA1_3
 		)
 	'

-- 
2.4.1-439-gcfa393f
Previous: Fredrik MedleyNext: Fredrik Medley
Message 28 of 34 in “upload-pack: Optionally allow fetching reachable sha1”
  1. upload-pack: Optionally allow fetching reachable sha1Fredrik Medley, May 2, 2015
  2. Junio C HamanoMay 3, 2015
  3. Fredrik MedleyMay 3, 2015
  4. Eric SunshineMay 3, 2015
  5. upload-pack: Optionally allow fetching reachable sha1Fredrik Medley, May 5, 2015
  6. Junio C HamanoMay 5, 2015
  7. Fredrik MedleyMay 6, 2015
  8. Junio C HamanoMay 6, 2015
  9. 1/3 config.txt: Clarify allowTipSHA1InWant with camelCaseFredrik Medley, May 12, 2015
  10. 2/3 upload-pack: Prepare to extend allow-tip-sha1-in-wantFredrik Medley, May 12, 2015
  11. Eric SunshineMay 12, 2015
  12. Junio C HamanoMay 12, 2015
  13. Fredrik MedleyMay 19, 2015
  14. 3/3 upload-pack: Optionally allow fetching reachable sha1Fredrik Medley, May 12, 2015
  15. Eric SunshineMay 12, 2015
  16. 1/3 config.txt: clarify allowTipSHA1InWant with camelCaseFredrik Medley, May 19, 2015
  17. 2/3 upload-pack: prepare to extend allow-tip-sha1-in-wantFredrik Medley, May 19, 2015
  18. Junio C HamanoMay 19, 2015
  19. Fredrik MedleyMay 20, 2015
  20. 3/3 upload-pack: optionally allow fetching reachable sha1Fredrik Medley, May 19, 2015
  21. Junio C HamanoMay 19, 2015
  22. 1/3 config.txt: clarify allowTipSHA1InWant with camelCaseFredrik Medley, May 21, 2015
  23. 2/3 upload-pack: prepare to extend allow-tip-sha1-in-wantFredrik Medley, May 21, 2015
  24. Junio C HamanoMay 21, 2015
  25. Fredrik MedleyMay 22, 2015
  26. Junio C HamanoMay 23, 2015
  27. 3/3 upload-pack: optionally allow fetching reachable sha1Fredrik Medley, May 21, 2015
  28. Junio C HamanoMay 21, 2015
  29. Fredrik MedleyMay 22, 2015
  30. Junio C HamanoMay 23, 2015
  31. Eric SunshineMay 12, 2015
  32. Junio C HamanoMay 12, 2015
  33. Junio C HamanoMay 12, 2015
  34. Eric SunshineMay 5, 2015

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.