git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH 2/2] t5351: avoid using `test_cmp` for binary data
Date: Fri, 29 Jul 2022 12:28:44 +0000	[thread overview]
Message-ID: <b9203ea247776332e4b6f519aa27d541207adc2f.1659097724.git.gitgitgadget@gmail.com> (raw)
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

  parent reply	other threads:[~2022-07-29 12:28 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-29 12:28 [PATCH 0/2] ci: fix the FreeBSD build Johannes Schindelin via GitGitGadget
2022-07-29 12:28 ` [PATCH 1/2] t5351: avoid relying on `core.fsyncMethod = batch` to be supported Johannes Schindelin via GitGitGadget
2022-07-29 16:07   ` Junio C Hamano
2022-07-29 21:24     ` brian m. carlson
2022-07-29 12:28 ` Johannes Schindelin via GitGitGadget [this message]
2022-07-29 12:58 ` [PATCH 0/2] ci: fix the FreeBSD build Derrick Stolee
2022-07-29 17:51   ` Carlo Marcelo Arenas Belón
2022-07-29 16:02 ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b9203ea247776332e4b6f519aa27d541207adc2f.1659097724.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).