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

[Christian Couder <christian.couder@gmail.com>]

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

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

> +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