Re: [PATCH v2 12/12] test-lib: split up and deprecate test_create_repo()

["SZEDER Gábor" <szeder.dev@gmail.com>]

On Sat, Apr 17, 2021 at 02:52:45PM +0200, Ævar Arnfjörð Bjarmason wrote:
> Remove various redundant or obsolete code from the test_create_repo()
> function, and split up its use in test-lib.sh from what tests need
> from it, leaving us with a pass-through wrapper for "git init" in
> test-lib-functions.sh
> 
> Reasons for why we can remove various code from test_create_repo():
> 
>  1. "mkdir -p" isn't needed because "git init" itself will create
>     leading directories if needed.
> 
>  2. Since we're now a simple wrapper for "git init" we don't need to
>     check that we have only one argument. If someone wants to run
>     "test_create_repo --bare x" that's OK.
> 
>  3. We won't ever hit that "Cannot setup test environment"
>     error.
> 
>     Checking the test environment sanity when doing "git init" dates
>     back to eea420693be (t0000: catch trivial pilot errors.,
>     2005-12-10) and 2ccd2027b01 (trivial: check, if t/trash directory
>     was successfully created, 2006-01-05).
> 
>     We can also see it in another form a bit later in my own
>     0d314ce834d (test-lib: use subshell instead of cd $new && .. && cd
>     $old, 2010-08-30).
> 
>     But since 2006f0adaee (t/test-lib: make sure Git has already been
>     built, 2012-09-17) we already check if we have a built git
>     earlier.
> 
>     The one thing this was testing after that 2012 change was that
>     we'd just built "git", but not "git-init", but since
>     3af4c7156c4 (tests: respect GIT_TEST_INSTALLED when initializing
>     repositories, 2018-11-12) we invoke "git", not "git-init".
> 
>     So all of that's been checked already, and we don't need to
>     re-check it here.
> 
>  4. We don't need to move .git/hooks out of the way.
> 
>     That dates back to c09a69a83e3 (Disable hooks during tests.,
>     2005-10-16), since then hooks became disabled by default in
>     f98f8cbac01 (Ship sample hooks with .sample suffix, 2008-06-24).
> 
>     So the hooks were already disabled by default, but as can be seen
>     from "mkdir .git/hooks" changes various tests needed to re-setup
>     that directory. Now they no longer do.
> 
>  5. Since we don't need to move the .git/hooks directory

Since we don't change directory anymore...

> we don't need
>     the subshell here either.
> 
>     That wasn't really needed for the .git/hooks either, but was being
>     done for the convenience of not having to quote the path to the
>     repository as we moved the hooks.

And then this dubious explanation will not be necessary.

>  6. We can drop the --template argument and instead rely on the
>     GIT_TEMPLATE_DIR set to the same path earlier in test-lib.sh. See
>     8683a45d669 (Introduce GIT_TEMPLATE_DIR, 2006-12-19)
> 
>  7. We only needed that ">&3 2>&4" redirection when invoked from
>     test-lib.sh, and the same goes for needing the full path to "git".
> 
>     Let's move that special behavior into test-lib.sh itself.

Quoting myself from my review of the previous version of this patch:

  PATH is already set up to start with GIT_TEST_INSTALLED and/or
  GIT_EXEC_PATH before 'test_create_repo' is called to init the repo in
  the trash directory, so we could simply run 'git' and rely on PATH
  lookup choosing the right executable.

> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 9ebb595c335..f73c3c6fc72 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -1364,7 +1364,10 @@ rm -fr "$TRASH_DIRECTORY" || {
>  remove_trash=t
>  if test -z "$TEST_NO_CREATE_REPO"
>  then
> -	test_create_repo "$TRASH_DIRECTORY"
> +	"${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git$X" \
> +		init \
> +		"$TRASH_DIRECTORY" >&3 2>&4 ||

So this could be just:

  git init "$TRASH_DIRECTORY" >&3 2>&4 ||

> +	error "cannot run git init"
>  else
>  	mkdir -p "$TRASH_DIRECTORY"
>  fi
> -- 
> 2.31.1.722.g788886f50a2
>