Re: [PATCH 7/9] cmake: support using GIT_TEST_OPTS from the environment

From: Phillip Wood <phillip.wood123@gmail.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Phillip Wood <phillip.wood123@gmail.com>,
	Victoria Dye <vdye@github.com>,
	Eric Sunshine <ericsunshine@gmail.com>
Subject: Re: [PATCH 7/9] cmake: support using GIT_TEST_OPTS from the environment
Date: Tue, 25 Oct 2022 14:38:09 +0100	[thread overview]
Message-ID: <494668dc-9df0-ee20-1cc1-bd5f01d1dc19@dunelm.org.uk> (raw)
In-Reply-To: <221021.864jvxfa3c.gmgdl@evledraar.gmail.com>

On 21/10/2022 16:45, Ævar Arnfjörð Bjarmason wrote:
> 
> On Fri, Oct 21 2022, Phillip Wood wrote:
> 
>> Hi Ævar
>>
>> On 21/10/2022 10:44, Ævar Arnfjörð Bjarmason wrote:
>>> Get "GIT_TEST_OPTS" from the environment, and use it to pass arguments
>>> to tests. This allows for passing arguments to tests with e.g.:
>>> 	GIT_TEST_OPTS="--verbose --debug" cmake .; ctest -R t0001
>>> --verbose
>>> There's some overlap with this and what was suggested in [1], but as
>>> noted there we're not passing "--verbose" and friends unconditionally,
>>> so a plain "ctest" invocation without a "cmake" re-build won't pick up
>>> the options.
>>
>> The aim of dscho's patch was to make debugging information available
>> in the test logs without the user having to do anything, now to get
>> that information every user has to set
>> GIT_TEST_OPTS="--no-bin-wrappers --no-chain-lint -vx" when running
>> cmake.
>>
>> I think it would be helpful to have some default options set if the
>> user does not pass GIT_TEST_OPTS. Ideally one would be able to do
>>
>> 	GIT_TEST_OPTS=... ctest
>>
>> and have the tests pick up the options at runtime. Following on from
>> my previous comment, if we used "sh -c" to launch the tests we could
>> have something like
>>
>> 	COMMAND ${SH_EXE} -c [[GIT_TEST_BUILD_DIR="$1"; . "$2"
>> 	${GIT_TEST_OPTS:---no-bin-wrappers --no-chain-lint -vx}]]
>> 	"${tsh}" "${CMAKE_BINARY_DIR}" "${tsh}"
> 
> That sounds reasonable to me. FWIW I looked into
> $CTEST_INTERACTIVE_DEBUG_MODE for this purpose, i.e. to stick something
> like this in test-lib.sh:
> 
> 	if test -n "$CTEST_INTERACTIVE_DEBUG_MODE"
> 	then
> 		verbose=t
> 		trace=t
> 	fi

I think it is useful to have -vx set even if we're not passing --verbose 
to ctest as if a test fails we've got the information to debug it stored 
in ctest's log without having to re-run the test.

> But I was hoping for some way to tell that "ctest" was in "--verbose"
> mode, but AFAICT there's no way to get at that without something like
> compat/linux/procinfo.c (basically a fancier way of parsing "ps auxf").
> 
> Anyway, as noted in my review of dscho's series I thought this part of
> it was odd/outdated given that this thing runs on Linux (mostly, but
> entirely after this series).
> 
> I.e. why would we hardcode Windows-specific trade-offs into a portable
> build-system, and if you do want e.g. "--no-bin-wrappers" why would you
> want that just when you run "cmake", not "make"? Surely if we're pushing
> for a new default it should be agnostic to the user's build system.
> 
> But in any case I think if we're pushing for new (or cmake-specific)
> opinionated defaults it makes sense to split those up & justify them
> separately from bug fixes or workarounds.
> 
> E.g. 2/9 in this series makes much of the tests pass on *nix, but so
> does "--no-bin-wrappers", but just because it happens to bypass the
> broken-on-master bin-wrappers/* made by cmake.

I agree that --no-chain-lint --no-bin-wrappers are there just for 
windows. We could quite easily add those just for windows builds but the 
cmake build is basically only there for windows and the linux support is 
just there to allow testing the cmake build without having to run the ci.

Best Wishes

Phillip