[PATCH 00/12] Group reffiles tests

[PATCH 00/12] Group reffiles tests

[John Cai via GitGitGadget]

This series groups REFFILES specific tests together. These tests are
currently grouped together across the test suite based on functionality.
However, since they exercise low-level behavior specific to the refs backend
being used (in these cases, the ref-files backend), group them together
based on which refs backend they test. This way, in the near future when the
reftables backend gets upstreamed we can add tests that exercise the
reftables backend close by in the t06xx area.

These patches also remove the REFFILES prerequisite, since all the tests in
t06xx are reffiles specific. In the near future, once the reftable backend
is upstreamed, all the tests in t06xx will be forced to run with the
reffiles backend.

John Cai (12):
  t3210: move to t0602
  remove REFFILES prerequisite
  t1414: convert test to use Git commands instead of writing refs
    manually
  t1404: move reffiles specific tests to t0600
  t1405: move reffiles specific tests to t0600
  t1406: move reffiles specific tests to t0600
  t1410: move reffiles specific tests to t0600
  t1415: move reffiles specific tests to t0600
  t1503: move reffiles specific tests to t0600
  t3903: move reffiles specific tests to t0600
  t4202: move reffiles specific tests to t0600
  t5312: move reffiles specific tests to t0600

 t/t0600-reffiles-backend.sh                   | 604 ++++++++++++++++++
 ...ck-refs.sh => t0602-reffiles-pack-refs.sh} |   0
 t/t1404-update-ref-errors.sh                  | 378 -----------
 t/t1405-main-ref-store.sh                     |  10 +-
 t/t1407-worktree-ref-store.sh                 |  37 --
 t/t1410-reflog.sh                             |  42 --
 t/t1414-reflog-walk.sh                        |  11 +-
 t/t1415-worktree-refs.sh                      |  11 -
 t/t1503-rev-parse-verify.sh                   |   5 -
 t/t2017-checkout-orphan.sh                    |   2 +-
 t/t3903-stash.sh                              |  43 --
 t/t4202-log.sh                                |  17 -
 t/t5312-prune-corruption.sh                   |  26 -
 t/test-lib-functions.sh                       |  16 +
 14 files changed, 628 insertions(+), 574 deletions(-)
 create mode 100755 t/t0600-reffiles-backend.sh
 rename t/{t3210-pack-refs.sh => t0602-reffiles-pack-refs.sh} (100%)


base-commit: 186b115d3062e6230ee296d1ddaa0c4b72a464b5
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1647%2Fjohn-cai%2Fjc%2Fgroup-reffiles-tests-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1647/john-cai/jc/group-reffiles-tests-v1
Pull-Request: https://github.com/git/git/pull/1647
-- 
gitgitgadget

[PATCH 01/12] t3210: move to t0602

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

Move t3210 to t0602, since these tests are reffiles specific in that
they modify loose refs manually. This is part of the effort to
categorize these tests together based on the ref backend they test. When
we upstream the reftable backend, we can add more tests to t06xx. This
way, all tests that test specific ref backend behavior will be grouped
together.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/{t3210-pack-refs.sh => t0602-reffiles-pack-refs.sh} | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename t/{t3210-pack-refs.sh => t0602-reffiles-pack-refs.sh} (100%)

diff --git a/t/t3210-pack-refs.sh b/t/t0602-reffiles-pack-refs.sh
similarity index 100%
rename from t/t3210-pack-refs.sh
rename to t/t0602-reffiles-pack-refs.sh
-- 
gitgitgadget

[PATCH 02/12] remove REFFILES prerequisite

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

These tests are compatible with the reftable backend and thus do not
need the REFFILES prerequisite.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t1405-main-ref-store.sh  | 2 +-
 t/t2017-checkout-orphan.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
index e4627cf1b61..62c1eadb190 100755
--- a/t/t1405-main-ref-store.sh
+++ b/t/t1405-main-ref-store.sh
@@ -112,7 +112,7 @@ test_expect_success 'delete_reflog(HEAD)' '
 	test_must_fail git reflog exists HEAD
 '
 
-test_expect_success REFFILES 'create-reflog(HEAD)' '
+test_expect_success 'create-reflog(HEAD)' '
 	$RUN create-reflog HEAD &&
 	git reflog exists HEAD
 '
diff --git a/t/t2017-checkout-orphan.sh b/t/t2017-checkout-orphan.sh
index 947d1587ac8..a5c7358eeab 100755
--- a/t/t2017-checkout-orphan.sh
+++ b/t/t2017-checkout-orphan.sh
@@ -86,7 +86,7 @@ test_expect_success '--orphan makes reflog by default' '
 	git rev-parse --verify delta@{0}
 '
 
-test_expect_success REFFILES '--orphan does not make reflog when core.logAllRefUpdates = false' '
+test_expect_success '--orphan does not make reflog when core.logAllRefUpdates = false' '
 	git checkout main &&
 	git config core.logAllRefUpdates false &&
 	git checkout --orphan epsilon &&
-- 
gitgitgadget

[PATCH 03/12] t1414: convert test to use Git commands instead of writing refs manually

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

This test can be re-written to use Git commands rather than writing a
manual ref in the reflog. This way this test no longer needs the
REFFILES prerequisite.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t1414-reflog-walk.sh | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/t/t1414-reflog-walk.sh b/t/t1414-reflog-walk.sh
index ea64cecf47b..c7b3817d3bd 100755
--- a/t/t1414-reflog-walk.sh
+++ b/t/t1414-reflog-walk.sh
@@ -121,13 +121,14 @@ test_expect_success 'min/max age uses entry date to limit' '
 
 # Create a situation where the reflog and ref database disagree about the latest
 # state of HEAD.
-test_expect_success REFFILES 'walk prefers reflog to ref tip' '
+test_expect_success 'walk prefers reflog to ref tip' '
+	test_commit A &&
+	test_commit B &&
+	git reflog delete HEAD@{0} &&
 	head=$(git rev-parse HEAD) &&
-	one=$(git rev-parse one) &&
-	ident="$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE" &&
-	echo "$head $one $ident	broken reflog entry" >>.git/logs/HEAD &&
+	A=$(git rev-parse A) &&
 
-	echo $one >expect &&
+	echo $A >expect &&
 	git log -g --format=%H -1 >actual &&
 	test_cmp expect actual
 '
-- 
gitgitgadget

[PATCH 04/12] t1404: move reffiles specific tests to t0600

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

These tests modify loose refs manually and are specific to the reffiles
backend. Move these to t0600 to be part of a test suite of reffiles
specific tests.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t0600-reffiles-backend.sh  | 398 +++++++++++++++++++++++++++++++++++
 t/t1404-update-ref-errors.sh | 378 ---------------------------------
 2 files changed, 398 insertions(+), 378 deletions(-)
 create mode 100755 t/t0600-reffiles-backend.sh

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
new file mode 100755
index 00000000000..332c8cbc004
--- /dev/null
+++ b/t/t0600-reffiles-backend.sh
@@ -0,0 +1,398 @@
+#!/bin/sh
+
+test_description='Test reffiles backend'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+TEST_PASSES_SANITIZE_LEAK=true
+. ./test-lib.sh
+
+# Test adding and deleting D/F-conflicting references in a single
+# transaction.
+df_test() {
+	prefix="$1"
+	pack=: symadd=false symdel=false add_del=false addref= delref=
+	shift
+	while test $# -gt 0
+	do
+		case "$1" in
+		--pack)
+			pack="git pack-refs --all"
+			shift
+			;;
+		--sym-add)
+			# Perform the add via a symbolic reference
+			symadd=true
+			shift
+			;;
+		--sym-del)
+			# Perform the del via a symbolic reference
+			symdel=true
+			shift
+			;;
+		--del-add)
+			# Delete first reference then add second
+			add_del=false
+			delref="$prefix/r/$2"
+			addref="$prefix/r/$3"
+			shift 3
+			;;
+		--add-del)
+			# Add first reference then delete second
+			add_del=true
+			addref="$prefix/r/$2"
+			delref="$prefix/r/$3"
+			shift 3
+			;;
+		*)
+			echo 1>&2 "Extra args to df_test: $*"
+			return 1
+			;;
+		esac
+	done
+	git update-ref "$delref" $C &&
+	if $symadd
+	then
+		addname="$prefix/s/symadd" &&
+		git symbolic-ref "$addname" "$addref"
+	else
+		addname="$addref"
+	fi &&
+	if $symdel
+	then
+		delname="$prefix/s/symdel" &&
+		git symbolic-ref "$delname" "$delref"
+	else
+		delname="$delref"
+	fi &&
+	cat >expected-err <<-EOF &&
+	fatal: cannot lock ref $SQ$addname$SQ: $SQ$delref$SQ exists; cannot create $SQ$addref$SQ
+	EOF
+	$pack &&
+	if $add_del
+	then
+		printf "%s\n" "create $addname $D" "delete $delname"
+	else
+		printf "%s\n" "delete $delname" "create $addname $D"
+	fi >commands &&
+	test_must_fail git update-ref --stdin <commands 2>output.err &&
+	test_cmp expected-err output.err &&
+	printf "%s\n" "$C $delref" >expected-refs &&
+	git for-each-ref --format="%(objectname) %(refname)" $prefix/r >actual-refs &&
+	test_cmp expected-refs actual-refs
+}
+
+test_expect_success 'setup' '
+	git commit --allow-empty -m Initial &&
+	C=$(git rev-parse HEAD) &&
+	git commit --allow-empty -m Second &&
+	D=$(git rev-parse HEAD) &&
+	git commit --allow-empty -m Third &&
+	E=$(git rev-parse HEAD)
+'
+
+test_expect_success 'empty directory should not fool rev-parse' '
+	prefix=refs/e-rev-parse &&
+	git update-ref $prefix/foo $C &&
+	git pack-refs --all &&
+	mkdir -p .git/$prefix/foo/bar/baz &&
+	echo "$C" >expected &&
+	git rev-parse $prefix/foo >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'empty directory should not fool for-each-ref' '
+	prefix=refs/e-for-each-ref &&
+	git update-ref $prefix/foo $C &&
+	git for-each-ref $prefix >expected &&
+	git pack-refs --all &&
+	mkdir -p .git/$prefix/foo/bar/baz &&
+	git for-each-ref $prefix >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'empty directory should not fool create' '
+	prefix=refs/e-create &&
+	mkdir -p .git/$prefix/foo/bar/baz &&
+	printf "create %s $C\n" $prefix/foo |
+	git update-ref --stdin
+'
+
+test_expect_success 'empty directory should not fool verify' '
+	prefix=refs/e-verify &&
+	git update-ref $prefix/foo $C &&
+	git pack-refs --all &&
+	mkdir -p .git/$prefix/foo/bar/baz &&
+	printf "verify %s $C\n" $prefix/foo |
+	git update-ref --stdin
+'
+
+test_expect_success 'empty directory should not fool 1-arg update' '
+	prefix=refs/e-update-1 &&
+	git update-ref $prefix/foo $C &&
+	git pack-refs --all &&
+	mkdir -p .git/$prefix/foo/bar/baz &&
+	printf "update %s $D\n" $prefix/foo |
+	git update-ref --stdin
+'
+
+test_expect_success 'empty directory should not fool 2-arg update' '
+	prefix=refs/e-update-2 &&
+	git update-ref $prefix/foo $C &&
+	git pack-refs --all &&
+	mkdir -p .git/$prefix/foo/bar/baz &&
+	printf "update %s $D $C\n" $prefix/foo |
+	git update-ref --stdin
+'
+
+test_expect_success 'empty directory should not fool 0-arg delete' '
+	prefix=refs/e-delete-0 &&
+	git update-ref $prefix/foo $C &&
+	git pack-refs --all &&
+	mkdir -p .git/$prefix/foo/bar/baz &&
+	printf "delete %s\n" $prefix/foo |
+	git update-ref --stdin
+'
+
+test_expect_success 'empty directory should not fool 1-arg delete' '
+	prefix=refs/e-delete-1 &&
+	git update-ref $prefix/foo $C &&
+	git pack-refs --all &&
+	mkdir -p .git/$prefix/foo/bar/baz &&
+	printf "delete %s $C\n" $prefix/foo |
+	git update-ref --stdin
+'
+
+test_expect_success 'D/F conflict prevents add long + delete short' '
+	df_test refs/df-al-ds --add-del foo/bar foo
+'
+
+test_expect_success 'D/F conflict prevents add short + delete long' '
+	df_test refs/df-as-dl --add-del foo foo/bar
+'
+
+test_expect_success 'D/F conflict prevents delete long + add short' '
+	df_test refs/df-dl-as --del-add foo/bar foo
+'
+
+test_expect_success 'D/F conflict prevents delete short + add long' '
+	df_test refs/df-ds-al --del-add foo foo/bar
+'
+
+test_expect_success 'D/F conflict prevents add long + delete short packed' '
+	df_test refs/df-al-dsp --pack --add-del foo/bar foo
+'
+
+test_expect_success 'D/F conflict prevents add short + delete long packed' '
+	df_test refs/df-as-dlp --pack --add-del foo foo/bar
+'
+
+test_expect_success 'D/F conflict prevents delete long packed + add short' '
+	df_test refs/df-dlp-as --pack --del-add foo/bar foo
+'
+
+test_expect_success 'D/F conflict prevents delete short packed + add long' '
+	df_test refs/df-dsp-al --pack --del-add foo foo/bar
+'
+
+# Try some combinations involving symbolic refs...
+
+test_expect_success 'D/F conflict prevents indirect add long + delete short' '
+	df_test refs/df-ial-ds --sym-add --add-del foo/bar foo
+'
+
+test_expect_success 'D/F conflict prevents indirect add long + indirect delete short' '
+	df_test refs/df-ial-ids --sym-add --sym-del --add-del foo/bar foo
+'
+
+test_expect_success 'D/F conflict prevents indirect add short + indirect delete long' '
+	df_test refs/df-ias-idl --sym-add --sym-del --add-del foo foo/bar
+'
+
+test_expect_success 'D/F conflict prevents indirect delete long + indirect add short' '
+	df_test refs/df-idl-ias --sym-add --sym-del --del-add foo/bar foo
+'
+
+test_expect_success 'D/F conflict prevents indirect add long + delete short packed' '
+	df_test refs/df-ial-dsp --sym-add --pack --add-del foo/bar foo
+'
+
+test_expect_success 'D/F conflict prevents indirect add long + indirect delete short packed' '
+	df_test refs/df-ial-idsp --sym-add --sym-del --pack --add-del foo/bar foo
+'
+
+test_expect_success 'D/F conflict prevents add long + indirect delete short packed' '
+	df_test refs/df-al-idsp --sym-del --pack --add-del foo/bar foo
+'
+
+test_expect_success 'D/F conflict prevents indirect delete long packed + indirect add short' '
+	df_test refs/df-idlp-ias --sym-add --sym-del --pack --del-add foo/bar foo
+'
+
+test_expect_success 'non-empty directory blocks create' '
+	prefix=refs/ne-create &&
+	mkdir -p .git/$prefix/foo/bar &&
+	: >.git/$prefix/foo/bar/baz.lock &&
+	test_when_finished "rm -f .git/$prefix/foo/bar/baz.lock" &&
+	cat >expected <<-EOF &&
+	fatal: cannot lock ref $SQ$prefix/foo$SQ: there is a non-empty directory $SQ.git/$prefix/foo$SQ blocking reference $SQ$prefix/foo$SQ
+	EOF
+	printf "%s\n" "update $prefix/foo $C" |
+	test_must_fail git update-ref --stdin 2>output.err &&
+	test_cmp expected output.err &&
+	cat >expected <<-EOF &&
+	fatal: cannot lock ref $SQ$prefix/foo$SQ: unable to resolve reference $SQ$prefix/foo$SQ
+	EOF
+	printf "%s\n" "update $prefix/foo $D $C" |
+	test_must_fail git update-ref --stdin 2>output.err &&
+	test_cmp expected output.err
+'
+
+test_expect_success 'broken reference blocks create' '
+	prefix=refs/broken-create &&
+	mkdir -p .git/$prefix &&
+	echo "gobbledigook" >.git/$prefix/foo &&
+	test_when_finished "rm -f .git/$prefix/foo" &&
+	cat >expected <<-EOF &&
+	fatal: cannot lock ref $SQ$prefix/foo$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken
+	EOF
+	printf "%s\n" "update $prefix/foo $C" |
+	test_must_fail git update-ref --stdin 2>output.err &&
+	test_cmp expected output.err &&
+	cat >expected <<-EOF &&
+	fatal: cannot lock ref $SQ$prefix/foo$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken
+	EOF
+	printf "%s\n" "update $prefix/foo $D $C" |
+	test_must_fail git update-ref --stdin 2>output.err &&
+	test_cmp expected output.err
+'
+
+test_expect_success 'non-empty directory blocks indirect create' '
+	prefix=refs/ne-indirect-create &&
+	git symbolic-ref $prefix/symref $prefix/foo &&
+	mkdir -p .git/$prefix/foo/bar &&
+	: >.git/$prefix/foo/bar/baz.lock &&
+	test_when_finished "rm -f .git/$prefix/foo/bar/baz.lock" &&
+	cat >expected <<-EOF &&
+	fatal: cannot lock ref $SQ$prefix/symref$SQ: there is a non-empty directory $SQ.git/$prefix/foo$SQ blocking reference $SQ$prefix/foo$SQ
+	EOF
+	printf "%s\n" "update $prefix/symref $C" |
+	test_must_fail git update-ref --stdin 2>output.err &&
+	test_cmp expected output.err &&
+	cat >expected <<-EOF &&
+	fatal: cannot lock ref $SQ$prefix/symref$SQ: unable to resolve reference $SQ$prefix/foo$SQ
+	EOF
+	printf "%s\n" "update $prefix/symref $D $C" |
+	test_must_fail git update-ref --stdin 2>output.err &&
+	test_cmp expected output.err
+'
+
+test_expect_success 'broken reference blocks indirect create' '
+	prefix=refs/broken-indirect-create &&
+	git symbolic-ref $prefix/symref $prefix/foo &&
+	echo "gobbledigook" >.git/$prefix/foo &&
+	test_when_finished "rm -f .git/$prefix/foo" &&
+	cat >expected <<-EOF &&
+	fatal: cannot lock ref $SQ$prefix/symref$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken
+	EOF
+	printf "%s\n" "update $prefix/symref $C" |
+	test_must_fail git update-ref --stdin 2>output.err &&
+	test_cmp expected output.err &&
+	cat >expected <<-EOF &&
+	fatal: cannot lock ref $SQ$prefix/symref$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken
+	EOF
+	printf "%s\n" "update $prefix/symref $D $C" |
+	test_must_fail git update-ref --stdin 2>output.err &&
+	test_cmp expected output.err
+'
+
+test_expect_success 'no bogus intermediate values during delete' '
+	prefix=refs/slow-transaction &&
+	# Set up a reference with differing loose and packed versions:
+	git update-ref $prefix/foo $C &&
+	git pack-refs --all &&
+	git update-ref $prefix/foo $D &&
+	# Now try to update the reference, but hold the `packed-refs` lock
+	# for a while to see what happens while the process is blocked:
+	: >.git/packed-refs.lock &&
+	test_when_finished "rm -f .git/packed-refs.lock" &&
+	{
+		# Note: the following command is intentionally run in the
+		# background. We increase the timeout so that `update-ref`
+		# attempts to acquire the `packed-refs` lock for much longer
+		# than it takes for us to do the check then delete it:
+		git -c core.packedrefstimeout=30000 update-ref -d $prefix/foo &
+	} &&
+	pid2=$! &&
+	# Give update-ref plenty of time to get to the point where it tries
+	# to lock packed-refs:
+	sleep 1 &&
+	# Make sure that update-ref did not complete despite the lock:
+	kill -0 $pid2 &&
+	# Verify that the reference still has its old value:
+	sha1=$(git rev-parse --verify --quiet $prefix/foo || echo undefined) &&
+	case "$sha1" in
+	$D)
+		# This is what we hope for; it means that nothing
+		# user-visible has changed yet.
+		: ;;
+	undefined)
+		# This is not correct; it means the deletion has happened
+		# already even though update-ref should not have been
+		# able to acquire the lock yet.
+		echo "$prefix/foo deleted prematurely" &&
+		break
+		;;
+	$C)
+		# This value should never be seen. Probably the loose
+		# reference has been deleted but the packed reference
+		# is still there:
+		echo "$prefix/foo incorrectly observed to be C" &&
+		break
+		;;
+	*)
+		# WTF?
+		echo "unexpected value observed for $prefix/foo: $sha1" &&
+		break
+		;;
+	esac >out &&
+	rm -f .git/packed-refs.lock &&
+	wait $pid2 &&
+	test_must_be_empty out &&
+	test_must_fail git rev-parse --verify --quiet $prefix/foo
+'
+
+test_expect_success 'delete fails cleanly if packed-refs file is locked' '
+	prefix=refs/locked-packed-refs &&
+	# Set up a reference with differing loose and packed versions:
+	git update-ref $prefix/foo $C &&
+	git pack-refs --all &&
+	git update-ref $prefix/foo $D &&
+	git for-each-ref $prefix >unchanged &&
+	# Now try to delete it while the `packed-refs` lock is held:
+	: >.git/packed-refs.lock &&
+	test_when_finished "rm -f .git/packed-refs.lock" &&
+	test_must_fail git update-ref -d $prefix/foo >out 2>err &&
+	git for-each-ref $prefix >actual &&
+	test_grep "Unable to create $SQ.*packed-refs.lock$SQ: " err &&
+	test_cmp unchanged actual
+'
+
+test_expect_success 'delete fails cleanly if packed-refs.new write fails' '
+	# Setup and expectations are similar to the test above.
+	prefix=refs/failed-packed-refs &&
+	git update-ref $prefix/foo $C &&
+	git pack-refs --all &&
+	git update-ref $prefix/foo $D &&
+	git for-each-ref $prefix >unchanged &&
+	# This should not happen in practice, but it is an easy way to get a
+	# reliable error (we open with create_tempfile(), which uses O_EXCL).
+	: >.git/packed-refs.new &&
+	test_when_finished "rm -f .git/packed-refs.new" &&
+	test_must_fail git update-ref -d $prefix/foo &&
+	git for-each-ref $prefix >actual &&
+	test_cmp unchanged actual
+'
+
+test_done
diff --git a/t/t1404-update-ref-errors.sh b/t/t1404-update-ref-errors.sh
index 0369beea33b..6edf3dca9d5 100755
--- a/t/t1404-update-ref-errors.sh
+++ b/t/t1404-update-ref-errors.sh
@@ -34,81 +34,6 @@ test_update_rejected () {
 	test_cmp unchanged actual
 }
 
-# Test adding and deleting D/F-conflicting references in a single
-# transaction.
-df_test() {
-	prefix="$1"
-	pack=: symadd=false symdel=false add_del=false addref= delref=
-	shift
-	while test $# -gt 0
-	do
-		case "$1" in
-		--pack)
-			pack="git pack-refs --all"
-			shift
-			;;
-		--sym-add)
-			# Perform the add via a symbolic reference
-			symadd=true
-			shift
-			;;
-		--sym-del)
-			# Perform the del via a symbolic reference
-			symdel=true
-			shift
-			;;
-		--del-add)
-			# Delete first reference then add second
-			add_del=false
-			delref="$prefix/r/$2"
-			addref="$prefix/r/$3"
-			shift 3
-			;;
-		--add-del)
-			# Add first reference then delete second
-			add_del=true
-			addref="$prefix/r/$2"
-			delref="$prefix/r/$3"
-			shift 3
-			;;
-		*)
-			echo 1>&2 "Extra args to df_test: $*"
-			return 1
-			;;
-		esac
-	done
-	git update-ref "$delref" $C &&
-	if $symadd
-	then
-		addname="$prefix/s/symadd" &&
-		git symbolic-ref "$addname" "$addref"
-	else
-		addname="$addref"
-	fi &&
-	if $symdel
-	then
-		delname="$prefix/s/symdel" &&
-		git symbolic-ref "$delname" "$delref"
-	else
-		delname="$delref"
-	fi &&
-	cat >expected-err <<-EOF &&
-	fatal: cannot lock ref $SQ$addname$SQ: $SQ$delref$SQ exists; cannot create $SQ$addref$SQ
-	EOF
-	$pack &&
-	if $add_del
-	then
-		printf "%s\n" "create $addname $D" "delete $delname"
-	else
-		printf "%s\n" "delete $delname" "create $addname $D"
-	fi >commands &&
-	test_must_fail git update-ref --stdin <commands 2>output.err &&
-	test_cmp expected-err output.err &&
-	printf "%s\n" "$C $delref" >expected-refs &&
-	git for-each-ref --format="%(objectname) %(refname)" $prefix/r >actual-refs &&
-	test_cmp expected-refs actual-refs
-}
-
 test_expect_success 'setup' '
 
 	git commit --allow-empty -m Initial &&
@@ -191,144 +116,6 @@ test_expect_success 'one new ref is a simple prefix of another' '
 
 '
 
-test_expect_success REFFILES 'empty directory should not fool rev-parse' '
-	prefix=refs/e-rev-parse &&
-	git update-ref $prefix/foo $C &&
-	git pack-refs --all &&
-	mkdir -p .git/$prefix/foo/bar/baz &&
-	echo "$C" >expected &&
-	git rev-parse $prefix/foo >actual &&
-	test_cmp expected actual
-'
-
-test_expect_success REFFILES 'empty directory should not fool for-each-ref' '
-	prefix=refs/e-for-each-ref &&
-	git update-ref $prefix/foo $C &&
-	git for-each-ref $prefix >expected &&
-	git pack-refs --all &&
-	mkdir -p .git/$prefix/foo/bar/baz &&
-	git for-each-ref $prefix >actual &&
-	test_cmp expected actual
-'
-
-test_expect_success REFFILES 'empty directory should not fool create' '
-	prefix=refs/e-create &&
-	mkdir -p .git/$prefix/foo/bar/baz &&
-	printf "create %s $C\n" $prefix/foo |
-	git update-ref --stdin
-'
-
-test_expect_success REFFILES 'empty directory should not fool verify' '
-	prefix=refs/e-verify &&
-	git update-ref $prefix/foo $C &&
-	git pack-refs --all &&
-	mkdir -p .git/$prefix/foo/bar/baz &&
-	printf "verify %s $C\n" $prefix/foo |
-	git update-ref --stdin
-'
-
-test_expect_success REFFILES 'empty directory should not fool 1-arg update' '
-	prefix=refs/e-update-1 &&
-	git update-ref $prefix/foo $C &&
-	git pack-refs --all &&
-	mkdir -p .git/$prefix/foo/bar/baz &&
-	printf "update %s $D\n" $prefix/foo |
-	git update-ref --stdin
-'
-
-test_expect_success REFFILES 'empty directory should not fool 2-arg update' '
-	prefix=refs/e-update-2 &&
-	git update-ref $prefix/foo $C &&
-	git pack-refs --all &&
-	mkdir -p .git/$prefix/foo/bar/baz &&
-	printf "update %s $D $C\n" $prefix/foo |
-	git update-ref --stdin
-'
-
-test_expect_success REFFILES 'empty directory should not fool 0-arg delete' '
-	prefix=refs/e-delete-0 &&
-	git update-ref $prefix/foo $C &&
-	git pack-refs --all &&
-	mkdir -p .git/$prefix/foo/bar/baz &&
-	printf "delete %s\n" $prefix/foo |
-	git update-ref --stdin
-'
-
-test_expect_success REFFILES 'empty directory should not fool 1-arg delete' '
-	prefix=refs/e-delete-1 &&
-	git update-ref $prefix/foo $C &&
-	git pack-refs --all &&
-	mkdir -p .git/$prefix/foo/bar/baz &&
-	printf "delete %s $C\n" $prefix/foo |
-	git update-ref --stdin
-'
-
-test_expect_success REFFILES 'D/F conflict prevents add long + delete short' '
-	df_test refs/df-al-ds --add-del foo/bar foo
-'
-
-test_expect_success REFFILES 'D/F conflict prevents add short + delete long' '
-	df_test refs/df-as-dl --add-del foo foo/bar
-'
-
-test_expect_success REFFILES 'D/F conflict prevents delete long + add short' '
-	df_test refs/df-dl-as --del-add foo/bar foo
-'
-
-test_expect_success REFFILES 'D/F conflict prevents delete short + add long' '
-	df_test refs/df-ds-al --del-add foo foo/bar
-'
-
-test_expect_success REFFILES 'D/F conflict prevents add long + delete short packed' '
-	df_test refs/df-al-dsp --pack --add-del foo/bar foo
-'
-
-test_expect_success REFFILES 'D/F conflict prevents add short + delete long packed' '
-	df_test refs/df-as-dlp --pack --add-del foo foo/bar
-'
-
-test_expect_success REFFILES 'D/F conflict prevents delete long packed + add short' '
-	df_test refs/df-dlp-as --pack --del-add foo/bar foo
-'
-
-test_expect_success REFFILES 'D/F conflict prevents delete short packed + add long' '
-	df_test refs/df-dsp-al --pack --del-add foo foo/bar
-'
-
-# Try some combinations involving symbolic refs...
-
-test_expect_success REFFILES 'D/F conflict prevents indirect add long + delete short' '
-	df_test refs/df-ial-ds --sym-add --add-del foo/bar foo
-'
-
-test_expect_success REFFILES 'D/F conflict prevents indirect add long + indirect delete short' '
-	df_test refs/df-ial-ids --sym-add --sym-del --add-del foo/bar foo
-'
-
-test_expect_success REFFILES 'D/F conflict prevents indirect add short + indirect delete long' '
-	df_test refs/df-ias-idl --sym-add --sym-del --add-del foo foo/bar
-'
-
-test_expect_success REFFILES 'D/F conflict prevents indirect delete long + indirect add short' '
-	df_test refs/df-idl-ias --sym-add --sym-del --del-add foo/bar foo
-'
-
-test_expect_success REFFILES 'D/F conflict prevents indirect add long + delete short packed' '
-	df_test refs/df-ial-dsp --sym-add --pack --add-del foo/bar foo
-'
-
-test_expect_success REFFILES 'D/F conflict prevents indirect add long + indirect delete short packed' '
-	df_test refs/df-ial-idsp --sym-add --sym-del --pack --add-del foo/bar foo
-'
-
-test_expect_success REFFILES 'D/F conflict prevents add long + indirect delete short packed' '
-	df_test refs/df-al-idsp --sym-del --pack --add-del foo/bar foo
-'
-
-test_expect_success REFFILES 'D/F conflict prevents indirect delete long packed + indirect add short' '
-	df_test refs/df-idlp-ias --sym-add --sym-del --pack --del-add foo/bar foo
-'
-
 # Test various errors when reading the old values of references...
 
 test_expect_success 'missing old value blocks update' '
@@ -468,169 +255,4 @@ test_expect_success 'incorrect old value blocks indirect no-deref delete' '
 	test_cmp expected output.err
 '
 
-test_expect_success REFFILES 'non-empty directory blocks create' '
-	prefix=refs/ne-create &&
-	mkdir -p .git/$prefix/foo/bar &&
-	: >.git/$prefix/foo/bar/baz.lock &&
-	test_when_finished "rm -f .git/$prefix/foo/bar/baz.lock" &&
-	cat >expected <<-EOF &&
-	fatal: cannot lock ref $SQ$prefix/foo$SQ: there is a non-empty directory $SQ.git/$prefix/foo$SQ blocking reference $SQ$prefix/foo$SQ
-	EOF
-	printf "%s\n" "update $prefix/foo $C" |
-	test_must_fail git update-ref --stdin 2>output.err &&
-	test_cmp expected output.err &&
-	cat >expected <<-EOF &&
-	fatal: cannot lock ref $SQ$prefix/foo$SQ: unable to resolve reference $SQ$prefix/foo$SQ
-	EOF
-	printf "%s\n" "update $prefix/foo $D $C" |
-	test_must_fail git update-ref --stdin 2>output.err &&
-	test_cmp expected output.err
-'
-
-test_expect_success REFFILES 'broken reference blocks create' '
-	prefix=refs/broken-create &&
-	mkdir -p .git/$prefix &&
-	echo "gobbledigook" >.git/$prefix/foo &&
-	test_when_finished "rm -f .git/$prefix/foo" &&
-	cat >expected <<-EOF &&
-	fatal: cannot lock ref $SQ$prefix/foo$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken
-	EOF
-	printf "%s\n" "update $prefix/foo $C" |
-	test_must_fail git update-ref --stdin 2>output.err &&
-	test_cmp expected output.err &&
-	cat >expected <<-EOF &&
-	fatal: cannot lock ref $SQ$prefix/foo$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken
-	EOF
-	printf "%s\n" "update $prefix/foo $D $C" |
-	test_must_fail git update-ref --stdin 2>output.err &&
-	test_cmp expected output.err
-'
-
-test_expect_success REFFILES 'non-empty directory blocks indirect create' '
-	prefix=refs/ne-indirect-create &&
-	git symbolic-ref $prefix/symref $prefix/foo &&
-	mkdir -p .git/$prefix/foo/bar &&
-	: >.git/$prefix/foo/bar/baz.lock &&
-	test_when_finished "rm -f .git/$prefix/foo/bar/baz.lock" &&
-	cat >expected <<-EOF &&
-	fatal: cannot lock ref $SQ$prefix/symref$SQ: there is a non-empty directory $SQ.git/$prefix/foo$SQ blocking reference $SQ$prefix/foo$SQ
-	EOF
-	printf "%s\n" "update $prefix/symref $C" |
-	test_must_fail git update-ref --stdin 2>output.err &&
-	test_cmp expected output.err &&
-	cat >expected <<-EOF &&
-	fatal: cannot lock ref $SQ$prefix/symref$SQ: unable to resolve reference $SQ$prefix/foo$SQ
-	EOF
-	printf "%s\n" "update $prefix/symref $D $C" |
-	test_must_fail git update-ref --stdin 2>output.err &&
-	test_cmp expected output.err
-'
-
-test_expect_success REFFILES 'broken reference blocks indirect create' '
-	prefix=refs/broken-indirect-create &&
-	git symbolic-ref $prefix/symref $prefix/foo &&
-	echo "gobbledigook" >.git/$prefix/foo &&
-	test_when_finished "rm -f .git/$prefix/foo" &&
-	cat >expected <<-EOF &&
-	fatal: cannot lock ref $SQ$prefix/symref$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken
-	EOF
-	printf "%s\n" "update $prefix/symref $C" |
-	test_must_fail git update-ref --stdin 2>output.err &&
-	test_cmp expected output.err &&
-	cat >expected <<-EOF &&
-	fatal: cannot lock ref $SQ$prefix/symref$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken
-	EOF
-	printf "%s\n" "update $prefix/symref $D $C" |
-	test_must_fail git update-ref --stdin 2>output.err &&
-	test_cmp expected output.err
-'
-
-test_expect_success REFFILES 'no bogus intermediate values during delete' '
-	prefix=refs/slow-transaction &&
-	# Set up a reference with differing loose and packed versions:
-	git update-ref $prefix/foo $C &&
-	git pack-refs --all &&
-	git update-ref $prefix/foo $D &&
-	# Now try to update the reference, but hold the `packed-refs` lock
-	# for a while to see what happens while the process is blocked:
-	: >.git/packed-refs.lock &&
-	test_when_finished "rm -f .git/packed-refs.lock" &&
-	{
-		# Note: the following command is intentionally run in the
-		# background. We increase the timeout so that `update-ref`
-		# attempts to acquire the `packed-refs` lock for much longer
-		# than it takes for us to do the check then delete it:
-		git -c core.packedrefstimeout=30000 update-ref -d $prefix/foo &
-	} &&
-	pid2=$! &&
-	# Give update-ref plenty of time to get to the point where it tries
-	# to lock packed-refs:
-	sleep 1 &&
-	# Make sure that update-ref did not complete despite the lock:
-	kill -0 $pid2 &&
-	# Verify that the reference still has its old value:
-	sha1=$(git rev-parse --verify --quiet $prefix/foo || echo undefined) &&
-	case "$sha1" in
-	$D)
-		# This is what we hope for; it means that nothing
-		# user-visible has changed yet.
-		: ;;
-	undefined)
-		# This is not correct; it means the deletion has happened
-		# already even though update-ref should not have been
-		# able to acquire the lock yet.
-		echo "$prefix/foo deleted prematurely" &&
-		break
-		;;
-	$C)
-		# This value should never be seen. Probably the loose
-		# reference has been deleted but the packed reference
-		# is still there:
-		echo "$prefix/foo incorrectly observed to be C" &&
-		break
-		;;
-	*)
-		# WTF?
-		echo "unexpected value observed for $prefix/foo: $sha1" &&
-		break
-		;;
-	esac >out &&
-	rm -f .git/packed-refs.lock &&
-	wait $pid2 &&
-	test_must_be_empty out &&
-	test_must_fail git rev-parse --verify --quiet $prefix/foo
-'
-
-test_expect_success REFFILES 'delete fails cleanly if packed-refs file is locked' '
-	prefix=refs/locked-packed-refs &&
-	# Set up a reference with differing loose and packed versions:
-	git update-ref $prefix/foo $C &&
-	git pack-refs --all &&
-	git update-ref $prefix/foo $D &&
-	git for-each-ref $prefix >unchanged &&
-	# Now try to delete it while the `packed-refs` lock is held:
-	: >.git/packed-refs.lock &&
-	test_when_finished "rm -f .git/packed-refs.lock" &&
-	test_must_fail git update-ref -d $prefix/foo >out 2>err &&
-	git for-each-ref $prefix >actual &&
-	test_grep "Unable to create $SQ.*packed-refs.lock$SQ: " err &&
-	test_cmp unchanged actual
-'
-
-test_expect_success REFFILES 'delete fails cleanly if packed-refs.new write fails' '
-	# Setup and expectations are similar to the test above.
-	prefix=refs/failed-packed-refs &&
-	git update-ref $prefix/foo $C &&
-	git pack-refs --all &&
-	git update-ref $prefix/foo $D &&
-	git for-each-ref $prefix >unchanged &&
-	# This should not happen in practice, but it is an easy way to get a
-	# reliable error (we open with create_tempfile(), which uses O_EXCL).
-	: >.git/packed-refs.new &&
-	test_when_finished "rm -f .git/packed-refs.new" &&
-	test_must_fail git update-ref -d $prefix/foo &&
-	git for-each-ref $prefix >actual &&
-	test_cmp unchanged actual
-'
-
 test_done
-- 
gitgitgadget

[PATCH 05/12] t1405: move reffiles specific tests to t0600

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

Move this test to t0600 with other reffiles specific tests since it is
reffiles specific in that it looks into the loose refs directory for an
assertion.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t0600-reffiles-backend.sh | 8 ++++++++
 t/t1405-main-ref-store.sh   | 8 --------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index 332c8cbc004..53ac4b9b5b8 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -92,6 +92,14 @@ test_expect_success 'setup' '
 	E=$(git rev-parse HEAD)
 '
 
+test_expect_success 'pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE)' '
+	N=`find .git/refs -type f | wc -l` &&
+	test "$N" != 0 &&
+	test-tool ref-store main pack-refs PACK_REFS_PRUNE,PACK_REFS_ALL &&
+	N=`find .git/refs -type f` &&
+	test -z "$N"
+'
+
 test_expect_success 'empty directory should not fool rev-parse' '
 	prefix=refs/e-rev-parse &&
 	git update-ref $prefix/foo $C &&
diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
index 62c1eadb190..976bd71efb5 100755
--- a/t/t1405-main-ref-store.sh
+++ b/t/t1405-main-ref-store.sh
@@ -15,14 +15,6 @@ test_expect_success 'setup' '
 	test_commit one
 '
 
-test_expect_success REFFILES 'pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE)' '
-	N=`find .git/refs -type f | wc -l` &&
-	test "$N" != 0 &&
-	$RUN pack-refs PACK_REFS_PRUNE,PACK_REFS_ALL &&
-	N=`find .git/refs -type f` &&
-	test -z "$N"
-'
-
 test_expect_success 'create_symref(FOO, refs/heads/main)' '
 	$RUN create-symref FOO refs/heads/main nothing &&
 	echo refs/heads/main >expected &&
-- 
gitgitgadget

[PATCH 06/12] t1406: move reffiles specific tests to t0600

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

Move this test to t0600 with the rest of the tests that are specific to
reffiles. This test reaches into reflog directories manually, and so are
specific to reffiles.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t0600-reffiles-backend.sh   | 48 +++++++++++++++++++++++++++++++++++
 t/t1407-worktree-ref-store.sh | 37 ---------------------------
 2 files changed, 48 insertions(+), 37 deletions(-)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index 53ac4b9b5b8..09fbe312092 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -403,4 +403,52 @@ test_expect_success 'delete fails cleanly if packed-refs.new write fails' '
 	test_cmp unchanged actual
 '
 
+RWT="test-tool ref-store worktree:wt"
+RMAIN="test-tool ref-store worktree:main"
+
+test_expect_success 'setup worktree' '
+	test_commit first &&
+	git worktree add -b wt-main wt &&
+	(
+		cd wt &&
+		test_commit second
+	)
+'
+
+# Some refs (refs/bisect/*, pseudorefs) are kept per worktree, so they should
+# only appear in the for-each-reflog output if it is called from the correct
+# worktree, which is exercised in this test. This test is poorly written for
+# mulitple reasons: 1) it creates invalidly formatted log entres. 2) it uses
+# direct FS access for creating the reflogs. 3) PSEUDO-WT and refs/bisect/random
+# do not create reflogs by default, so it is not testing a realistic scenario.
+test_expect_success 'for_each_reflog()' '
+	echo $ZERO_OID > .git/logs/PSEUDO-MAIN &&
+	mkdir -p     .git/logs/refs/bisect &&
+	echo $ZERO_OID > .git/logs/refs/bisect/random &&
+
+	echo $ZERO_OID > .git/worktrees/wt/logs/PSEUDO-WT &&
+	mkdir -p     .git/worktrees/wt/logs/refs/bisect &&
+	echo $ZERO_OID > .git/worktrees/wt/logs/refs/bisect/wt-random &&
+
+	$RWT for-each-reflog | cut -d" " -f 2- | sort >actual &&
+	cat >expected <<-\EOF &&
+	HEAD 0x1
+	PSEUDO-WT 0x0
+	refs/bisect/wt-random 0x0
+	refs/heads/main 0x0
+	refs/heads/wt-main 0x0
+	EOF
+	test_cmp expected actual &&
+
+	$RMAIN for-each-reflog | cut -d" " -f 2- | sort >actual &&
+	cat >expected <<-\EOF &&
+	HEAD 0x1
+	PSEUDO-MAIN 0x0
+	refs/bisect/random 0x0
+	refs/heads/main 0x0
+	refs/heads/wt-main 0x0
+	EOF
+	test_cmp expected actual
+'
+
 test_done
diff --git a/t/t1407-worktree-ref-store.sh b/t/t1407-worktree-ref-store.sh
index 05b1881c591..48b1c92a414 100755
--- a/t/t1407-worktree-ref-store.sh
+++ b/t/t1407-worktree-ref-store.sh
@@ -53,41 +53,4 @@ test_expect_success 'create_symref(FOO, refs/heads/main)' '
 	test_cmp expected actual
 '
 
-# Some refs (refs/bisect/*, pseudorefs) are kept per worktree, so they should
-# only appear in the for-each-reflog output if it is called from the correct
-# worktree, which is exercised in this test. This test is poorly written (and
-# therefore marked REFFILES) for mulitple reasons: 1) it creates invalidly
-# formatted log entres. 2) it uses direct FS access for creating the reflogs. 3)
-# PSEUDO-WT and refs/bisect/random do not create reflogs by default, so it is
-# not testing a realistic scenario.
-test_expect_success REFFILES 'for_each_reflog()' '
-	echo $ZERO_OID > .git/logs/PSEUDO-MAIN &&
-	mkdir -p     .git/logs/refs/bisect &&
-	echo $ZERO_OID > .git/logs/refs/bisect/random &&
-
-	echo $ZERO_OID > .git/worktrees/wt/logs/PSEUDO-WT &&
-	mkdir -p     .git/worktrees/wt/logs/refs/bisect &&
-	echo $ZERO_OID > .git/worktrees/wt/logs/refs/bisect/wt-random &&
-
-	$RWT for-each-reflog | cut -d" " -f 2- | sort >actual &&
-	cat >expected <<-\EOF &&
-	HEAD 0x1
-	PSEUDO-WT 0x0
-	refs/bisect/wt-random 0x0
-	refs/heads/main 0x0
-	refs/heads/wt-main 0x0
-	EOF
-	test_cmp expected actual &&
-
-	$RMAIN for-each-reflog | cut -d" " -f 2- | sort >actual &&
-	cat >expected <<-\EOF &&
-	HEAD 0x1
-	PSEUDO-MAIN 0x0
-	refs/bisect/random 0x0
-	refs/heads/main 0x0
-	refs/heads/wt-main 0x0
-	EOF
-	test_cmp expected actual
-'
-
 test_done
-- 
gitgitgadget

[PATCH 07/12] t1410: move reffiles specific tests to t0600

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

Move these tests to t0600 with other reffiles specific tests since they
do things like take a lock on an individual ref, and write directly into
the reflog refs

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t0600-reffiles-backend.sh | 51 +++++++++++++++++++++++++++++++++++++
 t/t1410-reflog.sh           | 42 ------------------------------
 2 files changed, 51 insertions(+), 42 deletions(-)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index 09fbe312092..0b28a2cc5ea 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -451,4 +451,55 @@ test_expect_success 'for_each_reflog()' '
 	test_cmp expected actual
 '
 
+# Triggering the bug detected by this test requires a newline to fall
+# exactly BUFSIZ-1 bytes from the end of the file. We don't know
+# what that value is, since it's platform dependent. However, if
+# we choose some value N, we also catch any D which divides N evenly
+# (since we will read backwards in chunks of D). So we choose 8K,
+# which catches glibc (with an 8K BUFSIZ) and *BSD (1K).
+#
+# Each line is 114 characters, so we need 75 to still have a few before the
+# last 8K. The 89-character padding on the final entry lines up our
+# newline exactly.
+test_expect_success SHA1 'parsing reverse reflogs at BUFSIZ boundaries' '
+	git checkout -b reflogskip &&
+	zf=$(test_oid zero_2) &&
+	ident="abc <xyz> 0000000001 +0000" &&
+	for i in $(test_seq 1 75); do
+		printf "$zf%02d $zf%02d %s\t" $i $(($i+1)) "$ident" &&
+		if test $i = 75; then
+			for j in $(test_seq 1 89); do
+				printf X || return 1
+			done
+		else
+			printf X
+		fi &&
+		printf "\n" || return 1
+	done >.git/logs/refs/heads/reflogskip &&
+	git rev-parse reflogskip@{73} >actual &&
+	echo ${zf}03 >expect &&
+	test_cmp expect actual
+'
+
+# This test takes a lock on an individual ref; this is not supported in
+# reftable.
+test_expect_success 'reflog expire operates on symref not referrent' '
+	git branch --create-reflog the_symref &&
+	git branch --create-reflog referrent &&
+	git update-ref referrent HEAD &&
+	git symbolic-ref refs/heads/the_symref refs/heads/referrent &&
+	test_when_finished "rm -f .git/refs/heads/referrent.lock" &&
+	touch .git/refs/heads/referrent.lock &&
+	git reflog expire --expire=all the_symref
+'
+
+test_expect_success 'empty reflog' '
+	test_when_finished "rm -rf empty" &&
+	git init empty &&
+	test_commit -C empty A &&
+	>empty/.git/logs/refs/heads/foo &&
+	git -C empty reflog expire --all 2>err &&
+	test_must_be_empty err
+'
+
 test_done
diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
index a0ff8d51f04..d2f5f42e674 100755
--- a/t/t1410-reflog.sh
+++ b/t/t1410-reflog.sh
@@ -354,36 +354,6 @@ test_expect_success 'stale dirs do not cause d/f conflicts (reflogs off)' '
 	test_must_be_empty actual
 '
 
-# Triggering the bug detected by this test requires a newline to fall
-# exactly BUFSIZ-1 bytes from the end of the file. We don't know
-# what that value is, since it's platform dependent. However, if
-# we choose some value N, we also catch any D which divides N evenly
-# (since we will read backwards in chunks of D). So we choose 8K,
-# which catches glibc (with an 8K BUFSIZ) and *BSD (1K).
-#
-# Each line is 114 characters, so we need 75 to still have a few before the
-# last 8K. The 89-character padding on the final entry lines up our
-# newline exactly.
-test_expect_success REFFILES,SHA1 'parsing reverse reflogs at BUFSIZ boundaries' '
-	git checkout -b reflogskip &&
-	zf=$(test_oid zero_2) &&
-	ident="abc <xyz> 0000000001 +0000" &&
-	for i in $(test_seq 1 75); do
-		printf "$zf%02d $zf%02d %s\t" $i $(($i+1)) "$ident" &&
-		if test $i = 75; then
-			for j in $(test_seq 1 89); do
-				printf X || return 1
-			done
-		else
-			printf X
-		fi &&
-		printf "\n" || return 1
-	done >.git/logs/refs/heads/reflogskip &&
-	git rev-parse reflogskip@{73} >actual &&
-	echo ${zf}03 >expect &&
-	test_cmp expect actual
-'
-
 test_expect_success 'no segfaults for reflog containing non-commit sha1s' '
 	git update-ref --create-reflog -m "Creating ref" \
 		refs/tests/tree-in-reflog HEAD &&
@@ -397,18 +367,6 @@ test_expect_failure 'reflog with non-commit entries displays all entries' '
 	test_line_count = 3 actual
 '
 
-# This test takes a lock on an individual ref; this is not supported in
-# reftable.
-test_expect_success REFFILES 'reflog expire operates on symref not referrent' '
-	git branch --create-reflog the_symref &&
-	git branch --create-reflog referrent &&
-	git update-ref referrent HEAD &&
-	git symbolic-ref refs/heads/the_symref refs/heads/referrent &&
-	test_when_finished "rm -f .git/refs/heads/referrent.lock" &&
-	touch .git/refs/heads/referrent.lock &&
-	git reflog expire --expire=all the_symref
-'
-
 test_expect_success 'continue walking past root commits' '
 	git init orphanage &&
 	(
-- 
gitgitgadget

[PATCH 08/12] t1415: move reffiles specific tests to t0600

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

Move this test into t0600 with other reffiles specific tests since it
checks for individua loose refs and thus is specific to the reffiles
backend.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t0600-reffiles-backend.sh | 20 ++++++++++++++++++++
 t/t1415-worktree-refs.sh    | 11 -----------
 2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index 0b28a2cc5ea..8526e5cf987 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -502,4 +502,24 @@ test_expect_success 'empty reflog' '
 	test_must_be_empty err
 '
 
+# The 'packed-refs' file is stored directly in .git/. This means it is global
+# to the repository, and can only contain refs that are shared across all
+# worktrees.
+test_expect_success 'refs/worktree must not be packed' '
+	test_commit initial &&
+	test_commit wt1 &&
+	test_commit wt2 &&
+	git worktree add wt1 wt1 &&
+	git worktree add wt2 wt2 &&
+	git checkout initial &&
+	git update-ref refs/worktree/foo HEAD &&
+	git -C wt1 update-ref refs/worktree/foo HEAD &&
+	git -C wt2 update-ref refs/worktree/foo HEAD &&
+	git pack-refs --all &&
+	test_path_is_missing .git/refs/tags/wt1 &&
+	test_path_is_file .git/refs/worktree/foo &&
+	test_path_is_file .git/worktrees/wt1/refs/worktree/foo &&
+	test_path_is_file .git/worktrees/wt2/refs/worktree/foo
+'
+
 test_done
diff --git a/t/t1415-worktree-refs.sh b/t/t1415-worktree-refs.sh
index 3b531842dd4..eb4eec8becb 100755
--- a/t/t1415-worktree-refs.sh
+++ b/t/t1415-worktree-refs.sh
@@ -17,17 +17,6 @@ test_expect_success 'setup' '
 	git -C wt2 update-ref refs/worktree/foo HEAD
 '
 
-# The 'packed-refs' file is stored directly in .git/. This means it is global
-# to the repository, and can only contain refs that are shared across all
-# worktrees.
-test_expect_success REFFILES 'refs/worktree must not be packed' '
-	git pack-refs --all &&
-	test_path_is_missing .git/refs/tags/wt1 &&
-	test_path_is_file .git/refs/worktree/foo &&
-	test_path_is_file .git/worktrees/wt1/refs/worktree/foo &&
-	test_path_is_file .git/worktrees/wt2/refs/worktree/foo
-'
-
 test_expect_success 'refs/worktree are per-worktree' '
 	test_cmp_rev worktree/foo initial &&
 	( cd wt1 && test_cmp_rev worktree/foo wt1 ) &&
-- 
gitgitgadget

[PATCH 09/12] t1503: move reffiles specific tests to t0600

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

Move this test to t0600 with other reffiles specific tests since it
checks for loose refs and is specific to the reffiles backend.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t0600-reffiles-backend.sh | 5 +++++
 t/t1503-rev-parse-verify.sh | 5 -----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index 8526e5cf987..704b73fdc54 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -522,4 +522,9 @@ test_expect_success 'refs/worktree must not be packed' '
 	test_path_is_file .git/worktrees/wt2/refs/worktree/foo
 '
 
+test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '
+       ln -s does-not-exist .git/refs/heads/broken &&
+       test_must_fail git rev-parse --verify broken
+'
+
 test_done
diff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh
index bc136833c10..79df65ec7f6 100755
--- a/t/t1503-rev-parse-verify.sh
+++ b/t/t1503-rev-parse-verify.sh
@@ -144,11 +144,6 @@ test_expect_success 'main@{n} for various n' '
 	test_must_fail git rev-parse --verify main@{$Np1}
 '
 
-test_expect_success SYMLINKS,REFFILES 'ref resolution not confused by broken symlinks' '
-	ln -s does-not-exist .git/refs/heads/broken &&
-	test_must_fail git rev-parse --verify broken
-'
-
 test_expect_success 'options can appear after --verify' '
 	git rev-parse --verify HEAD >expect &&
 	git rev-parse --verify -q HEAD >actual &&
-- 
gitgitgadget

[PATCH 10/12] t3903: move reffiles specific tests to t0600

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

Move this test into t0600 with other reffiles specific tests since it
modifies reflog refs manually and thus is specific to the reffiles
backend.

This change also consolidates setup_stash() into test-lib-functions.sh

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t0600-reffiles-backend.sh | 27 +++++++++++++++++++++++
 t/t3903-stash.sh            | 43 -------------------------------------
 t/test-lib-functions.sh     | 16 ++++++++++++++
 3 files changed, 43 insertions(+), 43 deletions(-)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index 704b73fdc54..bee61b2d19d 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -527,4 +527,31 @@ test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '
        test_must_fail git rev-parse --verify broken
 '
 
+test_expect_success 'drop stash reflog updates refs/stash with rewrite' '
+	git init repo &&
+	(
+		cd repo &&
+		setup_stash
+	) &&
+	echo 9 >repo/file &&
+
+	old_oid="$(git -C repo rev-parse stash@{0})" &&
+	git -C repo stash &&
+	new_oid="$(git -C repo rev-parse stash@{0})" &&
+
+	cat >expect <<-EOF &&
+	$(test_oid zero) $old_oid
+	$old_oid $new_oid
+	EOF
+	cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
+	test_cmp expect actual &&
+
+	git -C repo stash drop stash@{1} &&
+	cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
+	cat >expect <<-EOF &&
+	$(test_oid zero) $new_oid
+	EOF
+	test_cmp expect actual
+'
+
 test_done
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 34faeac3f1c..0b0e7b19fdc 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -42,22 +42,6 @@ diff_cmp () {
 	rm -f "$1.compare" "$2.compare"
 }
 
-setup_stash() {
-	echo 1 >file &&
-	git add file &&
-	echo unrelated >other-file &&
-	git add other-file &&
-	test_tick &&
-	git commit -m initial &&
-	echo 2 >file &&
-	git add file &&
-	echo 3 >file &&
-	test_tick &&
-	git stash &&
-	git diff-files --quiet &&
-	git diff-index --cached --quiet HEAD
-}
-
 test_expect_success 'stash some dirty working directory' '
 	setup_stash
 '
@@ -200,33 +184,6 @@ test_expect_success 'drop stash reflog updates refs/stash' '
 	test_cmp expect actual
 '
 
-test_expect_success REFFILES 'drop stash reflog updates refs/stash with rewrite' '
-	git init repo &&
-	(
-		cd repo &&
-		setup_stash
-	) &&
-	echo 9 >repo/file &&
-
-	old_oid="$(git -C repo rev-parse stash@{0})" &&
-	git -C repo stash &&
-	new_oid="$(git -C repo rev-parse stash@{0})" &&
-
-	cat >expect <<-EOF &&
-	$(test_oid zero) $old_oid
-	$old_oid $new_oid
-	EOF
-	cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
-	test_cmp expect actual &&
-
-	git -C repo stash drop stash@{1} &&
-	cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
-	cat >expect <<-EOF &&
-	$(test_oid zero) $new_oid
-	EOF
-	test_cmp expect actual
-'
-
 test_expect_success 'stash pop' '
 	git reset --hard &&
 	git stash pop &&
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index b5eaf7fdc11..68a6c8402d0 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1958,3 +1958,19 @@ test_trailing_hash () {
 		test-tool hexdump |
 		sed "s/ //g"
 }
+
+# Stash some changes
+setup_stash() { echo 1 >file &&
+	git add file &&
+	echo unrelated >other-file &&
+	git add other-file &&
+	test_tick &&
+	git commit -m initial &&
+	echo 2 >file &&
+	git add file &&
+	echo 3 >file &&
+	test_tick &&
+	git stash &&
+	git diff-files --quiet &&
+	git diff-index --cached --quiet HEAD
+}
-- 
gitgitgadget

[PATCH 11/12] t4202: move reffiles specific tests to t0600

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

Move two tests into t0600 since they write loose reflog refs manually
and thus are specific to the reffiles backend.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t0600-reffiles-backend.sh | 17 +++++++++++++++++
 t/t4202-log.sh              | 17 -----------------
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index bee61b2d19d..c88576dfea5 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -554,4 +554,21 @@ test_expect_success 'drop stash reflog updates refs/stash with rewrite' '
 	test_cmp expect actual
 '
 
+test_expect_success 'log diagnoses bogus HEAD hash' '
+	git init empty &&
+	test_when_finished "rm -rf empty" &&
+	echo 1234abcd >empty/.git/refs/heads/main &&
+	test_must_fail git -C empty log 2>stderr &&
+	test_grep broken stderr
+'
+
+test_expect_success 'log diagnoses bogus HEAD symref' '
+	git init empty &&
+	test-tool -C empty ref-store main create-symref HEAD refs/heads/invalid.lock &&
+	test_must_fail git -C empty log 2>stderr &&
+	test_grep broken stderr &&
+	test_must_fail git -C empty log --default totally-bogus 2>stderr &&
+	test_grep broken stderr
+'
+
 test_done
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index ddd205f98ab..60fe60d7610 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -2255,23 +2255,6 @@ test_expect_success 'log on empty repo fails' '
 	test_grep does.not.have.any.commits stderr
 '
 
-test_expect_success REFFILES 'log diagnoses bogus HEAD hash' '
-	git init empty &&
-	test_when_finished "rm -rf empty" &&
-	echo 1234abcd >empty/.git/refs/heads/main &&
-	test_must_fail git -C empty log 2>stderr &&
-	test_grep broken stderr
-'
-
-test_expect_success REFFILES 'log diagnoses bogus HEAD symref' '
-	git init empty &&
-	test-tool -C empty ref-store main create-symref HEAD refs/heads/invalid.lock &&
-	test_must_fail git -C empty log 2>stderr &&
-	test_grep broken stderr &&
-	test_must_fail git -C empty log --default totally-bogus 2>stderr &&
-	test_grep broken stderr
-'
-
 test_expect_success 'log does not default to HEAD when rev input is given' '
 	git log --branches=does-not-exist >actual &&
 	test_must_be_empty actual
-- 
gitgitgadget

[PATCH 12/12] t5312: move reffiles specific tests to t0600

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

Move a few tests into t0600 since they specifically test the packed-refs
file and thus are specific to the reffiles backend.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t0600-reffiles-backend.sh | 30 ++++++++++++++++++++++++++++++
 t/t5312-prune-corruption.sh | 26 --------------------------
 2 files changed, 30 insertions(+), 26 deletions(-)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index c88576dfea5..190155f592d 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -571,4 +571,34 @@ test_expect_success 'log diagnoses bogus HEAD symref' '
 	test_grep broken stderr
 '
 
+# we do not want to count on running pack-refs to
+# actually pack it, as it is perfectly reasonable to
+# skip processing a broken ref
+test_expect_success 'create packed-refs file with broken ref' '
+	test_tick && git commit --allow-empty -m one &&
+	recoverable=$(git rev-parse HEAD) &&
+	test_tick && git commit --allow-empty -m two &&
+	missing=$(git rev-parse HEAD) &&
+	rm -f .git/refs/heads/main &&
+	cat >.git/packed-refs <<-EOF &&
+	$missing refs/heads/main
+	$recoverable refs/heads/other
+	EOF
+	echo $missing >expect &&
+	git rev-parse refs/heads/main >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'pack-refs does not silently delete broken packed ref' '
+	git pack-refs --all --prune &&
+	git rev-parse refs/heads/main >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success  'pack-refs does not drop broken refs during deletion' '
+	git update-ref -d refs/heads/other &&
+	git rev-parse refs/heads/main >actual &&
+	test_cmp expect actual
+'
+
 test_done
diff --git a/t/t5312-prune-corruption.sh b/t/t5312-prune-corruption.sh
index 230cb387122..d8d2e304687 100755
--- a/t/t5312-prune-corruption.sh
+++ b/t/t5312-prune-corruption.sh
@@ -111,30 +111,4 @@ test_expect_success 'pack-refs does not silently delete broken loose ref' '
 	test_cmp expect actual
 '
 
-# we do not want to count on running pack-refs to
-# actually pack it, as it is perfectly reasonable to
-# skip processing a broken ref
-test_expect_success REFFILES 'create packed-refs file with broken ref' '
-	rm -f .git/refs/heads/main &&
-	cat >.git/packed-refs <<-EOF &&
-	$missing refs/heads/main
-	$recoverable refs/heads/other
-	EOF
-	echo $missing >expect &&
-	git rev-parse refs/heads/main >actual &&
-	test_cmp expect actual
-'
-
-test_expect_success REFFILES 'pack-refs does not silently delete broken packed ref' '
-	git pack-refs --all --prune &&
-	git rev-parse refs/heads/main >actual &&
-	test_cmp expect actual
-'
-
-test_expect_success REFFILES  'pack-refs does not drop broken refs during deletion' '
-	git update-ref -d refs/heads/other &&
-	git rev-parse refs/heads/main >actual &&
-	test_cmp expect actual
-'
-
 test_done
-- 
gitgitgadget

Re: [PATCH 01/12] t3210: move to t0602

[Junio C Hamano]

"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: John Cai <johncai86@gmail.com>
>
> Move t3210 to t0602, since these tests are reffiles specific in that
> they modify loose refs manually. This is part of the effort to
> categorize these tests together based on the ref backend they test. When
> we upstream the reftable backend, we can add more tests to t06xx. This
> way, all tests that test specific ref backend behavior will be grouped
> together.

So, ... is the idea to have (1) majority of ref tests, against which
all backends ought to behave the same way, will be written in
backend agnostic way (e.g., we have seen some patches to stop
touching the filesystem .git/refs/ hierarchy manually), and (2) some
backend specific tests will be grouped in a small number of test
script files for each backend and they all will use t6xx numbrs?

OK.  Sounds like a good plan to me.




> Signed-off-by: John Cai <johncai86@gmail.com>
> ---
>  t/{t3210-pack-refs.sh => t0602-reffiles-pack-refs.sh} | 0
>  1 file changed, 0 insertions(+), 0 deletions(-)
>  rename t/{t3210-pack-refs.sh => t0602-reffiles-pack-refs.sh} (100%)
>
> diff --git a/t/t3210-pack-refs.sh b/t/t0602-reffiles-pack-refs.sh
> similarity index 100%
> rename from t/t3210-pack-refs.sh
> rename to t/t0602-reffiles-pack-refs.sh

Re: [PATCH 02/12] remove REFFILES prerequisite

[Junio C Hamano]

"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: John Cai <johncai86@gmail.com>
>
> These tests are compatible with the reftable backend and thus do not
> need the REFFILES prerequisite.

May want to give a bit more backstory here?  After all, 53af25e4
(t1405: mark test that checks existence as REFFILES, 2022-01-31) and
53af25e4 (t1405: mark test that checks existence as REFFILES,
2022-01-31) marked these tests to require REFFILES and they explain
the reason for doing so was exactly because the reftable backend did
not have the notion of "the reflog for this ref exists" that is
independent from "the reflog for this ref exists and has one or more
reflog records".  If your work on the reftable backend during the
past few years added support for "already exists, but there is no
entry yet" state for reflogs, that would be great, but it would make
sense to explain why they suddenly have become "compatible with the
reftable backend".

Thanks.

>
> Signed-off-by: John Cai <johncai86@gmail.com>
> ---
>  t/t1405-main-ref-store.sh  | 2 +-
>  t/t2017-checkout-orphan.sh | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
> index e4627cf1b61..62c1eadb190 100755
> --- a/t/t1405-main-ref-store.sh
> +++ b/t/t1405-main-ref-store.sh
> @@ -112,7 +112,7 @@ test_expect_success 'delete_reflog(HEAD)' '
>  	test_must_fail git reflog exists HEAD
>  '
>  
> -test_expect_success REFFILES 'create-reflog(HEAD)' '
> +test_expect_success 'create-reflog(HEAD)' '
>  	$RUN create-reflog HEAD &&
>  	git reflog exists HEAD
>  '
> diff --git a/t/t2017-checkout-orphan.sh b/t/t2017-checkout-orphan.sh
> index 947d1587ac8..a5c7358eeab 100755
> --- a/t/t2017-checkout-orphan.sh
> +++ b/t/t2017-checkout-orphan.sh
> @@ -86,7 +86,7 @@ test_expect_success '--orphan makes reflog by default' '
>  	git rev-parse --verify delta@{0}
>  '
>  
> -test_expect_success REFFILES '--orphan does not make reflog when core.logAllRefUpdates = false' '
> +test_expect_success '--orphan does not make reflog when core.logAllRefUpdates = false' '
>  	git checkout main &&
>  	git config core.logAllRefUpdates false &&
>  	git checkout --orphan epsilon &&

Re: [PATCH 03/12] t1414: convert test to use Git commands instead of writing refs manually

[Junio C Hamano]

"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:

>  # Create a situation where the reflog and ref database disagree about the latest
>  # state of HEAD.
> -test_expect_success REFFILES 'walk prefers reflog to ref tip' '
> +test_expect_success 'walk prefers reflog to ref tip' '
> +	test_commit A &&
> +	test_commit B &&
> +	git reflog delete HEAD@{0} &&
>  	head=$(git rev-parse HEAD) &&
> +	A=$(git rev-parse A) &&
>  
> +	echo $A >expect &&

You do not need an intermediate variable A, i.e.

	git rev-parse A >expect &&

would suffice.  Also it seems that $head is no longer used
because you do not manufacture a reflog entry yourself, so the two
assignments to $A and $head can be removed.

>  	git log -g --format=%H -1 >actual &&
>  	test_cmp expect actual
>  '

The resulting code makes the intent of the test much clearer.
Nicely done.

Re: [PATCH 00/12] Group reffiles tests

[Junio C Hamano]

"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:

> This series groups REFFILES specific tests together. These tests are
> currently grouped together across the test suite based on functionality.
> However, since they exercise low-level behavior specific to the refs backend
> being used (in these cases, the ref-files backend), group them together
> based on which refs backend they test. This way, in the near future when the
> reftables backend gets upstreamed we can add tests that exercise the
> reftables backend close by in the t06xx area.
>
> These patches also remove the REFFILES prerequisite, since all the tests in
> t06xx are reffiles specific.

As we already have REFFILES lazy prereq, even _before_ we enable the
reftable backend, I think that we should start t0600 and t0602 with

	. ./test-lib.sh
	if ! test_have_prereq REFFILES
	then
		skip_all='skipping reffiles specific tests'
		test_done
	fi

which is more in line with the existing convention.  It is more
efficient than "forcing t0600 and t0602 to run always with reffiles"
when you have a CI job that uses reftable for all tests and another
CI job that uses reffiles for all tests.

> In the near future, once the reftable backend is upstreamed, all
> the tests in t06xx will be forced to run with the reffiles
> backend.

Presumably if there are reftable backend specific tests, they will
also be given names out of t06xx range, right?  And then they will
be skipped when the test is not using reftable as the default ref
backend, using the REFTABLE prerequisite in a similar way as shown
above for REFFILES, right?

Thanks.

Re: [PATCH 02/12] remove REFFILES prerequisite

[Patrick Steinhardt]

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

On Wed, Jan 17, 2024 at 04:46:20PM -0800, Junio C Hamano wrote:
> "John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
> > From: John Cai <johncai86@gmail.com>
> >
> > These tests are compatible with the reftable backend and thus do not
> > need the REFFILES prerequisite.
> 
> May want to give a bit more backstory here?  After all, 53af25e4
> (t1405: mark test that checks existence as REFFILES, 2022-01-31) and
> 53af25e4 (t1405: mark test that checks existence as REFFILES,
> 2022-01-31) marked these tests to require REFFILES and they explain
> the reason for doing so was exactly because the reftable backend did
> not have the notion of "the reflog for this ref exists" that is
> independent from "the reflog for this ref exists and has one or more
> reflog records".  If your work on the reftable backend during the
> past few years added support for "already exists, but there is no
> entry yet" state for reflogs, that would be great, but it would make
> sense to explain why they suddenly have become "compatible with the
> reftable backend".

I don't know a lot about the history any why we initially didn't think
it would be compatible, mostly because there is no history of how the
reftable backend itself evolved over time. I can only say that when I
took over the effort that this indeed worked as expected by writing
"existence" markers into the reflog, where this existence marker is a
simple entry where both old and new object ID are set to the null OID.

Patrick

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

Re: [PATCH 01/12] t3210: move to t0602

[Patrick Steinhardt]

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

On Wed, Jan 17, 2024 at 04:40:10PM -0800, Junio C Hamano wrote:
> "John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
> > From: John Cai <johncai86@gmail.com>
> >
> > Move t3210 to t0602, since these tests are reffiles specific in that
> > they modify loose refs manually. This is part of the effort to
> > categorize these tests together based on the ref backend they test. When
> > we upstream the reftable backend, we can add more tests to t06xx. This
> > way, all tests that test specific ref backend behavior will be grouped
> > together.
> 
> So, ... is the idea to have (1) majority of ref tests, against which
> all backends ought to behave the same way, will be written in
> backend agnostic way (e.g., we have seen some patches to stop
> touching the filesystem .git/refs/ hierarchy manually), and (2) some
> backend specific tests will be grouped in a small number of test
> script files for each backend and they all will use t6xx numbrs?
> 
> OK.  Sounds like a good plan to me.

Yes, that's the plan. The backend specific tests will be free to also
exercise filesystem-level behaviour in order to pin down that things
work as expected. But once their behaviour is nailed down all other
generic tests should refrain from doing that to the best extent possible
and instead use Git commands to do their thing.

> > Signed-off-by: John Cai <johncai86@gmail.com>
> > ---
> >  t/{t3210-pack-refs.sh => t0602-reffiles-pack-refs.sh} | 0
> >  1 file changed, 0 insertions(+), 0 deletions(-)
> >  rename t/{t3210-pack-refs.sh => t0602-reffiles-pack-refs.sh} (100%)

Is there a reason why you picked t0602 instead of the not-yet-taken
t0601? If it's only because I use t0601 in my reftable integration
branch then I'd like us to pick t0601 here instead to avoid a weird gap.
I'll adapt accordingly and rename the reftable tests to have a t061x
prefix in that case so that they are nicely grouped together.

Patrick

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

Re: [PATCH 00/12] Group reffiles tests

[Patrick Steinhardt]

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

On Wed, Jan 17, 2024 at 05:17:17PM -0800, Junio C Hamano wrote:
> "John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
> > This series groups REFFILES specific tests together. These tests are
> > currently grouped together across the test suite based on functionality.
> > However, since they exercise low-level behavior specific to the refs backend
> > being used (in these cases, the ref-files backend), group them together
> > based on which refs backend they test. This way, in the near future when the
> > reftables backend gets upstreamed we can add tests that exercise the
> > reftables backend close by in the t06xx area.
> >
> > These patches also remove the REFFILES prerequisite, since all the tests in
> > t06xx are reffiles specific.
> 
> As we already have REFFILES lazy prereq, even _before_ we enable the
> reftable backend, I think that we should start t0600 and t0602 with
> 
> 	. ./test-lib.sh
> 	if ! test_have_prereq REFFILES
> 	then
> 		skip_all='skipping reffiles specific tests'
> 		test_done
> 	fi
> 
> which is more in line with the existing convention.  It is more
> efficient than "forcing t0600 and t0602 to run always with reffiles"
> when you have a CI job that uses reftable for all tests and another
> CI job that uses reffiles for all tests.

I think it depends. If we use the REFFILES prereq for the files-specific
tests, then we should likely also use the REFTABLE prereq for the
reftable-specific tests.

But that raises the question of whether we want to add a CI job that
exercises code with the reftable backend for every major platform
(Linux, macOS, Windows). If so then your proposal would be fine with me
as we make sure that things work alright on all of them. But if we think
that this would be too expensive then I'd like to at least have very
basic test coverage on all platforms by always running these
backend-specific tests.

Patrick

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

Re: [PATCH 01/12] t3210: move to t0602

[John Cai]

Hi Patrick,

On 18 Jan 2024, at 6:32, Patrick Steinhardt wrote:

> On Wed, Jan 17, 2024 at 04:40:10PM -0800, Junio C Hamano wrote:
>> "John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>
>>> From: John Cai <johncai86@gmail.com>
>>>
>>> Move t3210 to t0602, since these tests are reffiles specific in that
>>> they modify loose refs manually. This is part of the effort to
>>> categorize these tests together based on the ref backend they test. When
>>> we upstream the reftable backend, we can add more tests to t06xx. This
>>> way, all tests that test specific ref backend behavior will be grouped
>>> together.
>>
>> So, ... is the idea to have (1) majority of ref tests, against which
>> all backends ought to behave the same way, will be written in
>> backend agnostic way (e.g., we have seen some patches to stop
>> touching the filesystem .git/refs/ hierarchy manually), and (2) some
>> backend specific tests will be grouped in a small number of test
>> script files for each backend and they all will use t6xx numbrs?
>>
>> OK.  Sounds like a good plan to me.
>
> Yes, that's the plan. The backend specific tests will be free to also
> exercise filesystem-level behaviour in order to pin down that things
> work as expected. But once their behaviour is nailed down all other
> generic tests should refrain from doing that to the best extent possible
> and instead use Git commands to do their thing.
>
>>> Signed-off-by: John Cai <johncai86@gmail.com>
>>> ---
>>>  t/{t3210-pack-refs.sh => t0602-reffiles-pack-refs.sh} | 0
>>>  1 file changed, 0 insertions(+), 0 deletions(-)
>>>  rename t/{t3210-pack-refs.sh => t0602-reffiles-pack-refs.sh} (100%)
>
> Is there a reason why you picked t0602 instead of the not-yet-taken
> t0601? If it's only because I use t0601 in my reftable integration
> branch then I'd like us to pick t0601 here instead to avoid a weird gap.
> I'll adapt accordingly and rename the reftable tests to have a t061x
> prefix in that case so that they are nicely grouped together.

Yes if I remember correctly, that's the reason. I can move this to t0601 then,
thanks.

>
> Patrick

Re: [PATCH 00/12] Group reffiles tests

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> I think it depends. If we use the REFFILES prereq for the files-specific
> tests, then we should likely also use the REFTABLE prereq for the
> reftable-specific tests.

Correct.  I've assumed that as a given; while introducing any new
implementation of a subsystem that has widespread impact, we would
test things with the original and new implementations.  It happened
while we were moving "ort" to replace "recursive" as an internal
tree merge machinery, for example.  linux-TEST-vars job that is
available both in GitHub and GitLab CI is an example of a separate
job that runs everything with non-default configurations, and "use
reftable as the default backend" GIT_TEST_REFTABLE knob may be an
appropriate thing to set there.

> But that raises the question of whether we want to add a CI job that
> exercises code with the reftable backend for every major platform
> (Linux, macOS, Windows). If so then your proposal would be fine with me
> as we make sure that things work alright on all of them. But if we think
> that this would be too expensive then I'd like to at least have very
> basic test coverage on all platforms by always running these
> backend-specific tests.
>
> Patrick

Re: [PATCH 04/12] t1404: move reffiles specific tests to t0600

[Patrick Steinhardt]

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

On Wed, Jan 17, 2024 at 07:52:27PM +0000, John Cai via GitGitGadget wrote:
> From: John Cai <johncai86@gmail.com>
[snip]
> +test_expect_success 'D/F conflict prevents add long + delete short' '
> +	df_test refs/df-al-ds --add-del foo/bar foo
> +'

All of the tests using `df_test ()` pass with the reftable backend, the
only thing that's incompatible is that there is an additional prefix in
the "files" backend's error message. So I'd like to drop moving those
D/F conflict tests so that we can instead make them generic in another
iteration.

All the other tests where we verify how the "files" backend behaves when
there are empty directories in the way do make sense to become backend
specific though.

Patrick

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

Re: [PATCH 08/12] t1415: move reffiles specific tests to t0600

[Patrick Steinhardt]

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

On Wed, Jan 17, 2024 at 07:52:31PM +0000, John Cai via GitGitGadget wrote:
> From: John Cai <johncai86@gmail.com>
> 
> Move this test into t0600 with other reffiles specific tests since it
> checks for individua loose refs and thus is specific to the reffiles
> backend.
> 
> Signed-off-by: John Cai <johncai86@gmail.com>
> ---
>  t/t0600-reffiles-backend.sh | 20 ++++++++++++++++++++
>  t/t1415-worktree-refs.sh    | 11 -----------
>  2 files changed, 20 insertions(+), 11 deletions(-)
> 
> diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
> index 0b28a2cc5ea..8526e5cf987 100755
> --- a/t/t0600-reffiles-backend.sh
> +++ b/t/t0600-reffiles-backend.sh
> @@ -502,4 +502,24 @@ test_expect_success 'empty reflog' '
>  	test_must_be_empty err
>  '
>  
> +# The 'packed-refs' file is stored directly in .git/. This means it is global
> +# to the repository, and can only contain refs that are shared across all
> +# worktrees.
> +test_expect_success 'refs/worktree must not be packed' '
> +	test_commit initial &&
> +	test_commit wt1 &&
> +	test_commit wt2 &&
> +	git worktree add wt1 wt1 &&
> +	git worktree add wt2 wt2 &&
> +	git checkout initial &&
> +	git update-ref refs/worktree/foo HEAD &&
> +	git -C wt1 update-ref refs/worktree/foo HEAD &&
> +	git -C wt2 update-ref refs/worktree/foo HEAD &&
> +	git pack-refs --all &&
> +	test_path_is_missing .git/refs/tags/wt1 &&
> +	test_path_is_file .git/refs/worktree/foo &&
> +	test_path_is_file .git/worktrees/wt1/refs/worktree/foo &&
> +	test_path_is_file .git/worktrees/wt2/refs/worktree/foo
> +'

Given that this test exercises git-pack-refs(1), should we move it to
t0601-reffiles-pack-refs.sh instead?

Patrick

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

Re: [PATCH 10/12] t3903: move reffiles specific tests to t0600

[Patrick Steinhardt]

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

On Wed, Jan 17, 2024 at 07:52:33PM +0000, John Cai via GitGitGadget wrote:
> From: John Cai <johncai86@gmail.com>
> 
> Move this test into t0600 with other reffiles specific tests since it
> modifies reflog refs manually and thus is specific to the reffiles
> backend.
> 
> This change also consolidates setup_stash() into test-lib-functions.sh
> 
> Signed-off-by: John Cai <johncai86@gmail.com>
> ---
>  t/t0600-reffiles-backend.sh | 27 +++++++++++++++++++++++
>  t/t3903-stash.sh            | 43 -------------------------------------
>  t/test-lib-functions.sh     | 16 ++++++++++++++
>  3 files changed, 43 insertions(+), 43 deletions(-)
> 
> diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
> index 704b73fdc54..bee61b2d19d 100755
> --- a/t/t0600-reffiles-backend.sh
> +++ b/t/t0600-reffiles-backend.sh
> @@ -527,4 +527,31 @@ test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '
>         test_must_fail git rev-parse --verify broken
>  '
>  
> +test_expect_success 'drop stash reflog updates refs/stash with rewrite' '
> +	git init repo &&
> +	(
> +		cd repo &&
> +		setup_stash
> +	) &&
> +	echo 9 >repo/file &&
> +
> +	old_oid="$(git -C repo rev-parse stash@{0})" &&
> +	git -C repo stash &&
> +	new_oid="$(git -C repo rev-parse stash@{0})" &&
> +
> +	cat >expect <<-EOF &&
> +	$(test_oid zero) $old_oid
> +	$old_oid $new_oid
> +	EOF
> +	cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
> +	test_cmp expect actual &&
> +
> +	git -C repo stash drop stash@{1} &&
> +	cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
> +	cat >expect <<-EOF &&
> +	$(test_oid zero) $new_oid
> +	EOF
> +	test_cmp expect actual
> +'

I think that there is no need to make this backend-specific. What we're
testing here is that `git stash drop` is able to drop the latest reflog
entry. The calls to cut(1) are only used to verify that the contents of
the reflog entry look as expected while only verifying the old and new
object IDs.

So how about below patch to make it generic instead?

Patrick

-- >8 --

diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 34faeac3f1..3319240515 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -200,7 +200,7 @@ test_expect_success 'drop stash reflog updates refs/stash' '
 	test_cmp expect actual
 '
 
-test_expect_success REFFILES 'drop stash reflog updates refs/stash with rewrite' '
+test_expect_success 'drop stash reflog updates refs/stash with rewrite' '
 	git init repo &&
 	(
 		cd repo &&
@@ -213,16 +213,16 @@ test_expect_success REFFILES 'drop stash reflog updates refs/stash with rewrite'
 	new_oid="$(git -C repo rev-parse stash@{0})" &&
 
 	cat >expect <<-EOF &&
-	$(test_oid zero) $old_oid
-	$old_oid $new_oid
+	$new_oid
+	$old_oid
 	EOF
-	cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
+	git -C repo reflog show refs/stash --format=%H >actual &&
 	test_cmp expect actual &&
 
 	git -C repo stash drop stash@{1} &&
-	cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
+	git -C repo reflog show refs/stash --format=%H >actual &&
 	cat >expect <<-EOF &&
-	$(test_oid zero) $new_oid
+	$new_oid
 	EOF
 	test_cmp expect actual
 '

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

Re: [PATCH 12/12] t5312: move reffiles specific tests to t0600

[Patrick Steinhardt]

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

On Wed, Jan 17, 2024 at 07:52:35PM +0000, John Cai via GitGitGadget wrote:
> From: John Cai <johncai86@gmail.com>
> 
> Move a few tests into t0600 since they specifically test the packed-refs
> file and thus are specific to the reffiles backend.
> 
> Signed-off-by: John Cai <johncai86@gmail.com>
> ---
>  t/t0600-reffiles-backend.sh | 30 ++++++++++++++++++++++++++++++
>  t/t5312-prune-corruption.sh | 26 --------------------------
>  2 files changed, 30 insertions(+), 26 deletions(-)
> 
> diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
> index c88576dfea5..190155f592d 100755
> --- a/t/t0600-reffiles-backend.sh
> +++ b/t/t0600-reffiles-backend.sh
> @@ -571,4 +571,34 @@ test_expect_success 'log diagnoses bogus HEAD symref' '
>  	test_grep broken stderr
>  '
>  
> +# we do not want to count on running pack-refs to
> +# actually pack it, as it is perfectly reasonable to
> +# skip processing a broken ref
> +test_expect_success 'create packed-refs file with broken ref' '
> +	test_tick && git commit --allow-empty -m one &&
> +	recoverable=$(git rev-parse HEAD) &&
> +	test_tick && git commit --allow-empty -m two &&
> +	missing=$(git rev-parse HEAD) &&
> +	rm -f .git/refs/heads/main &&
> +	cat >.git/packed-refs <<-EOF &&
> +	$missing refs/heads/main
> +	$recoverable refs/heads/other
> +	EOF
> +	echo $missing >expect &&
> +	git rev-parse refs/heads/main >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'pack-refs does not silently delete broken packed ref' '
> +	git pack-refs --all --prune &&
> +	git rev-parse refs/heads/main >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success  'pack-refs does not drop broken refs during deletion' '
> +	git update-ref -d refs/heads/other &&
> +	git rev-parse refs/heads/main >actual &&
> +	test_cmp expect actual
> +'

Should these tests be moved into t0601-reffiles-pack-refs.sh instead?

Patrick

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

Re: [PATCH 10/12] t3903: move reffiles specific tests to t0600

[John Cai]

Hi Patrick,

On 19 Jan 2024, at 8:39, Patrick Steinhardt wrote:

> On Wed, Jan 17, 2024 at 07:52:33PM +0000, John Cai via GitGitGadget wrote:
>> From: John Cai <johncai86@gmail.com>
>>
>> Move this test into t0600 with other reffiles specific tests since it
>> modifies reflog refs manually and thus is specific to the reffiles
>> backend.
>>
>> This change also consolidates setup_stash() into test-lib-functions.sh
>>
>> Signed-off-by: John Cai <johncai86@gmail.com>
>> ---
>>  t/t0600-reffiles-backend.sh | 27 +++++++++++++++++++++++
>>  t/t3903-stash.sh            | 43 -------------------------------------
>>  t/test-lib-functions.sh     | 16 ++++++++++++++
>>  3 files changed, 43 insertions(+), 43 deletions(-)
>>
>> diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
>> index 704b73fdc54..bee61b2d19d 100755
>> --- a/t/t0600-reffiles-backend.sh
>> +++ b/t/t0600-reffiles-backend.sh
>> @@ -527,4 +527,31 @@ test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '
>>         test_must_fail git rev-parse --verify broken
>>  '
>>
>> +test_expect_success 'drop stash reflog updates refs/stash with rewrite' '
>> +	git init repo &&
>> +	(
>> +		cd repo &&
>> +		setup_stash
>> +	) &&
>> +	echo 9 >repo/file &&
>> +
>> +	old_oid="$(git -C repo rev-parse stash@{0})" &&
>> +	git -C repo stash &&
>> +	new_oid="$(git -C repo rev-parse stash@{0})" &&
>> +
>> +	cat >expect <<-EOF &&
>> +	$(test_oid zero) $old_oid
>> +	$old_oid $new_oid
>> +	EOF
>> +	cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
>> +	test_cmp expect actual &&
>> +
>> +	git -C repo stash drop stash@{1} &&
>> +	cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
>> +	cat >expect <<-EOF &&
>> +	$(test_oid zero) $new_oid
>> +	EOF
>> +	test_cmp expect actual
>> +'
>
> I think that there is no need to make this backend-specific. What we're
> testing here is that `git stash drop` is able to drop the latest reflog
> entry. The calls to cut(1) are only used to verify that the contents of
> the reflog entry look as expected while only verifying the old and new
> object IDs.
>
> So how about below patch to make it generic instead?

Nice catch. This sounds perfect to me.

>
> Patrick
>
> -- >8 --
>
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index 34faeac3f1..3319240515 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -200,7 +200,7 @@ test_expect_success 'drop stash reflog updates refs/stash' '
>  	test_cmp expect actual
>  '
>
> -test_expect_success REFFILES 'drop stash reflog updates refs/stash with rewrite' '
> +test_expect_success 'drop stash reflog updates refs/stash with rewrite' '
>  	git init repo &&
>  	(
>  		cd repo &&
> @@ -213,16 +213,16 @@ test_expect_success REFFILES 'drop stash reflog updates refs/stash with rewrite'
>  	new_oid="$(git -C repo rev-parse stash@{0})" &&
>
>  	cat >expect <<-EOF &&
> -	$(test_oid zero) $old_oid
> -	$old_oid $new_oid
> +	$new_oid
> +	$old_oid
>  	EOF
> -	cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
> +	git -C repo reflog show refs/stash --format=%H >actual &&
>  	test_cmp expect actual &&
>
>  	git -C repo stash drop stash@{1} &&
> -	cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
> +	git -C repo reflog show refs/stash --format=%H >actual &&
>  	cat >expect <<-EOF &&
> -	$(test_oid zero) $new_oid
> +	$new_oid
>  	EOF
>  	test_cmp expect actual
>  '

[PATCH v2 00/12] Group reffiles tests

[John Cai via GitGitGadget]

This series groups REFFILES specific tests together. These tests are
currently grouped together across the test suite based on functionality.
However, since they exercise low-level behavior specific to the refs backend
being used (in these cases, the ref-files backend), group them together
based on which refs backend they test. This way, in the near future when the
reftables backend gets upstreamed we can add tests that exercise the
reftables backend close by in the t06xx area.

These patches also remove the REFFILES prerequisite, since all the tests in
t06xx are reffiles specific. In the near future, once the reftable backend
is upstreamed, all the tests in t06xx will be forced to run with the
reffiles backend.

Changes since V1:

 * Moved some pack-refs tests to t0601 instead of t0600
 * Clarified some commit messages
 * Converted a test to be refs-backend agnostic
 * Other minor rearranging of tests

John Cai (12):
  t3210: move to t0601
  remove REFFILES prerequisite for some tests in t1405 and t2017
  t1414: convert test to use Git commands instead of writing refs
    manually
  t1404: move reffiles specific tests to t0600
  t1405: move reffiles specific tests to t0601
  t1406: move reffiles specific tests to t0600
  t1410: move reffiles specific tests to t0600
  t1415: move reffiles specific tests to t0601
  t1503: move reffiles specific tests to t0600
  t3903: make drop stash test ref backend agnostic
  t4202: move reffiles specific tests to t0600
  t5312: move reffiles specific tests to t0601

 t/t0600-reffiles-backend.sh                   | 384 ++++++++++++++++++
 ...ck-refs.sh => t0601-reffiles-pack-refs.sh} |  64 +++
 t/t1404-update-ref-errors.sh                  | 237 -----------
 t/t1405-main-ref-store.sh                     |  10 +-
 t/t1407-worktree-ref-store.sh                 |  37 --
 t/t1410-reflog.sh                             |  42 --
 t/t1414-reflog-walk.sh                        |  11 +-
 t/t1415-worktree-refs.sh                      |  11 -
 t/t1503-rev-parse-verify.sh                   |   5 -
 t/t2017-checkout-orphan.sh                    |   2 +-
 t/t3903-stash.sh                              |  12 +-
 t/t4202-log.sh                                |  17 -
 t/t5312-prune-corruption.sh                   |  26 --
 13 files changed, 461 insertions(+), 397 deletions(-)
 create mode 100755 t/t0600-reffiles-backend.sh
 rename t/{t3210-pack-refs.sh => t0601-reffiles-pack-refs.sh} (81%)


base-commit: 186b115d3062e6230ee296d1ddaa0c4b72a464b5
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1647%2Fjohn-cai%2Fjc%2Fgroup-reffiles-tests-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1647/john-cai/jc/group-reffiles-tests-v2
Pull-Request: https://github.com/git/git/pull/1647

Range-diff vs v1:

  1:  0e2b6e197ab <  -:  ----------- t3210: move to t0602
  -:  ----------- >  1:  ca65b9e6122 t3210: move to t0601
  2:  624ad202305 !  2:  29c32d3e6f7 remove REFFILES prerequisite
     @@ Metadata
      Author: John Cai <johncai86@gmail.com>
      
       ## Commit message ##
     -    remove REFFILES prerequisite
     +    remove REFFILES prerequisite for some tests in t1405 and t2017
      
          These tests are compatible with the reftable backend and thus do not
     -    need the REFFILES prerequisite.
     +    need the REFFILES prerequisite. Even though 53af25e4
     +    (t1405: mark test that checks existence as REFFILES, 2022-01-31) and
     +    53af25e4 (t1405: mark test that checks existence as REFFILES,
     +    2022-01-31) marked these tests to require REFFILES, the reftable backend
     +    in its current state does indeed work with these tests.
      
          Signed-off-by: John Cai <johncai86@gmail.com>
      
  3:  19233aa0d44 !  3:  122d19a9095 t1414: convert test to use Git commands instead of writing refs manually
     @@ t/t1414-reflog-walk.sh: test_expect_success 'min/max age uses entry date to limi
      -	one=$(git rev-parse one) &&
      -	ident="$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE" &&
      -	echo "$head $one $ident	broken reflog entry" >>.git/logs/HEAD &&
     -+	A=$(git rev-parse A) &&
     - 
     +-
      -	echo $one >expect &&
     -+	echo $A >expect &&
     ++	git rev-parse A >expect &&
       	git log -g --format=%H -1 >actual &&
       	test_cmp expect actual
       '
  4:  0f6fea6d32d !  4:  c3f0b81200c t1404: move reffiles specific tests to t0600
     @@ t/t0600-reffiles-backend.sh (new)
      +TEST_PASSES_SANITIZE_LEAK=true
      +. ./test-lib.sh
      +
     -+# Test adding and deleting D/F-conflicting references in a single
     -+# transaction.
     -+df_test() {
     -+	prefix="$1"
     -+	pack=: symadd=false symdel=false add_del=false addref= delref=
     -+	shift
     -+	while test $# -gt 0
     -+	do
     -+		case "$1" in
     -+		--pack)
     -+			pack="git pack-refs --all"
     -+			shift
     -+			;;
     -+		--sym-add)
     -+			# Perform the add via a symbolic reference
     -+			symadd=true
     -+			shift
     -+			;;
     -+		--sym-del)
     -+			# Perform the del via a symbolic reference
     -+			symdel=true
     -+			shift
     -+			;;
     -+		--del-add)
     -+			# Delete first reference then add second
     -+			add_del=false
     -+			delref="$prefix/r/$2"
     -+			addref="$prefix/r/$3"
     -+			shift 3
     -+			;;
     -+		--add-del)
     -+			# Add first reference then delete second
     -+			add_del=true
     -+			addref="$prefix/r/$2"
     -+			delref="$prefix/r/$3"
     -+			shift 3
     -+			;;
     -+		*)
     -+			echo 1>&2 "Extra args to df_test: $*"
     -+			return 1
     -+			;;
     -+		esac
     -+	done
     -+	git update-ref "$delref" $C &&
     -+	if $symadd
     ++if ! test_have_prereq REFFILES
      +	then
     -+		addname="$prefix/s/symadd" &&
     -+		git symbolic-ref "$addname" "$addref"
     -+	else
     -+		addname="$addref"
     -+	fi &&
     -+	if $symdel
     -+	then
     -+		delname="$prefix/s/symdel" &&
     -+		git symbolic-ref "$delname" "$delref"
     -+	else
     -+		delname="$delref"
     -+	fi &&
     -+	cat >expected-err <<-EOF &&
     -+	fatal: cannot lock ref $SQ$addname$SQ: $SQ$delref$SQ exists; cannot create $SQ$addref$SQ
     -+	EOF
     -+	$pack &&
     -+	if $add_del
     -+	then
     -+		printf "%s\n" "create $addname $D" "delete $delname"
     -+	else
     -+		printf "%s\n" "delete $delname" "create $addname $D"
     -+	fi >commands &&
     -+	test_must_fail git update-ref --stdin <commands 2>output.err &&
     -+	test_cmp expected-err output.err &&
     -+	printf "%s\n" "$C $delref" >expected-refs &&
     -+	git for-each-ref --format="%(objectname) %(refname)" $prefix/r >actual-refs &&
     -+	test_cmp expected-refs actual-refs
     -+}
     ++		skip_all='skipping reffiles specific tests'
     ++		test_done
     ++fi
      +
      +test_expect_success 'setup' '
      +	git commit --allow-empty -m Initial &&
     @@ t/t0600-reffiles-backend.sh (new)
      +	git update-ref --stdin
      +'
      +
     -+test_expect_success 'D/F conflict prevents add long + delete short' '
     -+	df_test refs/df-al-ds --add-del foo/bar foo
     -+'
     -+
     -+test_expect_success 'D/F conflict prevents add short + delete long' '
     -+	df_test refs/df-as-dl --add-del foo foo/bar
     -+'
     -+
     -+test_expect_success 'D/F conflict prevents delete long + add short' '
     -+	df_test refs/df-dl-as --del-add foo/bar foo
     -+'
     -+
     -+test_expect_success 'D/F conflict prevents delete short + add long' '
     -+	df_test refs/df-ds-al --del-add foo foo/bar
     -+'
     -+
     -+test_expect_success 'D/F conflict prevents add long + delete short packed' '
     -+	df_test refs/df-al-dsp --pack --add-del foo/bar foo
     -+'
     -+
     -+test_expect_success 'D/F conflict prevents add short + delete long packed' '
     -+	df_test refs/df-as-dlp --pack --add-del foo foo/bar
     -+'
     -+
     -+test_expect_success 'D/F conflict prevents delete long packed + add short' '
     -+	df_test refs/df-dlp-as --pack --del-add foo/bar foo
     -+'
     -+
     -+test_expect_success 'D/F conflict prevents delete short packed + add long' '
     -+	df_test refs/df-dsp-al --pack --del-add foo foo/bar
     -+'
     -+
     -+# Try some combinations involving symbolic refs...
     -+
     -+test_expect_success 'D/F conflict prevents indirect add long + delete short' '
     -+	df_test refs/df-ial-ds --sym-add --add-del foo/bar foo
     -+'
     -+
     -+test_expect_success 'D/F conflict prevents indirect add long + indirect delete short' '
     -+	df_test refs/df-ial-ids --sym-add --sym-del --add-del foo/bar foo
     -+'
     -+
     -+test_expect_success 'D/F conflict prevents indirect add short + indirect delete long' '
     -+	df_test refs/df-ias-idl --sym-add --sym-del --add-del foo foo/bar
     -+'
     -+
     -+test_expect_success 'D/F conflict prevents indirect delete long + indirect add short' '
     -+	df_test refs/df-idl-ias --sym-add --sym-del --del-add foo/bar foo
     -+'
     -+
     -+test_expect_success 'D/F conflict prevents indirect add long + delete short packed' '
     -+	df_test refs/df-ial-dsp --sym-add --pack --add-del foo/bar foo
     -+'
     -+
     -+test_expect_success 'D/F conflict prevents indirect add long + indirect delete short packed' '
     -+	df_test refs/df-ial-idsp --sym-add --sym-del --pack --add-del foo/bar foo
     -+'
     -+
     -+test_expect_success 'D/F conflict prevents add long + indirect delete short packed' '
     -+	df_test refs/df-al-idsp --sym-del --pack --add-del foo/bar foo
     -+'
     -+
     -+test_expect_success 'D/F conflict prevents indirect delete long packed + indirect add short' '
     -+	df_test refs/df-idlp-ias --sym-add --sym-del --pack --del-add foo/bar foo
     -+'
     -+
      +test_expect_success 'non-empty directory blocks create' '
      +	prefix=refs/ne-create &&
      +	mkdir -p .git/$prefix/foo/bar &&
     @@ t/t0600-reffiles-backend.sh (new)
      +test_done
      
       ## t/t1404-update-ref-errors.sh ##
     -@@ t/t1404-update-ref-errors.sh: test_update_rejected () {
     - 	test_cmp unchanged actual
     - }
     - 
     --# Test adding and deleting D/F-conflicting references in a single
     --# transaction.
     --df_test() {
     --	prefix="$1"
     --	pack=: symadd=false symdel=false add_del=false addref= delref=
     --	shift
     --	while test $# -gt 0
     --	do
     --		case "$1" in
     --		--pack)
     --			pack="git pack-refs --all"
     --			shift
     --			;;
     --		--sym-add)
     --			# Perform the add via a symbolic reference
     --			symadd=true
     --			shift
     --			;;
     --		--sym-del)
     --			# Perform the del via a symbolic reference
     --			symdel=true
     --			shift
     --			;;
     --		--del-add)
     --			# Delete first reference then add second
     --			add_del=false
     --			delref="$prefix/r/$2"
     --			addref="$prefix/r/$3"
     --			shift 3
     --			;;
     --		--add-del)
     --			# Add first reference then delete second
     --			add_del=true
     --			addref="$prefix/r/$2"
     --			delref="$prefix/r/$3"
     --			shift 3
     --			;;
     --		*)
     --			echo 1>&2 "Extra args to df_test: $*"
     --			return 1
     --			;;
     --		esac
     --	done
     --	git update-ref "$delref" $C &&
     --	if $symadd
     --	then
     --		addname="$prefix/s/symadd" &&
     --		git symbolic-ref "$addname" "$addref"
     --	else
     --		addname="$addref"
     --	fi &&
     --	if $symdel
     --	then
     --		delname="$prefix/s/symdel" &&
     --		git symbolic-ref "$delname" "$delref"
     --	else
     --		delname="$delref"
     --	fi &&
     --	cat >expected-err <<-EOF &&
     --	fatal: cannot lock ref $SQ$addname$SQ: $SQ$delref$SQ exists; cannot create $SQ$addref$SQ
     --	EOF
     --	$pack &&
     --	if $add_del
     --	then
     --		printf "%s\n" "create $addname $D" "delete $delname"
     --	else
     --		printf "%s\n" "delete $delname" "create $addname $D"
     --	fi >commands &&
     --	test_must_fail git update-ref --stdin <commands 2>output.err &&
     --	test_cmp expected-err output.err &&
     --	printf "%s\n" "$C $delref" >expected-refs &&
     --	git for-each-ref --format="%(objectname) %(refname)" $prefix/r >actual-refs &&
     --	test_cmp expected-refs actual-refs
     --}
     --
     - test_expect_success 'setup' '
     - 
     - 	git commit --allow-empty -m Initial &&
      @@ t/t1404-update-ref-errors.sh: test_expect_success 'one new ref is a simple prefix of another' '
       
       '
     @@ t/t1404-update-ref-errors.sh: test_expect_success 'one new ref is a simple prefi
      -	git update-ref --stdin
      -'
      -
     --test_expect_success REFFILES 'D/F conflict prevents add long + delete short' '
     --	df_test refs/df-al-ds --add-del foo/bar foo
     --'
     --
     --test_expect_success REFFILES 'D/F conflict prevents add short + delete long' '
     --	df_test refs/df-as-dl --add-del foo foo/bar
     --'
     --
     --test_expect_success REFFILES 'D/F conflict prevents delete long + add short' '
     --	df_test refs/df-dl-as --del-add foo/bar foo
     --'
     --
     --test_expect_success REFFILES 'D/F conflict prevents delete short + add long' '
     --	df_test refs/df-ds-al --del-add foo foo/bar
     --'
     --
     --test_expect_success REFFILES 'D/F conflict prevents add long + delete short packed' '
     --	df_test refs/df-al-dsp --pack --add-del foo/bar foo
     --'
     --
     --test_expect_success REFFILES 'D/F conflict prevents add short + delete long packed' '
     --	df_test refs/df-as-dlp --pack --add-del foo foo/bar
     --'
     --
     --test_expect_success REFFILES 'D/F conflict prevents delete long packed + add short' '
     --	df_test refs/df-dlp-as --pack --del-add foo/bar foo
     --'
     --
     --test_expect_success REFFILES 'D/F conflict prevents delete short packed + add long' '
     --	df_test refs/df-dsp-al --pack --del-add foo foo/bar
     --'
     --
     --# Try some combinations involving symbolic refs...
     --
     --test_expect_success REFFILES 'D/F conflict prevents indirect add long + delete short' '
     --	df_test refs/df-ial-ds --sym-add --add-del foo/bar foo
     --'
     --
     --test_expect_success REFFILES 'D/F conflict prevents indirect add long + indirect delete short' '
     --	df_test refs/df-ial-ids --sym-add --sym-del --add-del foo/bar foo
     --'
     --
     --test_expect_success REFFILES 'D/F conflict prevents indirect add short + indirect delete long' '
     --	df_test refs/df-ias-idl --sym-add --sym-del --add-del foo foo/bar
     --'
     --
     --test_expect_success REFFILES 'D/F conflict prevents indirect delete long + indirect add short' '
     --	df_test refs/df-idl-ias --sym-add --sym-del --del-add foo/bar foo
     --'
     --
     --test_expect_success REFFILES 'D/F conflict prevents indirect add long + delete short packed' '
     --	df_test refs/df-ial-dsp --sym-add --pack --add-del foo/bar foo
     --'
     --
     --test_expect_success REFFILES 'D/F conflict prevents indirect add long + indirect delete short packed' '
     --	df_test refs/df-ial-idsp --sym-add --sym-del --pack --add-del foo/bar foo
     --'
     --
     --test_expect_success REFFILES 'D/F conflict prevents add long + indirect delete short packed' '
     --	df_test refs/df-al-idsp --sym-del --pack --add-del foo/bar foo
     --'
     --
     --test_expect_success REFFILES 'D/F conflict prevents indirect delete long packed + indirect add short' '
     --	df_test refs/df-idlp-ias --sym-add --sym-del --pack --del-add foo/bar foo
     --'
     --
     - # Test various errors when reading the old values of references...
     - 
     - test_expect_success 'missing old value blocks update' '
     + test_expect_success REFFILES 'D/F conflict prevents add long + delete short' '
     + 	df_test refs/df-al-ds --add-del foo/bar foo
     + '
      @@ t/t1404-update-ref-errors.sh: test_expect_success 'incorrect old value blocks indirect no-deref delete' '
       	test_cmp expected output.err
       '
  5:  c2af695f551 !  5:  42dc9948aa5 t1405: move reffiles specific tests to t0600
     @@ Metadata
      Author: John Cai <johncai86@gmail.com>
      
       ## Commit message ##
     -    t1405: move reffiles specific tests to t0600
     +    t1405: move reffiles specific tests to t0601
      
     -    Move this test to t0600 with other reffiles specific tests since it is
     -    reffiles specific in that it looks into the loose refs directory for an
     -    assertion.
     +    Move this test to t0601 with other reffiles specific pack-refs tests
     +    since it is reffiles specific in that it looks into the loose refs
     +    directory for an assertion.
      
          Signed-off-by: John Cai <johncai86@gmail.com>
      
     - ## t/t0600-reffiles-backend.sh ##
     -@@ t/t0600-reffiles-backend.sh: test_expect_success 'setup' '
     - 	E=$(git rev-parse HEAD)
     + ## t/t0601-reffiles-pack-refs.sh ##
     +@@ t/t0601-reffiles-pack-refs.sh: test_expect_success 'prepare a trivial repository' '
     + 	HEAD=$(git rev-parse --verify HEAD)
       '
       
      +test_expect_success 'pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE)' '
     @@ t/t0600-reffiles-backend.sh: test_expect_success 'setup' '
      +	test -z "$N"
      +'
      +
     - test_expect_success 'empty directory should not fool rev-parse' '
     - 	prefix=refs/e-rev-parse &&
     - 	git update-ref $prefix/foo $C &&
     + SHA1=
     + 
     + test_expect_success 'see if git show-ref works as expected' '
      
       ## t/t1405-main-ref-store.sh ##
      @@ t/t1405-main-ref-store.sh: test_expect_success 'setup' '
  6:  69ea950cfea =  6:  98e40a024b9 t1406: move reffiles specific tests to t0600
  7:  ae71747871c =  7:  d93c9c410b9 t1410: move reffiles specific tests to t0600
  8:  9d105263695 !  8:  8327b12a313 t1415: move reffiles specific tests to t0600
     @@ Metadata
      Author: John Cai <johncai86@gmail.com>
      
       ## Commit message ##
     -    t1415: move reffiles specific tests to t0600
     +    t1415: move reffiles specific tests to t0601
      
     -    Move this test into t0600 with other reffiles specific tests since it
     -    checks for individua loose refs and thus is specific to the reffiles
     -    backend.
     +    Move this test into t0601 with other reffiles pack-refs specific tests
     +    since it checks for individua loose refs and thus is specific to the
     +    reffiles backend.
      
          Signed-off-by: John Cai <johncai86@gmail.com>
      
     - ## t/t0600-reffiles-backend.sh ##
     -@@ t/t0600-reffiles-backend.sh: test_expect_success 'empty reflog' '
     - 	test_must_be_empty err
     + ## t/t0601-reffiles-pack-refs.sh ##
     +@@ t/t0601-reffiles-pack-refs.sh: test_expect_success SYMLINKS 'pack symlinked packed-refs' '
     + 	test "$(test_readlink .git/packed-refs)" = "my-deviant-packed-refs"
       '
       
      +# The 'packed-refs' file is stored directly in .git/. This means it is global
  9:  dcec7f10ab6 !  9:  891a3d057d2 t1503: move reffiles specific tests to t0600
     @@ Commit message
          Signed-off-by: John Cai <johncai86@gmail.com>
      
       ## t/t0600-reffiles-backend.sh ##
     -@@ t/t0600-reffiles-backend.sh: test_expect_success 'refs/worktree must not be packed' '
     - 	test_path_is_file .git/worktrees/wt2/refs/worktree/foo
     +@@ t/t0600-reffiles-backend.sh: test_expect_success 'empty reflog' '
     + 	test_must_be_empty err
       '
       
      +test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '
 10:  56a9c8f20dd <  -:  ----------- t3903: move reffiles specific tests to t0600
  -:  ----------- > 10:  bfd5b403170 t3903: make drop stash test ref backend agnostic
 11:  39e69fde3d7 ! 11:  976be7efc89 t4202: move reffiles specific tests to t0600
     @@ Commit message
          Signed-off-by: John Cai <johncai86@gmail.com>
      
       ## t/t0600-reffiles-backend.sh ##
     -@@ t/t0600-reffiles-backend.sh: test_expect_success 'drop stash reflog updates refs/stash with rewrite' '
     - 	test_cmp expect actual
     +@@ t/t0600-reffiles-backend.sh: test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '
     +        test_must_fail git rev-parse --verify broken
       '
       
      +test_expect_success 'log diagnoses bogus HEAD hash' '
 12:  316a20ed179 ! 12:  7329e87148a t5312: move reffiles specific tests to t0600
     @@ Metadata
      Author: John Cai <johncai86@gmail.com>
      
       ## Commit message ##
     -    t5312: move reffiles specific tests to t0600
     +    t5312: move reffiles specific tests to t0601
      
     -    Move a few tests into t0600 since they specifically test the packed-refs
     +    Move a few tests into t0601 since they specifically test the packed-refs
          file and thus are specific to the reffiles backend.
      
          Signed-off-by: John Cai <johncai86@gmail.com>
      
     - ## t/t0600-reffiles-backend.sh ##
     -@@ t/t0600-reffiles-backend.sh: test_expect_success 'log diagnoses bogus HEAD symref' '
     - 	test_grep broken stderr
     + ## t/t0601-reffiles-pack-refs.sh ##
     +@@ t/t0601-reffiles-pack-refs.sh: test_expect_success 'refs/worktree must not be packed' '
     + 	test_path_is_file .git/worktrees/wt2/refs/worktree/foo
       '
       
      +# we do not want to count on running pack-refs to
     @@ t/t0600-reffiles-backend.sh: test_expect_success 'log diagnoses bogus HEAD symre
      +	test_cmp expect actual
      +'
      +
     -+test_expect_success  'pack-refs does not drop broken refs during deletion' '
     ++test_expect_success 'pack-refs does not drop broken refs during deletion' '
      +	git update-ref -d refs/heads/other &&
      +	git rev-parse refs/heads/main >actual &&
      +	test_cmp expect actual

-- 
gitgitgadget

[PATCH v2 01/12] t3210: move to t0601

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

Move t3210 to t0601, since these tests are reffiles specific in that
they modify loose refs manually. This is part of the effort to
categorize these tests together based on the ref backend they test. When
we upstream the reftable backend, we can add more tests to t06xx. This
way, all tests that test specific ref backend behavior will be grouped
together.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/{t3210-pack-refs.sh => t0601-reffiles-pack-refs.sh} | 6 ++++++
 1 file changed, 6 insertions(+)
 rename t/{t3210-pack-refs.sh => t0601-reffiles-pack-refs.sh} (98%)

diff --git a/t/t3210-pack-refs.sh b/t/t0601-reffiles-pack-refs.sh
similarity index 98%
rename from t/t3210-pack-refs.sh
rename to t/t0601-reffiles-pack-refs.sh
index 7f4e98db7db..f7a3f693901 100755
--- a/t/t3210-pack-refs.sh
+++ b/t/t0601-reffiles-pack-refs.sh
@@ -15,6 +15,12 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
+if ! test_have_prereq REFFILES
+	then
+		skip_all='skipping reffiles specific tests'
+		test_done
+fi
+
 test_expect_success 'enable reflogs' '
 	git config core.logallrefupdates true
 '
-- 
gitgitgadget

[PATCH v2 02/12] remove REFFILES prerequisite for some tests in t1405 and t2017

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

These tests are compatible with the reftable backend and thus do not
need the REFFILES prerequisite. Even though 53af25e4
(t1405: mark test that checks existence as REFFILES, 2022-01-31) and
53af25e4 (t1405: mark test that checks existence as REFFILES,
2022-01-31) marked these tests to require REFFILES, the reftable backend
in its current state does indeed work with these tests.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t1405-main-ref-store.sh  | 2 +-
 t/t2017-checkout-orphan.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
index e4627cf1b61..62c1eadb190 100755
--- a/t/t1405-main-ref-store.sh
+++ b/t/t1405-main-ref-store.sh
@@ -112,7 +112,7 @@ test_expect_success 'delete_reflog(HEAD)' '
 	test_must_fail git reflog exists HEAD
 '
 
-test_expect_success REFFILES 'create-reflog(HEAD)' '
+test_expect_success 'create-reflog(HEAD)' '
 	$RUN create-reflog HEAD &&
 	git reflog exists HEAD
 '
diff --git a/t/t2017-checkout-orphan.sh b/t/t2017-checkout-orphan.sh
index 947d1587ac8..a5c7358eeab 100755
--- a/t/t2017-checkout-orphan.sh
+++ b/t/t2017-checkout-orphan.sh
@@ -86,7 +86,7 @@ test_expect_success '--orphan makes reflog by default' '
 	git rev-parse --verify delta@{0}
 '
 
-test_expect_success REFFILES '--orphan does not make reflog when core.logAllRefUpdates = false' '
+test_expect_success '--orphan does not make reflog when core.logAllRefUpdates = false' '
 	git checkout main &&
 	git config core.logAllRefUpdates false &&
 	git checkout --orphan epsilon &&
-- 
gitgitgadget

[PATCH v2 03/12] t1414: convert test to use Git commands instead of writing refs manually

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

This test can be re-written to use Git commands rather than writing a
manual ref in the reflog. This way this test no longer needs the
REFFILES prerequisite.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t1414-reflog-walk.sh | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/t/t1414-reflog-walk.sh b/t/t1414-reflog-walk.sh
index ea64cecf47b..be6c3f472c1 100755
--- a/t/t1414-reflog-walk.sh
+++ b/t/t1414-reflog-walk.sh
@@ -121,13 +121,12 @@ test_expect_success 'min/max age uses entry date to limit' '
 
 # Create a situation where the reflog and ref database disagree about the latest
 # state of HEAD.
-test_expect_success REFFILES 'walk prefers reflog to ref tip' '
+test_expect_success 'walk prefers reflog to ref tip' '
+	test_commit A &&
+	test_commit B &&
+	git reflog delete HEAD@{0} &&
 	head=$(git rev-parse HEAD) &&
-	one=$(git rev-parse one) &&
-	ident="$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE" &&
-	echo "$head $one $ident	broken reflog entry" >>.git/logs/HEAD &&
-
-	echo $one >expect &&
+	git rev-parse A >expect &&
 	git log -g --format=%H -1 >actual &&
 	test_cmp expect actual
 '
-- 
gitgitgadget

[PATCH v2 04/12] t1404: move reffiles specific tests to t0600

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

These tests modify loose refs manually and are specific to the reffiles
backend. Move these to t0600 to be part of a test suite of reffiles
specific tests.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t0600-reffiles-backend.sh  | 263 +++++++++++++++++++++++++++++++++++
 t/t1404-update-ref-errors.sh | 237 -------------------------------
 2 files changed, 263 insertions(+), 237 deletions(-)
 create mode 100755 t/t0600-reffiles-backend.sh

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
new file mode 100755
index 00000000000..2f910bd76ad
--- /dev/null
+++ b/t/t0600-reffiles-backend.sh
@@ -0,0 +1,263 @@
+#!/bin/sh
+
+test_description='Test reffiles backend'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+TEST_PASSES_SANITIZE_LEAK=true
+. ./test-lib.sh
+
+if ! test_have_prereq REFFILES
+	then
+		skip_all='skipping reffiles specific tests'
+		test_done
+fi
+
+test_expect_success 'setup' '
+	git commit --allow-empty -m Initial &&
+	C=$(git rev-parse HEAD) &&
+	git commit --allow-empty -m Second &&
+	D=$(git rev-parse HEAD) &&
+	git commit --allow-empty -m Third &&
+	E=$(git rev-parse HEAD)
+'
+
+test_expect_success 'empty directory should not fool rev-parse' '
+	prefix=refs/e-rev-parse &&
+	git update-ref $prefix/foo $C &&
+	git pack-refs --all &&
+	mkdir -p .git/$prefix/foo/bar/baz &&
+	echo "$C" >expected &&
+	git rev-parse $prefix/foo >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'empty directory should not fool for-each-ref' '
+	prefix=refs/e-for-each-ref &&
+	git update-ref $prefix/foo $C &&
+	git for-each-ref $prefix >expected &&
+	git pack-refs --all &&
+	mkdir -p .git/$prefix/foo/bar/baz &&
+	git for-each-ref $prefix >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'empty directory should not fool create' '
+	prefix=refs/e-create &&
+	mkdir -p .git/$prefix/foo/bar/baz &&
+	printf "create %s $C\n" $prefix/foo |
+	git update-ref --stdin
+'
+
+test_expect_success 'empty directory should not fool verify' '
+	prefix=refs/e-verify &&
+	git update-ref $prefix/foo $C &&
+	git pack-refs --all &&
+	mkdir -p .git/$prefix/foo/bar/baz &&
+	printf "verify %s $C\n" $prefix/foo |
+	git update-ref --stdin
+'
+
+test_expect_success 'empty directory should not fool 1-arg update' '
+	prefix=refs/e-update-1 &&
+	git update-ref $prefix/foo $C &&
+	git pack-refs --all &&
+	mkdir -p .git/$prefix/foo/bar/baz &&
+	printf "update %s $D\n" $prefix/foo |
+	git update-ref --stdin
+'
+
+test_expect_success 'empty directory should not fool 2-arg update' '
+	prefix=refs/e-update-2 &&
+	git update-ref $prefix/foo $C &&
+	git pack-refs --all &&
+	mkdir -p .git/$prefix/foo/bar/baz &&
+	printf "update %s $D $C\n" $prefix/foo |
+	git update-ref --stdin
+'
+
+test_expect_success 'empty directory should not fool 0-arg delete' '
+	prefix=refs/e-delete-0 &&
+	git update-ref $prefix/foo $C &&
+	git pack-refs --all &&
+	mkdir -p .git/$prefix/foo/bar/baz &&
+	printf "delete %s\n" $prefix/foo |
+	git update-ref --stdin
+'
+
+test_expect_success 'empty directory should not fool 1-arg delete' '
+	prefix=refs/e-delete-1 &&
+	git update-ref $prefix/foo $C &&
+	git pack-refs --all &&
+	mkdir -p .git/$prefix/foo/bar/baz &&
+	printf "delete %s $C\n" $prefix/foo |
+	git update-ref --stdin
+'
+
+test_expect_success 'non-empty directory blocks create' '
+	prefix=refs/ne-create &&
+	mkdir -p .git/$prefix/foo/bar &&
+	: >.git/$prefix/foo/bar/baz.lock &&
+	test_when_finished "rm -f .git/$prefix/foo/bar/baz.lock" &&
+	cat >expected <<-EOF &&
+	fatal: cannot lock ref $SQ$prefix/foo$SQ: there is a non-empty directory $SQ.git/$prefix/foo$SQ blocking reference $SQ$prefix/foo$SQ
+	EOF
+	printf "%s\n" "update $prefix/foo $C" |
+	test_must_fail git update-ref --stdin 2>output.err &&
+	test_cmp expected output.err &&
+	cat >expected <<-EOF &&
+	fatal: cannot lock ref $SQ$prefix/foo$SQ: unable to resolve reference $SQ$prefix/foo$SQ
+	EOF
+	printf "%s\n" "update $prefix/foo $D $C" |
+	test_must_fail git update-ref --stdin 2>output.err &&
+	test_cmp expected output.err
+'
+
+test_expect_success 'broken reference blocks create' '
+	prefix=refs/broken-create &&
+	mkdir -p .git/$prefix &&
+	echo "gobbledigook" >.git/$prefix/foo &&
+	test_when_finished "rm -f .git/$prefix/foo" &&
+	cat >expected <<-EOF &&
+	fatal: cannot lock ref $SQ$prefix/foo$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken
+	EOF
+	printf "%s\n" "update $prefix/foo $C" |
+	test_must_fail git update-ref --stdin 2>output.err &&
+	test_cmp expected output.err &&
+	cat >expected <<-EOF &&
+	fatal: cannot lock ref $SQ$prefix/foo$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken
+	EOF
+	printf "%s\n" "update $prefix/foo $D $C" |
+	test_must_fail git update-ref --stdin 2>output.err &&
+	test_cmp expected output.err
+'
+
+test_expect_success 'non-empty directory blocks indirect create' '
+	prefix=refs/ne-indirect-create &&
+	git symbolic-ref $prefix/symref $prefix/foo &&
+	mkdir -p .git/$prefix/foo/bar &&
+	: >.git/$prefix/foo/bar/baz.lock &&
+	test_when_finished "rm -f .git/$prefix/foo/bar/baz.lock" &&
+	cat >expected <<-EOF &&
+	fatal: cannot lock ref $SQ$prefix/symref$SQ: there is a non-empty directory $SQ.git/$prefix/foo$SQ blocking reference $SQ$prefix/foo$SQ
+	EOF
+	printf "%s\n" "update $prefix/symref $C" |
+	test_must_fail git update-ref --stdin 2>output.err &&
+	test_cmp expected output.err &&
+	cat >expected <<-EOF &&
+	fatal: cannot lock ref $SQ$prefix/symref$SQ: unable to resolve reference $SQ$prefix/foo$SQ
+	EOF
+	printf "%s\n" "update $prefix/symref $D $C" |
+	test_must_fail git update-ref --stdin 2>output.err &&
+	test_cmp expected output.err
+'
+
+test_expect_success 'broken reference blocks indirect create' '
+	prefix=refs/broken-indirect-create &&
+	git symbolic-ref $prefix/symref $prefix/foo &&
+	echo "gobbledigook" >.git/$prefix/foo &&
+	test_when_finished "rm -f .git/$prefix/foo" &&
+	cat >expected <<-EOF &&
+	fatal: cannot lock ref $SQ$prefix/symref$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken
+	EOF
+	printf "%s\n" "update $prefix/symref $C" |
+	test_must_fail git update-ref --stdin 2>output.err &&
+	test_cmp expected output.err &&
+	cat >expected <<-EOF &&
+	fatal: cannot lock ref $SQ$prefix/symref$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken
+	EOF
+	printf "%s\n" "update $prefix/symref $D $C" |
+	test_must_fail git update-ref --stdin 2>output.err &&
+	test_cmp expected output.err
+'
+
+test_expect_success 'no bogus intermediate values during delete' '
+	prefix=refs/slow-transaction &&
+	# Set up a reference with differing loose and packed versions:
+	git update-ref $prefix/foo $C &&
+	git pack-refs --all &&
+	git update-ref $prefix/foo $D &&
+	# Now try to update the reference, but hold the `packed-refs` lock
+	# for a while to see what happens while the process is blocked:
+	: >.git/packed-refs.lock &&
+	test_when_finished "rm -f .git/packed-refs.lock" &&
+	{
+		# Note: the following command is intentionally run in the
+		# background. We increase the timeout so that `update-ref`
+		# attempts to acquire the `packed-refs` lock for much longer
+		# than it takes for us to do the check then delete it:
+		git -c core.packedrefstimeout=30000 update-ref -d $prefix/foo &
+	} &&
+	pid2=$! &&
+	# Give update-ref plenty of time to get to the point where it tries
+	# to lock packed-refs:
+	sleep 1 &&
+	# Make sure that update-ref did not complete despite the lock:
+	kill -0 $pid2 &&
+	# Verify that the reference still has its old value:
+	sha1=$(git rev-parse --verify --quiet $prefix/foo || echo undefined) &&
+	case "$sha1" in
+	$D)
+		# This is what we hope for; it means that nothing
+		# user-visible has changed yet.
+		: ;;
+	undefined)
+		# This is not correct; it means the deletion has happened
+		# already even though update-ref should not have been
+		# able to acquire the lock yet.
+		echo "$prefix/foo deleted prematurely" &&
+		break
+		;;
+	$C)
+		# This value should never be seen. Probably the loose
+		# reference has been deleted but the packed reference
+		# is still there:
+		echo "$prefix/foo incorrectly observed to be C" &&
+		break
+		;;
+	*)
+		# WTF?
+		echo "unexpected value observed for $prefix/foo: $sha1" &&
+		break
+		;;
+	esac >out &&
+	rm -f .git/packed-refs.lock &&
+	wait $pid2 &&
+	test_must_be_empty out &&
+	test_must_fail git rev-parse --verify --quiet $prefix/foo
+'
+
+test_expect_success 'delete fails cleanly if packed-refs file is locked' '
+	prefix=refs/locked-packed-refs &&
+	# Set up a reference with differing loose and packed versions:
+	git update-ref $prefix/foo $C &&
+	git pack-refs --all &&
+	git update-ref $prefix/foo $D &&
+	git for-each-ref $prefix >unchanged &&
+	# Now try to delete it while the `packed-refs` lock is held:
+	: >.git/packed-refs.lock &&
+	test_when_finished "rm -f .git/packed-refs.lock" &&
+	test_must_fail git update-ref -d $prefix/foo >out 2>err &&
+	git for-each-ref $prefix >actual &&
+	test_grep "Unable to create $SQ.*packed-refs.lock$SQ: " err &&
+	test_cmp unchanged actual
+'
+
+test_expect_success 'delete fails cleanly if packed-refs.new write fails' '
+	# Setup and expectations are similar to the test above.
+	prefix=refs/failed-packed-refs &&
+	git update-ref $prefix/foo $C &&
+	git pack-refs --all &&
+	git update-ref $prefix/foo $D &&
+	git for-each-ref $prefix >unchanged &&
+	# This should not happen in practice, but it is an easy way to get a
+	# reliable error (we open with create_tempfile(), which uses O_EXCL).
+	: >.git/packed-refs.new &&
+	test_when_finished "rm -f .git/packed-refs.new" &&
+	test_must_fail git update-ref -d $prefix/foo &&
+	git for-each-ref $prefix >actual &&
+	test_cmp unchanged actual
+'
+
+test_done
diff --git a/t/t1404-update-ref-errors.sh b/t/t1404-update-ref-errors.sh
index 0369beea33b..00b70137053 100755
--- a/t/t1404-update-ref-errors.sh
+++ b/t/t1404-update-ref-errors.sh
@@ -191,78 +191,6 @@ test_expect_success 'one new ref is a simple prefix of another' '
 
 '
 
-test_expect_success REFFILES 'empty directory should not fool rev-parse' '
-	prefix=refs/e-rev-parse &&
-	git update-ref $prefix/foo $C &&
-	git pack-refs --all &&
-	mkdir -p .git/$prefix/foo/bar/baz &&
-	echo "$C" >expected &&
-	git rev-parse $prefix/foo >actual &&
-	test_cmp expected actual
-'
-
-test_expect_success REFFILES 'empty directory should not fool for-each-ref' '
-	prefix=refs/e-for-each-ref &&
-	git update-ref $prefix/foo $C &&
-	git for-each-ref $prefix >expected &&
-	git pack-refs --all &&
-	mkdir -p .git/$prefix/foo/bar/baz &&
-	git for-each-ref $prefix >actual &&
-	test_cmp expected actual
-'
-
-test_expect_success REFFILES 'empty directory should not fool create' '
-	prefix=refs/e-create &&
-	mkdir -p .git/$prefix/foo/bar/baz &&
-	printf "create %s $C\n" $prefix/foo |
-	git update-ref --stdin
-'
-
-test_expect_success REFFILES 'empty directory should not fool verify' '
-	prefix=refs/e-verify &&
-	git update-ref $prefix/foo $C &&
-	git pack-refs --all &&
-	mkdir -p .git/$prefix/foo/bar/baz &&
-	printf "verify %s $C\n" $prefix/foo |
-	git update-ref --stdin
-'
-
-test_expect_success REFFILES 'empty directory should not fool 1-arg update' '
-	prefix=refs/e-update-1 &&
-	git update-ref $prefix/foo $C &&
-	git pack-refs --all &&
-	mkdir -p .git/$prefix/foo/bar/baz &&
-	printf "update %s $D\n" $prefix/foo |
-	git update-ref --stdin
-'
-
-test_expect_success REFFILES 'empty directory should not fool 2-arg update' '
-	prefix=refs/e-update-2 &&
-	git update-ref $prefix/foo $C &&
-	git pack-refs --all &&
-	mkdir -p .git/$prefix/foo/bar/baz &&
-	printf "update %s $D $C\n" $prefix/foo |
-	git update-ref --stdin
-'
-
-test_expect_success REFFILES 'empty directory should not fool 0-arg delete' '
-	prefix=refs/e-delete-0 &&
-	git update-ref $prefix/foo $C &&
-	git pack-refs --all &&
-	mkdir -p .git/$prefix/foo/bar/baz &&
-	printf "delete %s\n" $prefix/foo |
-	git update-ref --stdin
-'
-
-test_expect_success REFFILES 'empty directory should not fool 1-arg delete' '
-	prefix=refs/e-delete-1 &&
-	git update-ref $prefix/foo $C &&
-	git pack-refs --all &&
-	mkdir -p .git/$prefix/foo/bar/baz &&
-	printf "delete %s $C\n" $prefix/foo |
-	git update-ref --stdin
-'
-
 test_expect_success REFFILES 'D/F conflict prevents add long + delete short' '
 	df_test refs/df-al-ds --add-del foo/bar foo
 '
@@ -468,169 +396,4 @@ test_expect_success 'incorrect old value blocks indirect no-deref delete' '
 	test_cmp expected output.err
 '
 
-test_expect_success REFFILES 'non-empty directory blocks create' '
-	prefix=refs/ne-create &&
-	mkdir -p .git/$prefix/foo/bar &&
-	: >.git/$prefix/foo/bar/baz.lock &&
-	test_when_finished "rm -f .git/$prefix/foo/bar/baz.lock" &&
-	cat >expected <<-EOF &&
-	fatal: cannot lock ref $SQ$prefix/foo$SQ: there is a non-empty directory $SQ.git/$prefix/foo$SQ blocking reference $SQ$prefix/foo$SQ
-	EOF
-	printf "%s\n" "update $prefix/foo $C" |
-	test_must_fail git update-ref --stdin 2>output.err &&
-	test_cmp expected output.err &&
-	cat >expected <<-EOF &&
-	fatal: cannot lock ref $SQ$prefix/foo$SQ: unable to resolve reference $SQ$prefix/foo$SQ
-	EOF
-	printf "%s\n" "update $prefix/foo $D $C" |
-	test_must_fail git update-ref --stdin 2>output.err &&
-	test_cmp expected output.err
-'
-
-test_expect_success REFFILES 'broken reference blocks create' '
-	prefix=refs/broken-create &&
-	mkdir -p .git/$prefix &&
-	echo "gobbledigook" >.git/$prefix/foo &&
-	test_when_finished "rm -f .git/$prefix/foo" &&
-	cat >expected <<-EOF &&
-	fatal: cannot lock ref $SQ$prefix/foo$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken
-	EOF
-	printf "%s\n" "update $prefix/foo $C" |
-	test_must_fail git update-ref --stdin 2>output.err &&
-	test_cmp expected output.err &&
-	cat >expected <<-EOF &&
-	fatal: cannot lock ref $SQ$prefix/foo$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken
-	EOF
-	printf "%s\n" "update $prefix/foo $D $C" |
-	test_must_fail git update-ref --stdin 2>output.err &&
-	test_cmp expected output.err
-'
-
-test_expect_success REFFILES 'non-empty directory blocks indirect create' '
-	prefix=refs/ne-indirect-create &&
-	git symbolic-ref $prefix/symref $prefix/foo &&
-	mkdir -p .git/$prefix/foo/bar &&
-	: >.git/$prefix/foo/bar/baz.lock &&
-	test_when_finished "rm -f .git/$prefix/foo/bar/baz.lock" &&
-	cat >expected <<-EOF &&
-	fatal: cannot lock ref $SQ$prefix/symref$SQ: there is a non-empty directory $SQ.git/$prefix/foo$SQ blocking reference $SQ$prefix/foo$SQ
-	EOF
-	printf "%s\n" "update $prefix/symref $C" |
-	test_must_fail git update-ref --stdin 2>output.err &&
-	test_cmp expected output.err &&
-	cat >expected <<-EOF &&
-	fatal: cannot lock ref $SQ$prefix/symref$SQ: unable to resolve reference $SQ$prefix/foo$SQ
-	EOF
-	printf "%s\n" "update $prefix/symref $D $C" |
-	test_must_fail git update-ref --stdin 2>output.err &&
-	test_cmp expected output.err
-'
-
-test_expect_success REFFILES 'broken reference blocks indirect create' '
-	prefix=refs/broken-indirect-create &&
-	git symbolic-ref $prefix/symref $prefix/foo &&
-	echo "gobbledigook" >.git/$prefix/foo &&
-	test_when_finished "rm -f .git/$prefix/foo" &&
-	cat >expected <<-EOF &&
-	fatal: cannot lock ref $SQ$prefix/symref$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken
-	EOF
-	printf "%s\n" "update $prefix/symref $C" |
-	test_must_fail git update-ref --stdin 2>output.err &&
-	test_cmp expected output.err &&
-	cat >expected <<-EOF &&
-	fatal: cannot lock ref $SQ$prefix/symref$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken
-	EOF
-	printf "%s\n" "update $prefix/symref $D $C" |
-	test_must_fail git update-ref --stdin 2>output.err &&
-	test_cmp expected output.err
-'
-
-test_expect_success REFFILES 'no bogus intermediate values during delete' '
-	prefix=refs/slow-transaction &&
-	# Set up a reference with differing loose and packed versions:
-	git update-ref $prefix/foo $C &&
-	git pack-refs --all &&
-	git update-ref $prefix/foo $D &&
-	# Now try to update the reference, but hold the `packed-refs` lock
-	# for a while to see what happens while the process is blocked:
-	: >.git/packed-refs.lock &&
-	test_when_finished "rm -f .git/packed-refs.lock" &&
-	{
-		# Note: the following command is intentionally run in the
-		# background. We increase the timeout so that `update-ref`
-		# attempts to acquire the `packed-refs` lock for much longer
-		# than it takes for us to do the check then delete it:
-		git -c core.packedrefstimeout=30000 update-ref -d $prefix/foo &
-	} &&
-	pid2=$! &&
-	# Give update-ref plenty of time to get to the point where it tries
-	# to lock packed-refs:
-	sleep 1 &&
-	# Make sure that update-ref did not complete despite the lock:
-	kill -0 $pid2 &&
-	# Verify that the reference still has its old value:
-	sha1=$(git rev-parse --verify --quiet $prefix/foo || echo undefined) &&
-	case "$sha1" in
-	$D)
-		# This is what we hope for; it means that nothing
-		# user-visible has changed yet.
-		: ;;
-	undefined)
-		# This is not correct; it means the deletion has happened
-		# already even though update-ref should not have been
-		# able to acquire the lock yet.
-		echo "$prefix/foo deleted prematurely" &&
-		break
-		;;
-	$C)
-		# This value should never be seen. Probably the loose
-		# reference has been deleted but the packed reference
-		# is still there:
-		echo "$prefix/foo incorrectly observed to be C" &&
-		break
-		;;
-	*)
-		# WTF?
-		echo "unexpected value observed for $prefix/foo: $sha1" &&
-		break
-		;;
-	esac >out &&
-	rm -f .git/packed-refs.lock &&
-	wait $pid2 &&
-	test_must_be_empty out &&
-	test_must_fail git rev-parse --verify --quiet $prefix/foo
-'
-
-test_expect_success REFFILES 'delete fails cleanly if packed-refs file is locked' '
-	prefix=refs/locked-packed-refs &&
-	# Set up a reference with differing loose and packed versions:
-	git update-ref $prefix/foo $C &&
-	git pack-refs --all &&
-	git update-ref $prefix/foo $D &&
-	git for-each-ref $prefix >unchanged &&
-	# Now try to delete it while the `packed-refs` lock is held:
-	: >.git/packed-refs.lock &&
-	test_when_finished "rm -f .git/packed-refs.lock" &&
-	test_must_fail git update-ref -d $prefix/foo >out 2>err &&
-	git for-each-ref $prefix >actual &&
-	test_grep "Unable to create $SQ.*packed-refs.lock$SQ: " err &&
-	test_cmp unchanged actual
-'
-
-test_expect_success REFFILES 'delete fails cleanly if packed-refs.new write fails' '
-	# Setup and expectations are similar to the test above.
-	prefix=refs/failed-packed-refs &&
-	git update-ref $prefix/foo $C &&
-	git pack-refs --all &&
-	git update-ref $prefix/foo $D &&
-	git for-each-ref $prefix >unchanged &&
-	# This should not happen in practice, but it is an easy way to get a
-	# reliable error (we open with create_tempfile(), which uses O_EXCL).
-	: >.git/packed-refs.new &&
-	test_when_finished "rm -f .git/packed-refs.new" &&
-	test_must_fail git update-ref -d $prefix/foo &&
-	git for-each-ref $prefix >actual &&
-	test_cmp unchanged actual
-'
-
 test_done
-- 
gitgitgadget

[PATCH v2 05/12] t1405: move reffiles specific tests to t0601

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

Move this test to t0601 with other reffiles specific pack-refs tests
since it is reffiles specific in that it looks into the loose refs
directory for an assertion.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t0601-reffiles-pack-refs.sh | 8 ++++++++
 t/t1405-main-ref-store.sh     | 8 --------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/t/t0601-reffiles-pack-refs.sh b/t/t0601-reffiles-pack-refs.sh
index f7a3f693901..2e457c4f2df 100755
--- a/t/t0601-reffiles-pack-refs.sh
+++ b/t/t0601-reffiles-pack-refs.sh
@@ -32,6 +32,14 @@ test_expect_success 'prepare a trivial repository' '
 	HEAD=$(git rev-parse --verify HEAD)
 '
 
+test_expect_success 'pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE)' '
+	N=`find .git/refs -type f | wc -l` &&
+	test "$N" != 0 &&
+	test-tool ref-store main pack-refs PACK_REFS_PRUNE,PACK_REFS_ALL &&
+	N=`find .git/refs -type f` &&
+	test -z "$N"
+'
+
 SHA1=
 
 test_expect_success 'see if git show-ref works as expected' '
diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
index 62c1eadb190..976bd71efb5 100755
--- a/t/t1405-main-ref-store.sh
+++ b/t/t1405-main-ref-store.sh
@@ -15,14 +15,6 @@ test_expect_success 'setup' '
 	test_commit one
 '
 
-test_expect_success REFFILES 'pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE)' '
-	N=`find .git/refs -type f | wc -l` &&
-	test "$N" != 0 &&
-	$RUN pack-refs PACK_REFS_PRUNE,PACK_REFS_ALL &&
-	N=`find .git/refs -type f` &&
-	test -z "$N"
-'
-
 test_expect_success 'create_symref(FOO, refs/heads/main)' '
 	$RUN create-symref FOO refs/heads/main nothing &&
 	echo refs/heads/main >expected &&
-- 
gitgitgadget

[PATCH v2 06/12] t1406: move reffiles specific tests to t0600

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

Move this test to t0600 with the rest of the tests that are specific to
reffiles. This test reaches into reflog directories manually, and so are
specific to reffiles.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t0600-reffiles-backend.sh   | 48 +++++++++++++++++++++++++++++++++++
 t/t1407-worktree-ref-store.sh | 37 ---------------------------
 2 files changed, 48 insertions(+), 37 deletions(-)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index 2f910bd76ad..3bd28699d53 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -260,4 +260,52 @@ test_expect_success 'delete fails cleanly if packed-refs.new write fails' '
 	test_cmp unchanged actual
 '
 
+RWT="test-tool ref-store worktree:wt"
+RMAIN="test-tool ref-store worktree:main"
+
+test_expect_success 'setup worktree' '
+	test_commit first &&
+	git worktree add -b wt-main wt &&
+	(
+		cd wt &&
+		test_commit second
+	)
+'
+
+# Some refs (refs/bisect/*, pseudorefs) are kept per worktree, so they should
+# only appear in the for-each-reflog output if it is called from the correct
+# worktree, which is exercised in this test. This test is poorly written for
+# mulitple reasons: 1) it creates invalidly formatted log entres. 2) it uses
+# direct FS access for creating the reflogs. 3) PSEUDO-WT and refs/bisect/random
+# do not create reflogs by default, so it is not testing a realistic scenario.
+test_expect_success 'for_each_reflog()' '
+	echo $ZERO_OID > .git/logs/PSEUDO-MAIN &&
+	mkdir -p     .git/logs/refs/bisect &&
+	echo $ZERO_OID > .git/logs/refs/bisect/random &&
+
+	echo $ZERO_OID > .git/worktrees/wt/logs/PSEUDO-WT &&
+	mkdir -p     .git/worktrees/wt/logs/refs/bisect &&
+	echo $ZERO_OID > .git/worktrees/wt/logs/refs/bisect/wt-random &&
+
+	$RWT for-each-reflog | cut -d" " -f 2- | sort >actual &&
+	cat >expected <<-\EOF &&
+	HEAD 0x1
+	PSEUDO-WT 0x0
+	refs/bisect/wt-random 0x0
+	refs/heads/main 0x0
+	refs/heads/wt-main 0x0
+	EOF
+	test_cmp expected actual &&
+
+	$RMAIN for-each-reflog | cut -d" " -f 2- | sort >actual &&
+	cat >expected <<-\EOF &&
+	HEAD 0x1
+	PSEUDO-MAIN 0x0
+	refs/bisect/random 0x0
+	refs/heads/main 0x0
+	refs/heads/wt-main 0x0
+	EOF
+	test_cmp expected actual
+'
+
 test_done
diff --git a/t/t1407-worktree-ref-store.sh b/t/t1407-worktree-ref-store.sh
index 05b1881c591..48b1c92a414 100755
--- a/t/t1407-worktree-ref-store.sh
+++ b/t/t1407-worktree-ref-store.sh
@@ -53,41 +53,4 @@ test_expect_success 'create_symref(FOO, refs/heads/main)' '
 	test_cmp expected actual
 '
 
-# Some refs (refs/bisect/*, pseudorefs) are kept per worktree, so they should
-# only appear in the for-each-reflog output if it is called from the correct
-# worktree, which is exercised in this test. This test is poorly written (and
-# therefore marked REFFILES) for mulitple reasons: 1) it creates invalidly
-# formatted log entres. 2) it uses direct FS access for creating the reflogs. 3)
-# PSEUDO-WT and refs/bisect/random do not create reflogs by default, so it is
-# not testing a realistic scenario.
-test_expect_success REFFILES 'for_each_reflog()' '
-	echo $ZERO_OID > .git/logs/PSEUDO-MAIN &&
-	mkdir -p     .git/logs/refs/bisect &&
-	echo $ZERO_OID > .git/logs/refs/bisect/random &&
-
-	echo $ZERO_OID > .git/worktrees/wt/logs/PSEUDO-WT &&
-	mkdir -p     .git/worktrees/wt/logs/refs/bisect &&
-	echo $ZERO_OID > .git/worktrees/wt/logs/refs/bisect/wt-random &&
-
-	$RWT for-each-reflog | cut -d" " -f 2- | sort >actual &&
-	cat >expected <<-\EOF &&
-	HEAD 0x1
-	PSEUDO-WT 0x0
-	refs/bisect/wt-random 0x0
-	refs/heads/main 0x0
-	refs/heads/wt-main 0x0
-	EOF
-	test_cmp expected actual &&
-
-	$RMAIN for-each-reflog | cut -d" " -f 2- | sort >actual &&
-	cat >expected <<-\EOF &&
-	HEAD 0x1
-	PSEUDO-MAIN 0x0
-	refs/bisect/random 0x0
-	refs/heads/main 0x0
-	refs/heads/wt-main 0x0
-	EOF
-	test_cmp expected actual
-'
-
 test_done
-- 
gitgitgadget

[PATCH v2 07/12] t1410: move reffiles specific tests to t0600

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

Move these tests to t0600 with other reffiles specific tests since they
do things like take a lock on an individual ref, and write directly into
the reflog refs

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t0600-reffiles-backend.sh | 51 +++++++++++++++++++++++++++++++++++++
 t/t1410-reflog.sh           | 42 ------------------------------
 2 files changed, 51 insertions(+), 42 deletions(-)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index 3bd28699d53..44571033fac 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -308,4 +308,55 @@ test_expect_success 'for_each_reflog()' '
 	test_cmp expected actual
 '
 
+# Triggering the bug detected by this test requires a newline to fall
+# exactly BUFSIZ-1 bytes from the end of the file. We don't know
+# what that value is, since it's platform dependent. However, if
+# we choose some value N, we also catch any D which divides N evenly
+# (since we will read backwards in chunks of D). So we choose 8K,
+# which catches glibc (with an 8K BUFSIZ) and *BSD (1K).
+#
+# Each line is 114 characters, so we need 75 to still have a few before the
+# last 8K. The 89-character padding on the final entry lines up our
+# newline exactly.
+test_expect_success SHA1 'parsing reverse reflogs at BUFSIZ boundaries' '
+	git checkout -b reflogskip &&
+	zf=$(test_oid zero_2) &&
+	ident="abc <xyz> 0000000001 +0000" &&
+	for i in $(test_seq 1 75); do
+		printf "$zf%02d $zf%02d %s\t" $i $(($i+1)) "$ident" &&
+		if test $i = 75; then
+			for j in $(test_seq 1 89); do
+				printf X || return 1
+			done
+		else
+			printf X
+		fi &&
+		printf "\n" || return 1
+	done >.git/logs/refs/heads/reflogskip &&
+	git rev-parse reflogskip@{73} >actual &&
+	echo ${zf}03 >expect &&
+	test_cmp expect actual
+'
+
+# This test takes a lock on an individual ref; this is not supported in
+# reftable.
+test_expect_success 'reflog expire operates on symref not referrent' '
+	git branch --create-reflog the_symref &&
+	git branch --create-reflog referrent &&
+	git update-ref referrent HEAD &&
+	git symbolic-ref refs/heads/the_symref refs/heads/referrent &&
+	test_when_finished "rm -f .git/refs/heads/referrent.lock" &&
+	touch .git/refs/heads/referrent.lock &&
+	git reflog expire --expire=all the_symref
+'
+
+test_expect_success 'empty reflog' '
+	test_when_finished "rm -rf empty" &&
+	git init empty &&
+	test_commit -C empty A &&
+	>empty/.git/logs/refs/heads/foo &&
+	git -C empty reflog expire --all 2>err &&
+	test_must_be_empty err
+'
+
 test_done
diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
index a0ff8d51f04..d2f5f42e674 100755
--- a/t/t1410-reflog.sh
+++ b/t/t1410-reflog.sh
@@ -354,36 +354,6 @@ test_expect_success 'stale dirs do not cause d/f conflicts (reflogs off)' '
 	test_must_be_empty actual
 '
 
-# Triggering the bug detected by this test requires a newline to fall
-# exactly BUFSIZ-1 bytes from the end of the file. We don't know
-# what that value is, since it's platform dependent. However, if
-# we choose some value N, we also catch any D which divides N evenly
-# (since we will read backwards in chunks of D). So we choose 8K,
-# which catches glibc (with an 8K BUFSIZ) and *BSD (1K).
-#
-# Each line is 114 characters, so we need 75 to still have a few before the
-# last 8K. The 89-character padding on the final entry lines up our
-# newline exactly.
-test_expect_success REFFILES,SHA1 'parsing reverse reflogs at BUFSIZ boundaries' '
-	git checkout -b reflogskip &&
-	zf=$(test_oid zero_2) &&
-	ident="abc <xyz> 0000000001 +0000" &&
-	for i in $(test_seq 1 75); do
-		printf "$zf%02d $zf%02d %s\t" $i $(($i+1)) "$ident" &&
-		if test $i = 75; then
-			for j in $(test_seq 1 89); do
-				printf X || return 1
-			done
-		else
-			printf X
-		fi &&
-		printf "\n" || return 1
-	done >.git/logs/refs/heads/reflogskip &&
-	git rev-parse reflogskip@{73} >actual &&
-	echo ${zf}03 >expect &&
-	test_cmp expect actual
-'
-
 test_expect_success 'no segfaults for reflog containing non-commit sha1s' '
 	git update-ref --create-reflog -m "Creating ref" \
 		refs/tests/tree-in-reflog HEAD &&
@@ -397,18 +367,6 @@ test_expect_failure 'reflog with non-commit entries displays all entries' '
 	test_line_count = 3 actual
 '
 
-# This test takes a lock on an individual ref; this is not supported in
-# reftable.
-test_expect_success REFFILES 'reflog expire operates on symref not referrent' '
-	git branch --create-reflog the_symref &&
-	git branch --create-reflog referrent &&
-	git update-ref referrent HEAD &&
-	git symbolic-ref refs/heads/the_symref refs/heads/referrent &&
-	test_when_finished "rm -f .git/refs/heads/referrent.lock" &&
-	touch .git/refs/heads/referrent.lock &&
-	git reflog expire --expire=all the_symref
-'
-
 test_expect_success 'continue walking past root commits' '
 	git init orphanage &&
 	(
-- 
gitgitgadget

[PATCH v2 08/12] t1415: move reffiles specific tests to t0601

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

Move this test into t0601 with other reffiles pack-refs specific tests
since it checks for individua loose refs and thus is specific to the
reffiles backend.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t0601-reffiles-pack-refs.sh | 20 ++++++++++++++++++++
 t/t1415-worktree-refs.sh      | 11 -----------
 2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/t/t0601-reffiles-pack-refs.sh b/t/t0601-reffiles-pack-refs.sh
index 2e457c4f2df..c2c19befacc 100755
--- a/t/t0601-reffiles-pack-refs.sh
+++ b/t/t0601-reffiles-pack-refs.sh
@@ -308,4 +308,24 @@ test_expect_success SYMLINKS 'pack symlinked packed-refs' '
 	test "$(test_readlink .git/packed-refs)" = "my-deviant-packed-refs"
 '
 
+# The 'packed-refs' file is stored directly in .git/. This means it is global
+# to the repository, and can only contain refs that are shared across all
+# worktrees.
+test_expect_success 'refs/worktree must not be packed' '
+	test_commit initial &&
+	test_commit wt1 &&
+	test_commit wt2 &&
+	git worktree add wt1 wt1 &&
+	git worktree add wt2 wt2 &&
+	git checkout initial &&
+	git update-ref refs/worktree/foo HEAD &&
+	git -C wt1 update-ref refs/worktree/foo HEAD &&
+	git -C wt2 update-ref refs/worktree/foo HEAD &&
+	git pack-refs --all &&
+	test_path_is_missing .git/refs/tags/wt1 &&
+	test_path_is_file .git/refs/worktree/foo &&
+	test_path_is_file .git/worktrees/wt1/refs/worktree/foo &&
+	test_path_is_file .git/worktrees/wt2/refs/worktree/foo
+'
+
 test_done
diff --git a/t/t1415-worktree-refs.sh b/t/t1415-worktree-refs.sh
index 3b531842dd4..eb4eec8becb 100755
--- a/t/t1415-worktree-refs.sh
+++ b/t/t1415-worktree-refs.sh
@@ -17,17 +17,6 @@ test_expect_success 'setup' '
 	git -C wt2 update-ref refs/worktree/foo HEAD
 '
 
-# The 'packed-refs' file is stored directly in .git/. This means it is global
-# to the repository, and can only contain refs that are shared across all
-# worktrees.
-test_expect_success REFFILES 'refs/worktree must not be packed' '
-	git pack-refs --all &&
-	test_path_is_missing .git/refs/tags/wt1 &&
-	test_path_is_file .git/refs/worktree/foo &&
-	test_path_is_file .git/worktrees/wt1/refs/worktree/foo &&
-	test_path_is_file .git/worktrees/wt2/refs/worktree/foo
-'
-
 test_expect_success 'refs/worktree are per-worktree' '
 	test_cmp_rev worktree/foo initial &&
 	( cd wt1 && test_cmp_rev worktree/foo wt1 ) &&
-- 
gitgitgadget

[PATCH v2 09/12] t1503: move reffiles specific tests to t0600

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

Move this test to t0600 with other reffiles specific tests since it
checks for loose refs and is specific to the reffiles backend.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t0600-reffiles-backend.sh | 5 +++++
 t/t1503-rev-parse-verify.sh | 5 -----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index 44571033fac..a2ef34eab28 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -359,4 +359,9 @@ test_expect_success 'empty reflog' '
 	test_must_be_empty err
 '
 
+test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '
+       ln -s does-not-exist .git/refs/heads/broken &&
+       test_must_fail git rev-parse --verify broken
+'
+
 test_done
diff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh
index bc136833c10..79df65ec7f6 100755
--- a/t/t1503-rev-parse-verify.sh
+++ b/t/t1503-rev-parse-verify.sh
@@ -144,11 +144,6 @@ test_expect_success 'main@{n} for various n' '
 	test_must_fail git rev-parse --verify main@{$Np1}
 '
 
-test_expect_success SYMLINKS,REFFILES 'ref resolution not confused by broken symlinks' '
-	ln -s does-not-exist .git/refs/heads/broken &&
-	test_must_fail git rev-parse --verify broken
-'
-
 test_expect_success 'options can appear after --verify' '
 	git rev-parse --verify HEAD >expect &&
 	git rev-parse --verify -q HEAD >actual &&
-- 
gitgitgadget

[PATCH v2 10/12] t3903: make drop stash test ref backend agnostic

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

In this test, the calls to cut(1) are only used to verify that the
contents of the reflog entry look as expected. By replacing these with
git-reflog(1) calls, we can make this test ref-backend agnostic.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t3903-stash.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 34faeac3f1c..33192405155 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -200,7 +200,7 @@ test_expect_success 'drop stash reflog updates refs/stash' '
 	test_cmp expect actual
 '
 
-test_expect_success REFFILES 'drop stash reflog updates refs/stash with rewrite' '
+test_expect_success 'drop stash reflog updates refs/stash with rewrite' '
 	git init repo &&
 	(
 		cd repo &&
@@ -213,16 +213,16 @@ test_expect_success REFFILES 'drop stash reflog updates refs/stash with rewrite'
 	new_oid="$(git -C repo rev-parse stash@{0})" &&
 
 	cat >expect <<-EOF &&
-	$(test_oid zero) $old_oid
-	$old_oid $new_oid
+	$new_oid
+	$old_oid
 	EOF
-	cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
+	git -C repo reflog show refs/stash --format=%H >actual &&
 	test_cmp expect actual &&
 
 	git -C repo stash drop stash@{1} &&
-	cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
+	git -C repo reflog show refs/stash --format=%H >actual &&
 	cat >expect <<-EOF &&
-	$(test_oid zero) $new_oid
+	$new_oid
 	EOF
 	test_cmp expect actual
 '
-- 
gitgitgadget

[PATCH v2 11/12] t4202: move reffiles specific tests to t0600

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

Move two tests into t0600 since they write loose reflog refs manually
and thus are specific to the reffiles backend.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t0600-reffiles-backend.sh | 17 +++++++++++++++++
 t/t4202-log.sh              | 17 -----------------
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index a2ef34eab28..17ff60dde77 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -364,4 +364,21 @@ test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '
        test_must_fail git rev-parse --verify broken
 '
 
+test_expect_success 'log diagnoses bogus HEAD hash' '
+	git init empty &&
+	test_when_finished "rm -rf empty" &&
+	echo 1234abcd >empty/.git/refs/heads/main &&
+	test_must_fail git -C empty log 2>stderr &&
+	test_grep broken stderr
+'
+
+test_expect_success 'log diagnoses bogus HEAD symref' '
+	git init empty &&
+	test-tool -C empty ref-store main create-symref HEAD refs/heads/invalid.lock &&
+	test_must_fail git -C empty log 2>stderr &&
+	test_grep broken stderr &&
+	test_must_fail git -C empty log --default totally-bogus 2>stderr &&
+	test_grep broken stderr
+'
+
 test_done
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index ddd205f98ab..60fe60d7610 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -2255,23 +2255,6 @@ test_expect_success 'log on empty repo fails' '
 	test_grep does.not.have.any.commits stderr
 '
 
-test_expect_success REFFILES 'log diagnoses bogus HEAD hash' '
-	git init empty &&
-	test_when_finished "rm -rf empty" &&
-	echo 1234abcd >empty/.git/refs/heads/main &&
-	test_must_fail git -C empty log 2>stderr &&
-	test_grep broken stderr
-'
-
-test_expect_success REFFILES 'log diagnoses bogus HEAD symref' '
-	git init empty &&
-	test-tool -C empty ref-store main create-symref HEAD refs/heads/invalid.lock &&
-	test_must_fail git -C empty log 2>stderr &&
-	test_grep broken stderr &&
-	test_must_fail git -C empty log --default totally-bogus 2>stderr &&
-	test_grep broken stderr
-'
-
 test_expect_success 'log does not default to HEAD when rev input is given' '
 	git log --branches=does-not-exist >actual &&
 	test_must_be_empty actual
-- 
gitgitgadget

[PATCH v2 12/12] t5312: move reffiles specific tests to t0601

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

Move a few tests into t0601 since they specifically test the packed-refs
file and thus are specific to the reffiles backend.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t0601-reffiles-pack-refs.sh | 30 ++++++++++++++++++++++++++++++
 t/t5312-prune-corruption.sh   | 26 --------------------------
 2 files changed, 30 insertions(+), 26 deletions(-)

diff --git a/t/t0601-reffiles-pack-refs.sh b/t/t0601-reffiles-pack-refs.sh
index c2c19befacc..263e99cd84b 100755
--- a/t/t0601-reffiles-pack-refs.sh
+++ b/t/t0601-reffiles-pack-refs.sh
@@ -328,4 +328,34 @@ test_expect_success 'refs/worktree must not be packed' '
 	test_path_is_file .git/worktrees/wt2/refs/worktree/foo
 '
 
+# we do not want to count on running pack-refs to
+# actually pack it, as it is perfectly reasonable to
+# skip processing a broken ref
+test_expect_success 'create packed-refs file with broken ref' '
+	test_tick && git commit --allow-empty -m one &&
+	recoverable=$(git rev-parse HEAD) &&
+	test_tick && git commit --allow-empty -m two &&
+	missing=$(git rev-parse HEAD) &&
+	rm -f .git/refs/heads/main &&
+	cat >.git/packed-refs <<-EOF &&
+	$missing refs/heads/main
+	$recoverable refs/heads/other
+	EOF
+	echo $missing >expect &&
+	git rev-parse refs/heads/main >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'pack-refs does not silently delete broken packed ref' '
+	git pack-refs --all --prune &&
+	git rev-parse refs/heads/main >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'pack-refs does not drop broken refs during deletion' '
+	git update-ref -d refs/heads/other &&
+	git rev-parse refs/heads/main >actual &&
+	test_cmp expect actual
+'
+
 test_done
diff --git a/t/t5312-prune-corruption.sh b/t/t5312-prune-corruption.sh
index 230cb387122..d8d2e304687 100755
--- a/t/t5312-prune-corruption.sh
+++ b/t/t5312-prune-corruption.sh
@@ -111,30 +111,4 @@ test_expect_success 'pack-refs does not silently delete broken loose ref' '
 	test_cmp expect actual
 '
 
-# we do not want to count on running pack-refs to
-# actually pack it, as it is perfectly reasonable to
-# skip processing a broken ref
-test_expect_success REFFILES 'create packed-refs file with broken ref' '
-	rm -f .git/refs/heads/main &&
-	cat >.git/packed-refs <<-EOF &&
-	$missing refs/heads/main
-	$recoverable refs/heads/other
-	EOF
-	echo $missing >expect &&
-	git rev-parse refs/heads/main >actual &&
-	test_cmp expect actual
-'
-
-test_expect_success REFFILES 'pack-refs does not silently delete broken packed ref' '
-	git pack-refs --all --prune &&
-	git rev-parse refs/heads/main >actual &&
-	test_cmp expect actual
-'
-
-test_expect_success REFFILES  'pack-refs does not drop broken refs during deletion' '
-	git update-ref -d refs/heads/other &&
-	git rev-parse refs/heads/main >actual &&
-	test_cmp expect actual
-'
-
 test_done
-- 
gitgitgadget

Re: [PATCH v2 01/12] t3210: move to t0601

[Patrick Steinhardt]

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

On Fri, Jan 19, 2024 at 08:18:49PM +0000, John Cai via GitGitGadget wrote:
> From: John Cai <johncai86@gmail.com>
> 
> Move t3210 to t0601, since these tests are reffiles specific in that
> they modify loose refs manually. This is part of the effort to
> categorize these tests together based on the ref backend they test. When
> we upstream the reftable backend, we can add more tests to t06xx. This
> way, all tests that test specific ref backend behavior will be grouped
> together.
> 
> Signed-off-by: John Cai <johncai86@gmail.com>
> ---
>  t/{t3210-pack-refs.sh => t0601-reffiles-pack-refs.sh} | 6 ++++++
>  1 file changed, 6 insertions(+)
>  rename t/{t3210-pack-refs.sh => t0601-reffiles-pack-refs.sh} (98%)
> 
> diff --git a/t/t3210-pack-refs.sh b/t/t0601-reffiles-pack-refs.sh
> similarity index 98%
> rename from t/t3210-pack-refs.sh
> rename to t/t0601-reffiles-pack-refs.sh
> index 7f4e98db7db..f7a3f693901 100755
> --- a/t/t3210-pack-refs.sh
> +++ b/t/t0601-reffiles-pack-refs.sh
> @@ -15,6 +15,12 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>  TEST_PASSES_SANITIZE_LEAK=true
>  . ./test-lib.sh
>  
> +if ! test_have_prereq REFFILES
> +	then
> +		skip_all='skipping reffiles specific tests'
> +		test_done
> +fi

Indentation here is off as we do not typically ident the `then`. So this
should rather look like the following:

if ! test_have_prereq REFFILES
then
	skip_all='skipping reffiles specific tests'
	test_done
fi

Patrick

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

Re: [PATCH v2 04/12] t1404: move reffiles specific tests to t0600

[Patrick Steinhardt]

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

On Fri, Jan 19, 2024 at 08:18:52PM +0000, John Cai via GitGitGadget wrote:
> From: John Cai <johncai86@gmail.com>
> 
> These tests modify loose refs manually and are specific to the reffiles
> backend. Move these to t0600 to be part of a test suite of reffiles
> specific tests.
> 
> Signed-off-by: John Cai <johncai86@gmail.com>
> ---
>  t/t0600-reffiles-backend.sh  | 263 +++++++++++++++++++++++++++++++++++
>  t/t1404-update-ref-errors.sh | 237 -------------------------------
>  2 files changed, 263 insertions(+), 237 deletions(-)
>  create mode 100755 t/t0600-reffiles-backend.sh
> 
> diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
> new file mode 100755
> index 00000000000..2f910bd76ad
> --- /dev/null
> +++ b/t/t0600-reffiles-backend.sh
> @@ -0,0 +1,263 @@
> +#!/bin/sh
> +
> +test_description='Test reffiles backend'
> +
> +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
> +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> +
> +TEST_PASSES_SANITIZE_LEAK=true
> +. ./test-lib.sh
> +
> +if ! test_have_prereq REFFILES
> +	then
> +		skip_all='skipping reffiles specific tests'
> +		test_done
> +fi

Same issue here, indentation is off.

Patrick

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

Re: [PATCH v2 00/12] Group reffiles tests

[Patrick Steinhardt]

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

On Fri, Jan 19, 2024 at 08:18:48PM +0000, John Cai via GitGitGadget wrote:
> This series groups REFFILES specific tests together. These tests are
> currently grouped together across the test suite based on functionality.
> However, since they exercise low-level behavior specific to the refs backend
> being used (in these cases, the ref-files backend), group them together
> based on which refs backend they test. This way, in the near future when the
> reftables backend gets upstreamed we can add tests that exercise the
> reftables backend close by in the t06xx area.
> 
> These patches also remove the REFFILES prerequisite, since all the tests in
> t06xx are reffiles specific. In the near future, once the reftable backend
> is upstreamed, all the tests in t06xx will be forced to run with the
> reffiles backend.
> 
> Changes since V1:
> 
>  * Moved some pack-refs tests to t0601 instead of t0600
>  * Clarified some commit messages
>  * Converted a test to be refs-backend agnostic
>  * Other minor rearranging of tests

I've got two minor nits, but other than that this looks good to me. I've
also verified that all tests continue to pass with the current version
of the reftable backend.

There's a minor merge conflict with db4192c364 (t: mark tests regarding
git-pack-refs(1) to be backend specific, 2024-01-10). This conflict
comes from the fact that both patch series add the REFFILES prereq to
t3210, semantically the changes are the same. So it doesn't quite matter
which of both versions we retain as they both do the same.

Patrick

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

Re: [PATCH v2 07/12] t1410: move reffiles specific tests to t0600

[Karthik Nayak]

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

"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: John Cai <johncai86@gmail.com>
>
> Move these tests to t0600 with other reffiles specific tests since they
> do things like take a lock on an individual ref, and write directly into
> the reflog refs
>

Nit: missing period.

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

Re: [PATCH v2 08/12] t1415: move reffiles specific tests to t0601

[Karthik Nayak]

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

"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: John Cai <johncai86@gmail.com>
>
> Move this test into t0601 with other reffiles pack-refs specific tests
> since it checks for individua loose refs and thus is specific to the

Nit: s/individua/individual

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

Re: [PATCH v2 00/12] Group reffiles tests

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> I've got two minor nits, but other than that this looks good to me. I've
> also verified that all tests continue to pass with the current version
> of the reftable backend.

OK.  I've squashed all the nits from you and Karthik into the copy
in my tree.  If there is nothing else, let's declare a victory and
merge the topic down to 'next' soonish.

> There's a minor merge conflict with db4192c364 (t: mark tests regarding
> git-pack-refs(1) to be backend specific, 2024-01-10). This conflict
> comes from the fact that both patch series add the REFFILES prereq to
> t3210, semantically the changes are the same. So it doesn't quite matter
> which of both versions we retain as they both do the same.

Yup, that is what I've been resolving them.

Thanks.

Re: [PATCH v2 00/12] Group reffiles tests

[John Cai]

Hi Junio,

On 22 Jan 2024, at 19:01, Junio C Hamano wrote:

> Patrick Steinhardt <ps@pks.im> writes:
>
>> I've got two minor nits, but other than that this looks good to me. I've
>> also verified that all tests continue to pass with the current version
>> of the reftable backend.
>
> OK.  I've squashed all the nits from you and Karthik into the copy
> in my tree.  If there is nothing else, let's declare a victory and
> merge the topic down to 'next' soonish.

Thank you for doing these tedious corrections!
>
>> There's a minor merge conflict with db4192c364 (t: mark tests regarding
>> git-pack-refs(1) to be backend specific, 2024-01-10). This conflict
>> comes from the fact that both patch series add the REFFILES prereq to
>> t3210, semantically the changes are the same. So it doesn't quite matter
>> which of both versions we retain as they both do the same.
>
> Yup, that is what I've been resolving them.
>
> Thanks.

thanks
John