Re: [PATCH v3 3/4] test-lib: make $GIT_BUILD_DIR an absolute path
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Feb 21, 2022, 17:29 UTC
- Message-ID
- <YhPL+wSxtI0KIz07@nand.local>
- In-Reply-To
- <patch-v3-3.4-b03ae29fc92-20220221T155656Z-avarab@gmail.com>
On Mon, Feb 21, 2022 at 04:58:34PM +0100, Ævar Arnfjörð Bjarmason wrote:
Show 10 quoted lines
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 77c3fabd041..ff13321bfd3 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -41,7 +41,7 @@ then
> # elsewhere
> TEST_OUTPUT_DIRECTORY=$TEST_DIRECTORY
> fi
> -GIT_BUILD_DIR="$TEST_DIRECTORY"/..
> +GIT_BUILD_DIR="${TEST_DIRECTORY%/t}"Sorry to notice this so late, but this hunk caught my eye. What happens if `TEST_DIRECTORY` is provided by the user (and doesn't end in "/t")?
Before this change, we would have set GIT_BUILD_DIR to the parent of whatever TEST_DIRECTORY is, whether or not it ended in "/t". We'll still do the same thing with this patch if TEST_DIRECTORY ends in "/t". But if it doesn't, then we'll set GIT_BUILD_DIR to be the same as TEST_DIRECTORY, which is a behavior change.
If you just want GIT_BUILD_DIR to be absolute in order to for LSan to understand how to correctly strip it out of filenames, could we replace this with:
GIT_BUILD_DIR="$(cd "$TEST_DIRECTORY/.." && pwd)"
or (an admittedly less clear)
GIT_BUILD_DIR="$(dirname "$TEST_DIRECTORY")"
Thanks, Taylor