git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] t/interop: allow tests to run "git env--helper"
@ 2023-01-10 13:39 Jeff King
  2023-01-12 16:03 ` [PATCH] env-helper: move this built-in to to "test-tool env-helper" Ævar Arnfjörð Bjarmason
  0 siblings, 1 reply; 6+ messages in thread
From: Jeff King @ 2023-01-10 13:39 UTC (permalink / raw)
  To: git

The interop test library sets up wrappers "git.a" and "git.b" to
represent the two versions to be tested. It also wraps vanilla "git" to
report an error, with the goal of catching tests which accidentally fail
to use one of the version-specific wrappers (which could invalidate the
tests in a very subtle way).

As a result, t/interop/i5700 has refused to run since 3b072c577b (tests:
replace test_tristate with "git env--helper", 2019-06-21). The problem
is that lib-git-daemon.sh uses "git env--helper" to decide whether to
run daemon tests, which triggers the vanilla wrapper. That produces an
error, and we think that the daemon tests are disabled.

Let's make our wrapper a little smarter, and allow env--helper
specifically. It's not an interesting part of Git to test for interop,
and it's used extensively in test setup. The matching is rudimentary
(e.g., it would not catch "git --some-arg env--helper", but it's enough
for our small set of interop tests).

Let's likewise improve the error message from the wrapper (when it does
complain) to show the arguments to git. That makes debugging a situation
like this much easier, since otherwise you are clueless about who is
calling "git" and why.

Signed-off-by: Jeff King <peff@peff.net>
---
Obviously nobody has run these for a while. I just happened to do so
today while investigating something else. Maybe they're not worth
keeping around, and we should consider it all a failed experiment.
But it's easy enough to fix in the meantime.

Arguably "git env--helper" should be "test-tool", which wouldn't run
into this problem, but it's probably not worth refactoring it for the
sake of these tests.

By the way, if you do try to run the whole set of tests, note that Git
v1.0.0 no longer builds with modern openssl (you can't declare a BIGNUM
anymore; you have to get it from BN_new()), and thus i5500 fails. You
can work around it with:

  # this gets baked into GIT-BUILD-OPTIONS
  make GIT_INTEROP_MAKE_OPTS=NO_OPENSSL=Nope

  # and now interop tests will use it for building other versions
  cd t/interop
  make

 t/interop/interop-lib.sh | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/t/interop/interop-lib.sh b/t/interop/interop-lib.sh
index 3e0a2911d4..f65baa607d 100644
--- a/t/interop/interop-lib.sh
+++ b/t/interop/interop-lib.sh
@@ -67,9 +67,18 @@ generate_wrappers () {
 	mkdir -p .bin &&
 	wrap_git .bin/git.a "$DIR_A" &&
 	wrap_git .bin/git.b "$DIR_B" &&
+	GENERIC_GIT="$TEST_DIRECTORY/../bin-wrappers/git" &&
+	export GENERIC_GIT &&
 	write_script .bin/git <<-\EOF &&
-	echo >&2 fatal: test tried to run generic git
-	exit 1
+	case "$1" in
+	env--helper)
+		exec "$GENERIC_GIT" "$@"
+		;;
+	*)
+		echo >&2 fatal: test tried to run generic git: $*
+		exit 1
+		;;
+	esac
 	EOF
 	PATH=$(pwd)/.bin:$PATH
 }
-- 
2.39.0.508.g93b13bde48

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-01-15  2:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-10 13:39 [PATCH] t/interop: allow tests to run "git env--helper" Jeff King
2023-01-12 16:03 ` [PATCH] env-helper: move this built-in to to "test-tool env-helper" Ævar Arnfjörð Bjarmason
2023-01-12 16:39   ` Jeff King
2023-01-13 20:17     ` Junio C Hamano
2023-01-14 17:43   ` Andrei Rybak
2023-01-15  2:06     ` Junio C Hamano

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).