On Wed, Nov 13, 2024 at 11:19:13AM +0100, Toon Claes wrote:
> > diff --git a/t/test-lib.sh b/t/test-lib.sh
> > index a278181a05..4fe757fe9a 100644
> > --- a/t/test-lib.sh
> > +++ b/t/test-lib.sh
> > @@ -593,9 +593,12 @@ then
> > }
> > else
> > _USE_GLIBC_TUNABLES=
> > + _USE_GLIBC_PRELOAD=
> > if _GLIBC_VERSION=$(getconf GNU_LIBC_VERSION 2>/dev/null) &&
> > _GLIBC_VERSION=${_GLIBC_VERSION#"glibc "} &&
> > - expr 2.34 \<= "$_GLIBC_VERSION" >/dev/null
> > + expr 2.34 \<= "$_GLIBC_VERSION" >/dev/null &&
> > + stderr=$(LD_PRELOAD=$_USE_GLIBC_PRELOAD git version 2>&1 >/dev/null) &&
>
> Can we assume some version of git is in the $PATH here? I see $PATH and
> $GIT_EXEC_PATH are only determined at line 1440 and further.
Hmm, good question. This is after the "you do not seem to have built
git" check, so I thought we were OK. But of course that one is using:
"${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git$X"
So the check is probably running some system "git" and not the built
one. That mostly works out anyway since the real variable there is the
LD_PRELOAD, not the specific version of git. And that's why testing it
worked for me. But on a system without an existing git in the $PATH at
all, it would disable the preload, even though it would work fine.
So some possible fixes are:
- use a more complete path, as the earlier check does
- delay the malloc-debug checking until after we've set up the $PATH
- use a different program. We care about the preload working, so:
LD_PRELOAD=$_USE_GLIBC_PRELOAD ls
would mostly work the same. Though I suppose if you want to get
really crazy, it's possible that "ls" might not be linked in the
same way as our built git (e.g., it could be statically linked
against a different libc).
So I guess just moving it is probably the least-bad option.
> > + test -z "$stderr"
> > then
> > _USE_GLIBC_TUNABLES=YesPlease
>
> Shall we include a warning in a else clause to inform the user the tests
> were started with malloc check, but libc_malloc_debug.so.0 was not found
> and they should either install it or run with TEST_NO_MALLOC_CHECK?
I'm not sure. It is optional, and many systems will happily run without
it. Just identifying glibc ones that happen not to have the debug
library installed seems weird when, say, Windows or FreeBSD similarly
run without it. And we'd be forcing the user to set TEST_NO_MALLOC_CHECK
unless they want to be spammed with the warning from every single test
script that is run.
At that point we should almost just revert my patch and let it fail when
the preload doesn't work (the only advantage is that we could produce a
more useful message).
-Peff
