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

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2859 bytes --]

On Wed, Nov 11, 2020 at 06:06:59PM -0500, Jeff King wrote:
> On Wed, Nov 11, 2020 at 07:58:38AM +0100, Patrick Steinhardt wrote:
> > +# 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.

Good point, we don't.

> >  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).
> 
> > -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

It would be, but I bailed at this point because we don't actually have
"$m" at this point. But agreed, i'll also include this into both tests.

Patrick

> >  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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]