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

[PATCH 2/2] t5351: avoid using `test_cmp` for binary data

From
Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>
Date
Jul 29, 2022, 12:28 UTC
Message-ID
<b9203ea247776332e4b6f519aa27d541207adc2f.1659097724.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.1308.git.1659097724.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>

The `test_cmp` function is meant to provide nicer output than `cmp` when expected and actual output of Git commands disagree. The implicit assumption is that the output is line-based and human readable.

However, aaf81223f48 (unpack-objects: use stream_loose_object() to unpack large objects, 2022-06-11) introduced a call that compares the contents of pack files, which are distinctly not line-based nor human readable.

This causes problems because on Windows, we hand off to the Bash function `mingw_test_cmp` that compares the lines while ignoring line ending differences. And this Bash function spends an insane amount of cycles trying to read in that binary pack file, so that it is almost indistinguishable from an infinite loop.

For example, t5351 took 1486 seconds in the CI run at https://github.com/git/git/runs/7398490747?check_suite_focus=true#step:5:171, to complete. And yes, that is almost half an hour.

Since Git's tests already use `cmp` consistently when comparing pack files, let's change this instance to use `cmp` instead of `test_cmp`, too, and fix that performance problem.

Now t5351 takes all of 22 seconds.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t5351-unpack-large-objects.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t5351-unpack-large-objects.sh b/t/t5351-unpack-large-objects.sh
index dd7ebc40072..e936f91c3ba 100755
--- a/t/t5351-unpack-large-objects.sh
+++ b/t/t5351-unpack-large-objects.sh
@@ -93,7 +93,7 @@ test_expect_success 'do not unpack existing large objects' '
 
 	# The destination came up with the exact same pack...
 	DEST_PACK=$(echo dest.git/objects/pack/pack-*.pack) &&
-	test_cmp pack-$PACK.pack $DEST_PACK &&
+	cmp pack-$PACK.pack $DEST_PACK &&
 
 	# ...and wrote no loose objects
 	test_stdout_line_count = 0 find dest.git/objects -type f ! -name "pack-*"
-- 
gitgitgadget
Previous: brian m. carlsonNext: Derrick Stolee
Message 5 of 8 in “ci: fix the FreeBSD build”
  1. 0/2 ci: fix the FreeBSD buildJohannes Schindelin via GitGitGadget, Jul 29, 2022
  2. 1/2 t5351: avoid relying on `core.fsyncMethod = batch` to be supportedJohannes Schindelin via GitGitGadget, Jul 29, 2022
  3. Junio C HamanoJul 29, 2022
  4. brian m. carlsonJul 29, 2022
  5. 2/2 t5351: avoid using `test_cmp` for binary dataJohannes Schindelin via GitGitGadget, Jul 29, 2022
  6. Derrick StoleeJul 29, 2022
  7. Carlo Marcelo Arenas BelónJul 29, 2022
  8. Junio C HamanoJul 29, 2022

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.