Re: [PATCH v3 1/4] t1400: avoid touching refs on filesystem

From: Jeff King <peff@peff.net>
Date: Nov 11, 2020, 23:06 UTC
Message-ID: <20201111230659.GA632312@coredump.intra.peff.net>
In-Reply-To: <e66b1bcc62139f62866dc9f25856eaebfe107056.1605077740.git.ps@pks.im>

On Wed, Nov 11, 2020 at 07:58:38AM +0100, Patrick Steinhardt wrote:

Show 9 quoted lines

> The testcase t1400 exercises the git-update-ref(1) utility. To do so,
> many tests directly read and write references via the filesystem,
> assuming that we always use loose and/or packed references. While this
> is true now, it'll change with the introduction of the reftable backend.
> 
> Convert those tests to use git-update-ref(1) and git-show-ref(1) where
> possible. As some tests exercise behaviour with broken references and
> neither of those tools actually allows writing or reading broken
> references, this commit doesn't adjust all tests.

Do you want to mention the switch away from using HEAD in some tests here?

Show 5 quoted lines

> +# Some of the tests delete HEAD, which causes us to not treat the current
> +# working directory as a Git repository anymore. To avoid using any potential
> +# parent repository to be discovered, we need to set up the ceiling directories.
> +GIT_CEILING_DIRECTORIES="$PWD/.."
> +export GIT_CEILING_DIRECTORIES

Do we still need this, now that we're not deleting HEAD? I think we do still delete a branch via HEAD, but that should leave an unborn branch, which is still a valid repo.

Show 10 quoted lines

>  test_expect_success "deleting current branch adds message to HEAD's log" '
> -	test_when_finished "rm -f .git/$m" &&
> +	test_when_finished "git update-ref -d $m" &&
>  	git update-ref $m $A &&
>  	git symbolic-ref HEAD $m &&
>  	git update-ref -m delete-$m -d $m &&
> -	test_path_is_missing .git/$m &&
> +	test_must_fail git show-ref --verify -q $m &&
>  	grep "delete-$m$" .git/logs/HEAD
>  '

E.g., these ones should leave a valid repo (and must remain HEAD, because it's special for reflogging).

Show 9 quoted lines

> -cp -f .git/HEAD .git/HEAD.orig
>  test_expect_success 'delete symref without dereference' '
> -	test_when_finished "cp -f .git/HEAD.orig .git/HEAD" &&
> -	git update-ref --no-deref -d HEAD &&
> -	test_path_is_missing .git/HEAD
> +	git symbolic-ref SYMREF $m &&
> +	git update-ref --no-deref -d SYMREF &&
> +	test_must_fail git show-ref --verify -q SYMREF
>  '

And now this one is safe. Good.

I wonder, though...is it still testing the same thing as the original? This is not related to the use of SYMREF vs HEAD, but wouldn't show-ref similarly fail if we had deleted $m, but left SYMREF in place (i.e., if --no-deref didn't actually do anything)?

Perhaps this would be better:

  # confirm that the pointed-to ref is still there
  git show-ref --verify $m &&
  # but our symref is not
  test_must_fail git show-ref --verify SYMREF &&
  test_must_fail git symbolic-ref SYMREF

Show 12 quoted lines

>  test_expect_success 'delete symref without dereference when the referred ref is packed' '
> -	test_when_finished "cp -f .git/HEAD.orig .git/HEAD" &&
>  	echo foo >foo.c &&
>  	git add foo.c &&
>  	git commit -m foo &&
> +	git symbolic-ref SYMREF $m &&
>  	git pack-refs --all &&
> -	git update-ref --no-deref -d HEAD &&
> -	test_path_is_missing .git/HEAD
> +	git update-ref --no-deref -d SYMREF &&
> +	test_must_fail git show-ref --verify -q SYMREF
>  '

Likewise here.

-Peff