Hi Victoria,
On Thu, 8 Sep 2022, Victoria Dye wrote:
> Johannes Schindelin via GitGitGadget wrote:
>
> > [...]
> > diff --git a/t/test-lib.sh b/t/test-lib.sh
> > index 55857af601b..4468ac51f25 100644
> > --- a/t/test-lib.sh
> > +++ b/t/test-lib.sh
> > @@ -42,7 +42,16 @@ then
> > TEST_OUTPUT_DIRECTORY=$TEST_DIRECTORY
> > fi
> > GIT_BUILD_DIR="${TEST_DIRECTORY%/t}"
> > -if test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
> > +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
> > +elif test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
> > then
> > echo "PANIC: Running in a $TEST_DIRECTORY that doesn't end in '/t'?" >&2
> > exit 1
> Referring to Ævar's review in [1] - while I'm not overly concerned about the
> "switching between make & CMake" file staleness (if I'm not mistaken, the
> same thing can happen now with the modified 'test-lib.sh', so this patch
> doesn't really make anything worse), I do think the changes to 'test-lib.sh'
> should be rearranged to preserve the "PANIC" check:
>
> ----------------->8----------------->8----------------->8-----------------
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 4468ac51f2..7b57f55c37 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -42,6 +42,11 @@ then
> TEST_OUTPUT_DIRECTORY=$TEST_DIRECTORY
> fi
> 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
> if test -f "$GIT_BUILD_DIR/GIT-BUILD-DIR"
> then
> GIT_BUILD_DIR="$(cat "$GIT_BUILD_DIR/GIT-BUILD-DIR")" || exit 1
> @@ -51,10 +56,6 @@ then
> GIT_BUILD_DIR="$(cygpath -au "$GIT_BUILD_DIR")"
> ;;
> esac
> -elif test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR"
> -then
> - echo "PANIC: Running in a $TEST_DIRECTORY that doesn't end in '/t'?" >&2
> - exit 1
> fi
>
> # Prepend a string to a VAR using an arbitrary ":" delimiter, not
> -----------------8<-----------------8<-----------------8<-----------------
>
> Otherwise, a user could run the tests from outside a 't/' directory if they
> built Git with CMake, which doesn't appear to be part of the intended
> behavior of this patch.
Good point!
Thank you,
Dscho
