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

[PATCH 1/2] t5801: fix object-format handling in git-remote-testgit

From
Jeff King <peff@peff.net>
Date
Mar 7, 2024, 08:51 UTC
Message-ID
<20240307085158.GA2072294@coredump.intra.peff.net>
In-Reply-To
<20240307084735.GA2072130@coredump.intra.peff.net>

Our fake remote helper tries to handle the object-format capability, courtesy of 3716d50dd5 (remote-testgit: adapt for object-format, 2020-06-19). But its parsing isn't quite right; it expects to receive "option object-format true", but the transport-helper code just sends "option object-format" with no value.

As a result, we never set the $object_format variable to "true". And worse, because $val is used unquoted, this confuses the shell's "test" command, which prints something like:

  .../git/t/t5801/git-remote-testgit: 150: test: =: unexpected operator

It all turns out to be harmless, though, because we never look at $object_format after that!

The Git-side behavior comes from 8b85ee4f47 (transport-helper: implement object-format extensions, 2020-05-25). It is a bit unlike other "option" variables, which always say "true" or "false". But in this case, there's not really any need to do so. As I understand it from that commit, the sequence is something like:

  1. the remote helper in its capabilities list says "object-format" to
     tell Git that it understands the object-format option.
  2. Git then tells the helper "option object-format" to tell it that it
     too understands object-formats.
  3. when the remote helper lists refs, it sends a special
     ":object-format" line that tells Git which object format it is
     using. But it presumably should only do this if we found out that
     the other side supports object-formats in step (2).
So let's improve our remote-testgit helper a bit:
  - when we see an object-format line, just set object_format=true;
    that's the only useful thing to take away from it
  - make sure that object_format is set before sending the special
    ":object-format" line. Since we're always testing against a version
    of Git recent enough to have sent us the object-format option, this
    is mostly a noop. But it confirms that the transport-helper code is
    correctly sending us the option (if we fail to send the line, then
    the test will fail when run with GIT_TEST_DEFAULT_HASH=sha256).
Signed-off-by: Jeff King <peff@peff.net>
---
The only other helper we ship that knows about object-format is
remote-curl. And there it _does_ expect "true" or an algorithm.
Curiously the "true" thing works because the remote-curl code silently
rewrites "option foo" to be the same as "option foo true". And even
though it understands receiving a specific algorithm, I'm not sure it
would do anything useful (whatever the caller says is generally
overwritten by the info/refs response).
So I dunno.
 t/t5801/git-remote-testgit | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/t/t5801/git-remote-testgit b/t/t5801/git-remote-testgit
index 1544d6dc6b..b348608847 100755
--- a/t/t5801/git-remote-testgit
+++ b/t/t5801/git-remote-testgit
@@ -25,6 +25,7 @@ GIT_DIR="$url/.git"
 export GIT_DIR
 
 force=
+object_format=
 
 mkdir -p "$dir"
 
@@ -56,7 +57,8 @@ do
 		echo
 		;;
 	list)
-		echo ":object-format $(git rev-parse --show-object-format=storage)"
+		test -n "$object_format" &&
+			echo ":object-format $(git rev-parse --show-object-format=storage)"
 		git for-each-ref --format='? %(refname)' 'refs/heads/' 'refs/tags/'
 		head=$(git symbolic-ref HEAD)
 		echo "@$head HEAD"
@@ -142,7 +144,7 @@ do
 			echo "ok"
 			;;
 		object-format)
-			test $val = "true" && object_format="true" || object_format=
+			object_format=true
 			echo "ok"
 			;;
 		*)
-- 
2.44.0.463.g71abcb3a9f
Previous: Jeff KingNext: Jeff King
Message 2 of 21 in “some transport-helper "option object-format" confusion”
  1. 0/2 some transport-helper "option object-format" confusionJeff King, Mar 7, 2024
  2. 1/2 t5801: fix object-format handling in git-remote-testgitJeff King, Mar 7, 2024
  3. 2/2 doc/gitremote-helpers: match object-format option docs to codeJeff King, Mar 7, 2024
  4. brian m. carlsonMar 7, 2024
  5. Jeff KingMar 12, 2024
  6. brian m. carlsonMar 13, 2024
  7. Eric W. BiedermanMar 14, 2024
  8. brian m. carlsonMar 14, 2024
  9. Eric W. BiedermanMar 15, 2024
  10. Jeff KingMar 16, 2024
  11. Eric W. BiedermanMar 17, 2024
  12. Jeff KingMar 18, 2024
  13. Junio C HamanoMar 14, 2024
  14. brian m. carlsonMar 14, 2024
  15. 0/3 some transport-helper "option object-format" confusionJeff King, Mar 20, 2024
  16. 1/3 transport-helper: use write helpers more consistentlyJeff King, Mar 20, 2024
  17. 2/3 transport-helper: drop "object-format <algo>" optionJeff King, Mar 20, 2024
  18. 3/3 transport-helper: send "true" value for object-format optionJeff King, Mar 20, 2024
  19. Junio C HamanoMar 20, 2024
  20. Eric W. BiedermanMar 20, 2024
  21. Jeff KingMar 27, 2024

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.