Re: [PATCH v2 3/5] t/perf: fix benchmarks with out-of-tree builds

From: Christian Couder <christian.couder@gmail.com>
Date: Apr 20, 2025, 10:00 UTC
Message-ID: <CAP8UFD3HcAVs351sabU2qd70PmMzTtGbS7P7eKYyDyUQZ4nQ2Q@mail.gmail.com>
In-Reply-To: <20250414-pks-meson-benchmarks-v2-3-04377080a167@pks.im>

On Mon, Apr 14, 2025 at 8:51 AM Patrick Steinhardt <ps@pks.im> wrote:

Show 11 quoted lines

> --- a/t/perf/perf-lib.sh
> +++ b/t/perf/perf-lib.sh
> @@ -25,7 +25,29 @@ TEST_OUTPUT_DIRECTORY=$(pwd)
>  TEST_NO_CREATE_REPO=t
>  TEST_NO_MALLOC_CHECK=t
>
> -. ../test-lib.sh
> +# While test-lib.sh computes the build directory for us, we also have to do the
> +# same thing in order to locate the script via GIT-BUILD-OPTIONS in the first
> +# place.
> +GIT_BUILD_DIR="${GIT_BUILD_DIR:-$TEST_DIRECTORY/..}"

Right now on 'master' there is:

GIT_BUILD_DIR="${GIT_BUILD_DIR:-${TEST_DIRECTORY%/t}}"
if test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
then
       echo "PANIC: Running in a $TEST_DIRECTORY that doesn't end in '/t'?" >&2
       exit 1
fi

so it's not exactly the same thing, even if it still probably works well.

Future readers might wonder if this discrepancy results from changes that were made to only one of the files or if we really wanted to get rid of the "/t" check here. In case we do want to get rid of the "/t" check, I think it might be worth saying it clearly in the comment.

Show 14 quoted lines

> +if test -f "$GIT_BUILD_DIR/GIT-BUILD-DIR"
> +then
> +       GIT_BUILD_DIR="$(cat "$GIT_BUILD_DIR/GIT-BUILD-DIR")" || exit 1
> +       # On Windows, we must convert Windows paths lest they contain a colon
> +       case "$(uname -s)" in
> +       *MINGW*)
> +               GIT_BUILD_DIR="$(cygpath -au "$GIT_BUILD_DIR")"
> +               ;;
> +       esac
> +fi
> +
> +if test ! -f "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
> +then
> +       echo >&2 'error: GIT-BUILD-OPTIONS missing (has Git been built?).'

Maybe something like the following could help debug this:

       echo >&2 "error: GIT-BUILD-OPTIONS file missing from '$GIT_BUILD_DIR'"
       echo >&2 'error: (has Git been built?).'

> +       exit 1
> +fi