From: Patrick Steinhardt <ps@pks.im>
Date: Mon, 04 Aug 2025 07:56:40 GMT
Subject: Re: [PATCH] t0450: fix test for out-of-tree builds
Message-ID: <aJBnuOcQQ1AY6hR4@pks.im>
In-Reply-To: <20250804073002.1586332-1-toon@iotcl.com>

On Mon, Aug 04, 2025 at 09:30:02AM +0200, Toon Claes wrote:
> When using Meson, builds are out-of-tree and $GIT_BUILD_DIR gets set to
> the path where the build output is landing. To locate the Documentation
> sources, test 't0450' was using that path.
> 
> Modify test 't0450' to use `$GIT_SOURCE_DIR/Documentation` to find the
> documentation sources.
> 
> Signed-off-by: Toon Claes <toon@iotcl.com>
> ---
>  t/t0450-txt-doc-vs-help.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/t/t0450-txt-doc-vs-help.sh b/t/t0450-txt-doc-vs-help.sh
> index 2f7504ae7e..da2d0af5b0 100755
> --- a/t/t0450-txt-doc-vs-help.sh
> +++ b/t/t0450-txt-doc-vs-help.sh
> @@ -41,7 +41,7 @@ help_to_synopsis () {
>  }
>  
>  builtin_to_adoc () {
> -       echo "$GIT_BUILD_DIR/Documentation/git-$1.adoc"
> +       echo "$GIT_SOURCE_DIR/Documentation/git-$1.adoc"
>  }

Ok, the change itself looks reasonable to me. One question that the
commit message doesn't answer though is why this didn't cause the test
to fail. I think the answer is that we have the following loop:

	while read builtin
	do
		...

		adoc="$(builtin_to_adoc "$builtin")" &&
		preq="$(echo BUILTIN_ADOC_$builtin | tr '[:lower:]-' '[:upper:]_')" &&

		if test -f "$adoc"
		then
			test_set_prereq "$preq"
		fi &&

		...
	done <builtins

So we explicitly check wether the ".adoc" file exists, and if it doesn't
we don't have its prereq. All subsequent tests then use that prereq, so
we skip all of those tests with Meson entirely.

Which indicates that this prereq is overly loose: ideally we should not
skip such tests, but rather print an error that something is fishy. I
suspect that we have the prereq in place because there are some builtins
that don't have a corresponding manpage though.

Maybe this is something we could explore: what breaks if we remove the
prereq entirely? And if this breakage is limited to a small number of
builtins we can maybe use an explicit skip-list like we already do with
"t/t0450/adoc-help-mismatches".

Thanks!

Patrick