Re: [PATCH 12/16] test-lib: modernize test_create_repo() function

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

On Thu, Apr 15, 2021 at 11:10:13PM +0200, SZEDER Gábor wrote:
> >  3. We won't ever hit that "Cannot setup test environment"
> >     error.
> 
> ENOSPC?  Some rogue background process on Windows still desperately
> clinging to an open file descriptor to some file in the same
> directory, preventing 'rm -rf "$TRASH_DIRECTORY"' near the beginning
> of 'test-lib.sh' and interfering with 'git init'?
> 
> >     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.
> 
> I agree that if we already have a 'git' binary that can run 'git
> version', then we can safely assume that it will be able to run 'git
> init' as well.  It might be that 'git init' is buggy and segfaults,
> but that is not a "have you built things yet?" kind of error.

> > -	mkdir -p "$repo"
> > -	(
> > -		cd "$repo" || error "Cannot setup test environment"
> > -		"${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git$X" \
> > -			init \
> > -			"--template=$GIT_BUILD_DIR/templates/blt/" >&3 2>&4 ||
> > -		error "cannot run git init -- have you built things yet?"
> > -		mv .git/hooks .git/hooks-disabled
> > -	) || exit
> 
> This patch removes this '|| exit', which is...
> 
>   - good: if 'test_create_repo' is invoked in a test case (and not in
>     a subshell) and if it were to fail for some reason, then it won't
>     abort the whole test script, but will fail only that test case.
> 
>   - bad: 'test_create_repo' is responsible for creating the repository
>     in the trash directory as well; if that were to fail for any
>     reason, then the test script will not be aborted early.
> 
> I think the 'exit' on error should be removed from 'test_create_repo',
> but the callsite in 'test-lib.sh' should become 'test_create_repo ||
> exit 1'.

Case in point: the bug I just reported in

  https://public-inbox.org/git/20210416211451.GP2947267@szeder.dev/

does break Git in a way that in one of our CI jobs 'git init' is
unable to create the repository in the trash directory.