git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/7] t: drop more REFFILES prereqs
@ 2024-02-09  7:23 Patrick Steinhardt
  2024-02-09  7:23 ` [PATCH 1/7] t: move tests exercising the "files" backend Patrick Steinhardt
                   ` (9 more replies)
  0 siblings, 10 replies; 28+ messages in thread
From: Patrick Steinhardt @ 2024-02-09  7:23 UTC (permalink / raw
  To: git

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

Hi,

this patch series is another step towards less tests with the REFFILES
prerequisite. Some of the tests are simply moved into t0600 because they
are inherently exercising low-level behaviour of the "files" backend.
Other tests are adapted so that they work across both the "files" and
the "reftable" backends.

Patrick

Patrick Steinhardt (7):
  t: move tests exercising the "files" backend
  t0410: enable tests with extensions with non-default repo format
  t1400: exercise reflog with gaps with reftable backend
  t1404: make D/F conflict tests compatible with reftable backend
  t1405: remove unneeded cleanup step
  t2011: exercise D/F conflicts with HEAD with the reftable backend
  t7003: ensure filter-branch prunes reflogs with the reftable backend

 t/t0410-partial-clone.sh         |  6 +--
 t/t0600-reffiles-backend.sh      | 91 ++++++++++++++++++++++++++++++++
 t/t1301-shared-repo.sh           | 16 ------
 t/t1400-update-ref.sh            | 50 +++---------------
 t/t1404-update-ref-errors.sh     | 37 ++++++-------
 t/t1405-main-ref-store.sh        |  6 ---
 t/t2011-checkout-invalid-head.sh | 17 +++---
 t/t3200-branch.sh                | 29 ----------
 t/t3400-rebase.sh                | 10 ----
 t/t7003-filter-branch.sh         |  5 +-
 10 files changed, 125 insertions(+), 142 deletions(-)

-- 
2.43.GIT


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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 1/7] t: move tests exercising the "files" backend
  2024-02-09  7:23 [PATCH 0/7] t: drop more REFFILES prereqs Patrick Steinhardt
@ 2024-02-09  7:23 ` Patrick Steinhardt
  2024-02-14 22:45   ` Junio C Hamano
  2024-02-09  7:23 ` [PATCH 2/7] t0410: enable tests with extensions with non-default repo format Patrick Steinhardt
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 28+ messages in thread
From: Patrick Steinhardt @ 2024-02-09  7:23 UTC (permalink / raw
  To: git

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

We still have a bunch of tests scattered across our test suites that
exercise on-disk files of the "files" backend directly:

  - t1301 exercises permissions of reflog files when the config
    "core.sharedRepository" is set.

  - t1400 exercises whether empty directories in the ref store are
    handled correctly.

  - t3200 exercises what happens when there are symlinks in the ref
    store.

  - t3400 also exercises what happens when ".git/logs" is a symlink.

All of these are inherently low-level tests specific to the "files"
backend. Move them into "t0600-reffiles-backend.sh" to reflect this.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0600-reffiles-backend.sh | 91 +++++++++++++++++++++++++++++++++++++
 t/t1301-shared-repo.sh      | 16 -------
 t/t1400-update-ref.sh       | 36 ---------------
 t/t3200-branch.sh           | 29 ------------
 t/t3400-rebase.sh           | 10 ----
 5 files changed, 91 insertions(+), 91 deletions(-)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index e6a5f1868f..485481d6b4 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -381,4 +381,95 @@ test_expect_success 'log diagnoses bogus HEAD symref' '
 	test_grep broken stderr
 '
 
+test_expect_success 'empty directory removal' '
+	git branch d1/d2/r1 HEAD &&
+	git branch d1/r2 HEAD &&
+	test_path_is_file .git/refs/heads/d1/d2/r1 &&
+	test_path_is_file .git/logs/refs/heads/d1/d2/r1 &&
+	git branch -d d1/d2/r1 &&
+	test_must_fail git show-ref --verify -q refs/heads/d1/d2 &&
+	test_must_fail git show-ref --verify -q logs/refs/heads/d1/d2 &&
+	test_path_is_file .git/refs/heads/d1/r2 &&
+	test_path_is_file .git/logs/refs/heads/d1/r2
+'
+
+test_expect_success 'symref empty directory removal' '
+	git branch e1/e2/r1 HEAD &&
+	git branch e1/r2 HEAD &&
+	git checkout e1/e2/r1 &&
+	test_when_finished "git checkout main" &&
+	test_path_is_file .git/refs/heads/e1/e2/r1 &&
+	test_path_is_file .git/logs/refs/heads/e1/e2/r1 &&
+	git update-ref -d HEAD &&
+	test_must_fail git show-ref --verify -q refs/heads/e1/e2 &&
+	test_must_fail git show-ref --verify -q logs/refs/heads/e1/e2 &&
+	test_path_is_file .git/refs/heads/e1/r2 &&
+	test_path_is_file .git/logs/refs/heads/e1/r2 &&
+	test_path_is_file .git/logs/HEAD
+'
+
+test_expect_success 'directory not created deleting packed ref' '
+	git branch d1/d2/r1 HEAD &&
+	git pack-refs --all &&
+	test_path_is_missing .git/refs/heads/d1/d2 &&
+	git update-ref -d refs/heads/d1/d2/r1 &&
+	test_path_is_missing .git/refs/heads/d1/d2 &&
+	test_path_is_missing .git/refs/heads/d1
+'
+
+test_expect_success SYMLINKS 'git branch -m u v should fail when the reflog for u is a symlink' '
+	git branch --create-reflog u &&
+	mv .git/logs/refs/heads/u real-u &&
+	ln -s real-u .git/logs/refs/heads/u &&
+	test_must_fail git branch -m u v
+'
+
+test_expect_success SYMLINKS 'git branch -m with symlinked .git/refs' '
+	test_when_finished "rm -rf subdir" &&
+	git init --bare subdir &&
+
+	rm -rfv subdir/refs subdir/objects subdir/packed-refs &&
+	ln -s ../.git/refs subdir/refs &&
+	ln -s ../.git/objects subdir/objects &&
+	ln -s ../.git/packed-refs subdir/packed-refs &&
+
+	git -C subdir rev-parse --absolute-git-dir >subdir.dir &&
+	git rev-parse --absolute-git-dir >our.dir &&
+	! test_cmp subdir.dir our.dir &&
+
+	git -C subdir log &&
+	git -C subdir branch rename-src &&
+	git rev-parse rename-src >expect &&
+	git -C subdir branch -m rename-src rename-dest &&
+	git rev-parse rename-dest >actual &&
+	test_cmp expect actual &&
+	git branch -D rename-dest
+'
+
+test_expect_success MINGW,SYMLINKS_WINDOWS 'rebase when .git/logs is a symlink' '
+	git checkout main &&
+	mv .git/logs actual_logs &&
+	cmd //c "mklink /D .git\logs ..\actual_logs" &&
+	git rebase -f HEAD^ &&
+	test -L .git/logs &&
+	rm .git/logs &&
+	mv actual_logs .git/logs
+'
+
+test_expect_success POSIXPERM 'git reflog expire honors core.sharedRepository' '
+	umask 077 &&
+	git config core.sharedRepository group &&
+	git reflog expire --all &&
+	actual="$(ls -l .git/logs/refs/heads/main)" &&
+	case "$actual" in
+	-rw-rw-*)
+		: happy
+		;;
+	*)
+		echo Ooops, .git/logs/refs/heads/main is not 066x [$actual]
+		false
+		;;
+	esac
+'
+
 test_done
diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
index 8e2c01e760..b1eb5c01b8 100755
--- a/t/t1301-shared-repo.sh
+++ b/t/t1301-shared-repo.sh
@@ -137,22 +137,6 @@ test_expect_success POSIXPERM 'info/refs respects umask in unshared repo' '
 	test_cmp expect actual
 '
 
-test_expect_success REFFILES,POSIXPERM 'git reflog expire honors core.sharedRepository' '
-	umask 077 &&
-	git config core.sharedRepository group &&
-	git reflog expire --all &&
-	actual="$(ls -l .git/logs/refs/heads/main)" &&
-	case "$actual" in
-	-rw-rw-*)
-		: happy
-		;;
-	*)
-		echo Ooops, .git/logs/refs/heads/main is not 066x [$actual]
-		false
-		;;
-	esac
-'
-
 test_expect_success POSIXPERM 'forced modes' '
 	test_when_finished "rm -rf new" &&
 	mkdir -p templates/hooks &&
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index f18843bf7a..3294b7ce08 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -288,33 +288,6 @@ test_expect_success "set $m (logged by touch)" '
 	test $A = $(git show-ref -s --verify $m)
 '
 
-test_expect_success REFFILES 'empty directory removal' '
-	git branch d1/d2/r1 HEAD &&
-	git branch d1/r2 HEAD &&
-	test_path_is_file .git/refs/heads/d1/d2/r1 &&
-	test_path_is_file .git/logs/refs/heads/d1/d2/r1 &&
-	git branch -d d1/d2/r1 &&
-	test_must_fail git show-ref --verify -q refs/heads/d1/d2 &&
-	test_must_fail git show-ref --verify -q logs/refs/heads/d1/d2 &&
-	test_path_is_file .git/refs/heads/d1/r2 &&
-	test_path_is_file .git/logs/refs/heads/d1/r2
-'
-
-test_expect_success REFFILES 'symref empty directory removal' '
-	git branch e1/e2/r1 HEAD &&
-	git branch e1/r2 HEAD &&
-	git checkout e1/e2/r1 &&
-	test_when_finished "git checkout main" &&
-	test_path_is_file .git/refs/heads/e1/e2/r1 &&
-	test_path_is_file .git/logs/refs/heads/e1/e2/r1 &&
-	git update-ref -d HEAD &&
-	test_must_fail git show-ref --verify -q refs/heads/e1/e2 &&
-	test_must_fail git show-ref --verify -q logs/refs/heads/e1/e2 &&
-	test_path_is_file .git/refs/heads/e1/r2 &&
-	test_path_is_file .git/logs/refs/heads/e1/r2 &&
-	test_path_is_file .git/logs/HEAD
-'
-
 cat >expect <<EOF
 $Z $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000	Initial Creation
 $A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150260 +0000	Switch
@@ -1668,13 +1641,4 @@ test_expect_success PIPE 'transaction flushes status updates' '
 	test_cmp expected actual
 '
 
-test_expect_success REFFILES 'directory not created deleting packed ref' '
-	git branch d1/d2/r1 HEAD &&
-	git pack-refs --all &&
-	test_path_is_missing .git/refs/heads/d1/d2 &&
-	git update-ref -d refs/heads/d1/d2/r1 &&
-	test_path_is_missing .git/refs/heads/d1/d2 &&
-	test_path_is_missing .git/refs/heads/d1
-'
-
 test_done
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index de7d3014e4..e36f4d15f2 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -836,35 +836,6 @@ test_expect_success 'renaming a symref is not allowed' '
 	test_ref_missing refs/heads/new-topic
 '
 
-test_expect_success SYMLINKS,REFFILES 'git branch -m u v should fail when the reflog for u is a symlink' '
-	git branch --create-reflog u &&
-	mv .git/logs/refs/heads/u real-u &&
-	ln -s real-u .git/logs/refs/heads/u &&
-	test_must_fail git branch -m u v
-'
-
-test_expect_success SYMLINKS,REFFILES 'git branch -m with symlinked .git/refs' '
-	test_when_finished "rm -rf subdir" &&
-	git init --bare subdir &&
-
-	rm -rfv subdir/refs subdir/objects subdir/packed-refs &&
-	ln -s ../.git/refs subdir/refs &&
-	ln -s ../.git/objects subdir/objects &&
-	ln -s ../.git/packed-refs subdir/packed-refs &&
-
-	git -C subdir rev-parse --absolute-git-dir >subdir.dir &&
-	git rev-parse --absolute-git-dir >our.dir &&
-	! test_cmp subdir.dir our.dir &&
-
-	git -C subdir log &&
-	git -C subdir branch rename-src &&
-	git rev-parse rename-src >expect &&
-	git -C subdir branch -m rename-src rename-dest &&
-	git rev-parse rename-dest >actual &&
-	test_cmp expect actual &&
-	git branch -D rename-dest
-'
-
 test_expect_success 'test tracking setup via --track' '
 	git config remote.local.url . &&
 	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 57f1392926..e1c8c5f701 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -424,16 +424,6 @@ test_expect_success 'refuse to switch to branch checked out elsewhere' '
 	test_grep "already used by worktree at" err
 '
 
-test_expect_success REFFILES,MINGW,SYMLINKS_WINDOWS 'rebase when .git/logs is a symlink' '
-	git checkout main &&
-	mv .git/logs actual_logs &&
-	cmd //c "mklink /D .git\logs ..\actual_logs" &&
-	git rebase -f HEAD^ &&
-	test -L .git/logs &&
-	rm .git/logs &&
-	mv actual_logs .git/logs
-'
-
 test_expect_success 'rebase when inside worktree subdirectory' '
 	git init main-wt &&
 	(
-- 
2.43.GIT


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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 2/7] t0410: enable tests with extensions with non-default repo format
  2024-02-09  7:23 [PATCH 0/7] t: drop more REFFILES prereqs Patrick Steinhardt
  2024-02-09  7:23 ` [PATCH 1/7] t: move tests exercising the "files" backend Patrick Steinhardt
@ 2024-02-09  7:23 ` Patrick Steinhardt
  2024-02-14 22:57   ` Junio C Hamano
  2024-02-09  7:23 ` [PATCH 3/7] t1400: exercise reflog with gaps with reftable backend Patrick Steinhardt
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 28+ messages in thread
From: Patrick Steinhardt @ 2024-02-09  7:23 UTC (permalink / raw
  To: git

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

In t0410 we have two tests which exercise how partial clones behave in
the context of a repository with extensions. These tests are marked to
require a default repository using SHA1 and the "files" backend because
we explicitly set the repository format version to 0.

Changing the repository format version to 0 is not needed though. The
"noop" extension is ignored as expected regardless of what the version
is set to, same as the "nonsense" extension leads to failure regardless
of the version.

Stop setting the version so that these tests can execute with SHA256 and
"reftable" repositories.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0410-partial-clone.sh | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
index 6b6424b3df..d913f3c453 100755
--- a/t/t0410-partial-clone.sh
+++ b/t/t0410-partial-clone.sh
@@ -49,24 +49,22 @@ test_expect_success 'convert shallow clone to partial clone' '
 	test_cmp_config -C client 1 core.repositoryformatversion
 '
 
-test_expect_success SHA1,REFFILES 'convert to partial clone with noop extension' '
+test_expect_success 'convert to partial clone with noop extension' '
 	rm -fr server client &&
 	test_create_repo server &&
 	test_commit -C server my_commit 1 &&
 	test_commit -C server my_commit2 1 &&
 	git clone --depth=1 "file://$(pwd)/server" client &&
-	test_cmp_config -C client 0 core.repositoryformatversion &&
 	git -C client config extensions.noop true &&
 	git -C client fetch --unshallow --filter="blob:none"
 '
 
-test_expect_success SHA1,REFFILES 'converting to partial clone fails with unrecognized extension' '
+test_expect_success 'converting to partial clone fails with unrecognized extension' '
 	rm -fr server client &&
 	test_create_repo server &&
 	test_commit -C server my_commit 1 &&
 	test_commit -C server my_commit2 1 &&
 	git clone --depth=1 "file://$(pwd)/server" client &&
-	test_cmp_config -C client 0 core.repositoryformatversion &&
 	git -C client config extensions.nonsense true &&
 	test_must_fail git -C client fetch --unshallow --filter="blob:none"
 '
-- 
2.43.GIT


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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 3/7] t1400: exercise reflog with gaps with reftable backend
  2024-02-09  7:23 [PATCH 0/7] t: drop more REFFILES prereqs Patrick Steinhardt
  2024-02-09  7:23 ` [PATCH 1/7] t: move tests exercising the "files" backend Patrick Steinhardt
  2024-02-09  7:23 ` [PATCH 2/7] t0410: enable tests with extensions with non-default repo format Patrick Steinhardt
@ 2024-02-09  7:23 ` Patrick Steinhardt
  2024-02-14 22:59   ` Junio C Hamano
  2024-02-09  7:23 ` [PATCH 4/7] t1404: make D/F conflict tests compatible " Patrick Steinhardt
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 28+ messages in thread
From: Patrick Steinhardt @ 2024-02-09  7:23 UTC (permalink / raw
  To: git

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

In t1400, we have a test that exercises whether we print a warning
message as expected when the reflog contains entries which have a gap
between the old entry's new object ID and the new entry's old object ID.
While the logic should apply to all ref backends, the test setup writes
into `.git/logs` directly and is thus "files"-backend specific.

Refactor the test to instead use `git reflog delete` to create the gap
and drop the REFFILES prerequisite.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t1400-update-ref.sh | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 3294b7ce08..3aeb103d34 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -426,15 +426,15 @@ test_expect_success 'Query "main@{2005-05-28}" (past end of history)' '
 rm -f expect
 git update-ref -d $m
 
-test_expect_success REFFILES 'query reflog with gap' '
+test_expect_success 'query reflog with gap' '
 	test_when_finished "git update-ref -d $m" &&
 
-	git update-ref $m $F &&
-	cat >.git/logs/$m <<-EOF &&
-	$Z $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150320 -0500
-	$A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150380 -0500
-	$D $F $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150680 -0500
-	EOF
+	GIT_COMMITTER_DATE="1117150320 -0500" git update-ref $m $A &&
+	GIT_COMMITTER_DATE="1117150380 -0500" git update-ref $m $B &&
+	GIT_COMMITTER_DATE="1117150480 -0500" git update-ref $m $C &&
+	GIT_COMMITTER_DATE="1117150580 -0500" git update-ref $m $D &&
+	GIT_COMMITTER_DATE="1117150680 -0500" git update-ref $m $F &&
+	git reflog delete $m@{2} &&
 
 	git rev-parse --verify "main@{2005-05-26 23:33:01}" >actual 2>stderr &&
 	echo "$B" >expect &&
-- 
2.43.GIT


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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 4/7] t1404: make D/F conflict tests compatible with reftable backend
  2024-02-09  7:23 [PATCH 0/7] t: drop more REFFILES prereqs Patrick Steinhardt
                   ` (2 preceding siblings ...)
  2024-02-09  7:23 ` [PATCH 3/7] t1400: exercise reflog with gaps with reftable backend Patrick Steinhardt
@ 2024-02-09  7:23 ` Patrick Steinhardt
  2024-02-14 23:11   ` Junio C Hamano
  2024-02-09  7:23 ` [PATCH 5/7] t1405: remove unneeded cleanup step Patrick Steinhardt
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 28+ messages in thread
From: Patrick Steinhardt @ 2024-02-09  7:23 UTC (permalink / raw
  To: git

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

Some of the tests in t1404 exercise whether Git correctly aborts
transactions when there is a directory/file conflict with ref names.
While these tests are all marked to require the "files" backend, they do
in fact apply to the "reftable" backend as well.

This may not make much sense on the surface: D/F conflicts only exist
because the "files" backend uses the filesystem to store loose refs, and
thus the restriction theoretically shouldn't apply to the "reftable"
backend. But for now, the "reftable" backend artificially restricts the
creation of such conflicting refs so that it is a drop-in replacement
for the "files" backend. This also ensures that the "reftable" backend
can easily be used on the server side without causing issues for clients
which only know to use the "files" backend.

The only difference between the "files" and "reftable" backends is a
slightly different error message. Adapt the tests to accomodate for this
difference and remove the REFFILES prerequisite so that we start testing
with both backends.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t1404-update-ref-errors.sh | 37 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/t/t1404-update-ref-errors.sh b/t/t1404-update-ref-errors.sh
index 00b7013705..98e9158bd2 100755
--- a/t/t1404-update-ref-errors.sh
+++ b/t/t1404-update-ref-errors.sh
@@ -92,9 +92,6 @@ df_test() {
 	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
@@ -103,7 +100,7 @@ df_test() {
 		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 &&
+	grep "fatal:\( cannot lock ref $SQ$addname$SQ:\)\? $SQ$delref$SQ exists; cannot create $SQ$addref$SQ" 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
@@ -191,69 +188,69 @@ test_expect_success 'one new ref is a simple prefix of another' '
 
 '
 
-test_expect_success REFFILES 'D/F conflict prevents add long + delete short' '
+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 REFFILES 'D/F conflict prevents add short + delete long' '
+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 REFFILES 'D/F conflict prevents delete long + add short' '
+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 REFFILES 'D/F conflict prevents delete short + add long' '
+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 REFFILES 'D/F conflict prevents add long + delete short packed' '
+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 REFFILES 'D/F conflict prevents add short + delete long packed' '
+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 REFFILES 'D/F conflict prevents delete long packed + add short' '
+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 REFFILES 'D/F conflict prevents delete short packed + add long' '
+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 REFFILES 'D/F conflict prevents indirect add long + delete short' '
+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 REFFILES 'D/F conflict prevents indirect add long + indirect delete short' '
+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 REFFILES 'D/F conflict prevents indirect add short + indirect delete long' '
+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 REFFILES 'D/F conflict prevents indirect delete long + indirect add short' '
+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 REFFILES 'D/F conflict prevents indirect add long + delete short packed' '
+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 REFFILES 'D/F conflict prevents indirect add long + indirect delete short packed' '
+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 REFFILES 'D/F conflict prevents add long + indirect delete short packed' '
+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 REFFILES 'D/F conflict prevents indirect delete long packed + indirect add short' '
+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
 '
 
-- 
2.43.GIT


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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 5/7] t1405: remove unneeded cleanup step
  2024-02-09  7:23 [PATCH 0/7] t: drop more REFFILES prereqs Patrick Steinhardt
                   ` (3 preceding siblings ...)
  2024-02-09  7:23 ` [PATCH 4/7] t1404: make D/F conflict tests compatible " Patrick Steinhardt
@ 2024-02-09  7:23 ` Patrick Steinhardt
  2024-02-14 23:17   ` Junio C Hamano
  2024-02-09  7:23 ` [PATCH 6/7] t2011: exercise D/F conflicts with HEAD with the reftable backend Patrick Steinhardt
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 28+ messages in thread
From: Patrick Steinhardt @ 2024-02-09  7:23 UTC (permalink / raw
  To: git

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

In 5e00514745 (t1405: explictly delete reflogs for reftable, 2022-01-31)
we have added a test that explicitly deletes the reflog when not using
the "files" backend. This was required because back then, the "reftable"
backend didn't yet delete reflogs when deleting their corresponding
branches, and thus subsequent tests would fail because some unexpected
reflogs still exist.

The "reftable" backend was eventually changed though so that it behaves
the same as the "files" backend and deletes reflogs when deleting refs.
This was done to make the "reftable" backend behave like the "files"
backend as closely as possible so that it can act as a drop-in
replacement.

The cleanup-style test is thus not required anymore. Remove it.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t1405-main-ref-store.sh | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
index 976bd71efb..1183232a72 100755
--- a/t/t1405-main-ref-store.sh
+++ b/t/t1405-main-ref-store.sh
@@ -33,12 +33,6 @@ test_expect_success 'delete_refs(FOO, refs/tags/new-tag)' '
 	test_must_fail git rev-parse refs/tags/new-tag --
 '
 
-# In reftable, we keep the reflogs around for deleted refs.
-test_expect_success !REFFILES 'delete-reflog(FOO, refs/tags/new-tag)' '
-	$RUN delete-reflog FOO &&
-	$RUN delete-reflog refs/tags/new-tag
-'
-
 test_expect_success 'rename_refs(main, new-main)' '
 	git rev-parse main >expected &&
 	$RUN rename-ref refs/heads/main refs/heads/new-main &&
-- 
2.43.GIT


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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 6/7] t2011: exercise D/F conflicts with HEAD with the reftable backend
  2024-02-09  7:23 [PATCH 0/7] t: drop more REFFILES prereqs Patrick Steinhardt
                   ` (4 preceding siblings ...)
  2024-02-09  7:23 ` [PATCH 5/7] t1405: remove unneeded cleanup step Patrick Steinhardt
@ 2024-02-09  7:23 ` Patrick Steinhardt
  2024-02-09  7:23 ` [PATCH 7/7] t7003: ensure filter-branch prunes reflogs " Patrick Steinhardt
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 28+ messages in thread
From: Patrick Steinhardt @ 2024-02-09  7:23 UTC (permalink / raw
  To: git

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

Some of the tests in t2011 exercise whether it is possible to move away
from a symbolic HEAD ref whose target ref has a directory-file conflict
with another, preexisting ref. These tests don't use git-symbolic-ref(1)
but manually write HEAD. This is supposedly done to avoid using logic
that we're about to exercise, but it makes it impossible to verify
whether the logic also works for ref backends other than "files".

Refactor the code to use git-symbolic-ref(1) instead so that the tests
work with the "reftable" backend, as well. We already have lots of tests
in t1404 that ensure that both git-update-ref(1) and git-symbolic-ref(1)
work in such a scenario, so it should be safe to rely on it here.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t2011-checkout-invalid-head.sh | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/t/t2011-checkout-invalid-head.sh b/t/t2011-checkout-invalid-head.sh
index 3c8135831b..04f53b1ea1 100755
--- a/t/t2011-checkout-invalid-head.sh
+++ b/t/t2011-checkout-invalid-head.sh
@@ -29,36 +29,33 @@ test_expect_success REFFILES 'checkout notices failure to lock HEAD' '
 	test_must_fail git checkout -b other
 '
 
-test_expect_success REFFILES 'create ref directory/file conflict scenario' '
+test_expect_success 'create ref directory/file conflict scenario' '
 	git update-ref refs/heads/outer/inner main &&
-
-	# do not rely on symbolic-ref to get a known state,
-	# as it may use the same code we are testing
 	reset_to_df () {
-		echo "ref: refs/heads/outer" >.git/HEAD
+		git symbolic-ref HEAD refs/heads/outer
 	}
 '
 
-test_expect_success REFFILES 'checkout away from d/f HEAD (unpacked, to branch)' '
+test_expect_success 'checkout away from d/f HEAD (unpacked, to branch)' '
 	reset_to_df &&
 	git checkout main
 '
 
-test_expect_success REFFILES 'checkout away from d/f HEAD (unpacked, to detached)' '
+test_expect_success 'checkout away from d/f HEAD (unpacked, to detached)' '
 	reset_to_df &&
 	git checkout --detach main
 '
 
-test_expect_success REFFILES 'pack refs' '
+test_expect_success 'pack refs' '
 	git pack-refs --all --prune
 '
 
-test_expect_success REFFILES 'checkout away from d/f HEAD (packed, to branch)' '
+test_expect_success 'checkout away from d/f HEAD (packed, to branch)' '
 	reset_to_df &&
 	git checkout main
 '
 
-test_expect_success REFFILES 'checkout away from d/f HEAD (packed, to detached)' '
+test_expect_success 'checkout away from d/f HEAD (packed, to detached)' '
 	reset_to_df &&
 	git checkout --detach main
 '
-- 
2.43.GIT


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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 7/7] t7003: ensure filter-branch prunes reflogs with the reftable backend
  2024-02-09  7:23 [PATCH 0/7] t: drop more REFFILES prereqs Patrick Steinhardt
                   ` (5 preceding siblings ...)
  2024-02-09  7:23 ` [PATCH 6/7] t2011: exercise D/F conflicts with HEAD with the reftable backend Patrick Steinhardt
@ 2024-02-09  7:23 ` Patrick Steinhardt
  2024-02-11 14:00 ` [PATCH 0/7] t: drop more REFFILES prereqs Karthik Nayak
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 28+ messages in thread
From: Patrick Steinhardt @ 2024-02-09  7:23 UTC (permalink / raw
  To: git

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

In t7003 we conditionally check whether the reflog for branches pruned
by git-filter-branch(1) get deleted based on whether or not we use the
"files" backend. Same as with the preceding commit, this condition was
added because in its initial iteration the "reftable" backend did not
delete reflogs when their corresponding ref was deleted. Since then, the
backend has been aligned to behave the same as the "files" backend
though, which makes this check unnecessary.

Remove it.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t7003-filter-branch.sh | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index f6aebe92ff..5ab4d41ee7 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -396,10 +396,7 @@ test_expect_success '--prune-empty is able to prune entire branch' '
 	git branch prune-entire B &&
 	git filter-branch -f --prune-empty --index-filter "git update-index --remove A.t B.t" prune-entire &&
 	test_must_fail git rev-parse refs/heads/prune-entire &&
-	if test_have_prereq REFFILES
-	then
-		test_must_fail git reflog exists refs/heads/prune-entire
-	fi
+	test_must_fail git reflog exists refs/heads/prune-entire
 '
 
 test_expect_success '--remap-to-ancestor with filename filters' '
-- 
2.43.GIT


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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH 0/7] t: drop more REFFILES prereqs
  2024-02-09  7:23 [PATCH 0/7] t: drop more REFFILES prereqs Patrick Steinhardt
                   ` (6 preceding siblings ...)
  2024-02-09  7:23 ` [PATCH 7/7] t7003: ensure filter-branch prunes reflogs " Patrick Steinhardt
@ 2024-02-11 14:00 ` Karthik Nayak
  2024-02-14 23:20 ` Junio C Hamano
  2024-02-15  8:25 ` [PATCH v2 " Patrick Steinhardt
  9 siblings, 0 replies; 28+ messages in thread
From: Karthik Nayak @ 2024-02-11 14:00 UTC (permalink / raw
  To: Patrick Steinhardt, git

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

Hello,

Patrick Steinhardt <ps@pks.im> writes:
> Hi,
>
> this patch series is another step towards less tests with the REFFILES
> prerequisite. Some of the tests are simply moved into t0600 because they
> are inherently exercising low-level behaviour of the "files" backend.
> Other tests are adapted so that they work across both the "files" and
> the "reftable" backends.
>
> Patrick
>

I had a look at this series and have nothing to add. Looks great!

Thanks!
Karthik

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/7] t: move tests exercising the "files" backend
  2024-02-09  7:23 ` [PATCH 1/7] t: move tests exercising the "files" backend Patrick Steinhardt
@ 2024-02-14 22:45   ` Junio C Hamano
  0 siblings, 0 replies; 28+ messages in thread
From: Junio C Hamano @ 2024-02-14 22:45 UTC (permalink / raw
  To: Patrick Steinhardt; +Cc: git

Patrick Steinhardt <ps@pks.im> writes:

> We still have a bunch of tests scattered across our test suites that
> exercise on-disk files of the "files" backend directly:
>
>   - t1301 exercises permissions of reflog files when the config
>     "core.sharedRepository" is set.
>
>   - t1400 exercises whether empty directories in the ref store are
>     handled correctly.
>
>   - t3200 exercises what happens when there are symlinks in the ref
>     store.
>
>   - t3400 also exercises what happens when ".git/logs" is a symlink.
>
> All of these are inherently low-level tests specific to the "files"
> backend. Move them into "t0600-reffiles-backend.sh" to reflect this.

Makes sense, and they look like straight code movements.

> diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
> index e6a5f1868f..485481d6b4 100755
> --- a/t/t0600-reffiles-backend.sh
> +++ b/t/t0600-reffiles-backend.sh
> @@ -381,4 +381,95 @@ test_expect_success 'log diagnoses bogus HEAD symref' '
>  	test_grep broken stderr
>  '

These two are from t1400.

> +test_expect_success 'empty directory removal' '
> +	git branch d1/d2/r1 HEAD &&
> +	git branch d1/r2 HEAD &&
> +	test_path_is_file .git/refs/heads/d1/d2/r1 &&
> +	test_path_is_file .git/logs/refs/heads/d1/d2/r1 &&
> +	git branch -d d1/d2/r1 &&
> +	test_must_fail git show-ref --verify -q refs/heads/d1/d2 &&
> +	test_must_fail git show-ref --verify -q logs/refs/heads/d1/d2 &&
> +	test_path_is_file .git/refs/heads/d1/r2 &&
> +	test_path_is_file .git/logs/refs/heads/d1/r2
> +'
> +
> +test_expect_success 'symref empty directory removal' '
> +	git branch e1/e2/r1 HEAD &&
> +	git branch e1/r2 HEAD &&
> +	git checkout e1/e2/r1 &&
> +	test_when_finished "git checkout main" &&
> +	test_path_is_file .git/refs/heads/e1/e2/r1 &&
> +	test_path_is_file .git/logs/refs/heads/e1/e2/r1 &&
> +	git update-ref -d HEAD &&
> +	test_must_fail git show-ref --verify -q refs/heads/e1/e2 &&
> +	test_must_fail git show-ref --verify -q logs/refs/heads/e1/e2 &&
> +	test_path_is_file .git/refs/heads/e1/r2 &&
> +	test_path_is_file .git/logs/refs/heads/e1/r2 &&
> +	test_path_is_file .git/logs/HEAD
> +'

Luckily there aren't refs that were created at this point by earlier
tests that would contradict/block the creation of the new refs this
moved tests would want to create.

> +test_expect_success 'directory not created deleting packed ref' '
> +	git branch d1/d2/r1 HEAD &&
> +	git pack-refs --all &&
> +	test_path_is_missing .git/refs/heads/d1/d2 &&
> +	git update-ref -d refs/heads/d1/d2/r1 &&
> +	test_path_is_missing .git/refs/heads/d1/d2 &&
> +	test_path_is_missing .git/refs/heads/d1
> +'

And this is from the same t1400 but appears much later in the file.
Curiously, this tries to create d1/d2/r1 that an earlier test
already created, and this one passes because the branch gets removed
in the other test that also created the branch.  Tricky but not a
fault of this patch.

> +test_expect_success SYMLINKS 'git branch -m u v should fail when the reflog for u is a symlink' '
> +	git branch --create-reflog u &&
> +	mv .git/logs/refs/heads/u real-u &&
> +	ln -s real-u .git/logs/refs/heads/u &&
> +	test_must_fail git branch -m u v
> +'

This was migrated from t3200 and has no interaction with the new
context of t0600 as branch 'u' has never been used.  I notice that
this test is buggy since its inception at 16c2bfbb (rename_ref: use
lstat(2) when testing for symlink, 2006-11-29) in that we move the
log for branch 'u' up and then make the log for branch 'u' into a
dangling symlink, so it probably failed for a wrong reason even
before 16c2bfbb updated stat() to lstat().  Anyway, the current code
will see that logs/refs/heads/u is a symbolic link and fails,
regardless of what the link points at, so we are OK.  In any case,
not a fault of this patch.

> +test_expect_success SYMLINKS 'git branch -m with symlinked .git/refs' '
> +	test_when_finished "rm -rf subdir" &&
> +	git init --bare subdir &&
> +
> +	rm -rfv subdir/refs subdir/objects subdir/packed-refs &&
> +	ln -s ../.git/refs subdir/refs &&
> +	ln -s ../.git/objects subdir/objects &&
> +	ln -s ../.git/packed-refs subdir/packed-refs &&
> +
> +	git -C subdir rev-parse --absolute-git-dir >subdir.dir &&
> +	git rev-parse --absolute-git-dir >our.dir &&
> +	! test_cmp subdir.dir our.dir &&
> +
> +	git -C subdir log &&
> +	git -C subdir branch rename-src &&
> +	git rev-parse rename-src >expect &&
> +	git -C subdir branch -m rename-src rename-dest &&
> +	git rev-parse rename-dest >actual &&
> +	test_cmp expect actual &&
> +	git branch -D rename-dest
> +'

OK.  As long as it cleans after itself, I won't read deeply into it
and stop at making sure this came verbatim from t3200.

> +test_expect_success MINGW,SYMLINKS_WINDOWS 'rebase when .git/logs is a symlink' '
> +	git checkout main &&
> +	mv .git/logs actual_logs &&
> +	cmd //c "mklink /D .git\logs ..\actual_logs" &&
> +	git rebase -f HEAD^ &&
> +	test -L .git/logs &&
> +	rm .git/logs &&
> +	mv actual_logs .git/logs
> +'

As t0600 forces the default branch to be 'main', having this one
migrated from t3400 should be safe.

> +test_expect_success POSIXPERM 'git reflog expire honors core.sharedRepository' '
> +	umask 077 &&
> +	git config core.sharedRepository group &&
> +	git reflog expire --all &&
> +	actual="$(ls -l .git/logs/refs/heads/main)" &&
> +	case "$actual" in
> +	-rw-rw-*)
> +		: happy
> +		;;
> +	*)
> +		echo Ooops, .git/logs/refs/heads/main is not 066x [$actual]
> +		false
> +		;;
> +	esac
> +'

And this is from t1301, another verbatim copy.


>  test_done
> diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
> index 8e2c01e760..b1eb5c01b8 100755
> --- a/t/t1301-shared-repo.sh
> +++ b/t/t1301-shared-repo.sh
> @@ -137,22 +137,6 @@ test_expect_success POSIXPERM 'info/refs respects umask in unshared repo' '
>  	test_cmp expect actual
>  '
>  
> -test_expect_success REFFILES,POSIXPERM 'git reflog expire honors core.sharedRepository' '
> -	umask 077 &&
> -	git config core.sharedRepository group &&
> -	git reflog expire --all &&
> -	actual="$(ls -l .git/logs/refs/heads/main)" &&
> -	case "$actual" in
> -	-rw-rw-*)
> -		: happy
> -		;;
> -	*)
> -		echo Ooops, .git/logs/refs/heads/main is not 066x [$actual]
> -		false
> -		;;
> -	esac
> -'
> -

The rest of t1301 does not appear to depend on the reflog being
expired, so this move should be safe.

>  test_expect_success POSIXPERM 'forced modes' '
>  	test_when_finished "rm -rf new" &&
>  	mkdir -p templates/hooks &&
> diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
> index f18843bf7a..3294b7ce08 100755
> --- a/t/t1400-update-ref.sh
> +++ b/t/t1400-update-ref.sh
> @@ -288,33 +288,6 @@ test_expect_success "set $m (logged by touch)" '
>  	test $A = $(git show-ref -s --verify $m)
>  '
>  
> -test_expect_success REFFILES 'empty directory removal' '
> -	git branch d1/d2/r1 HEAD &&
> -	git branch d1/r2 HEAD &&
> -	test_path_is_file .git/refs/heads/d1/d2/r1 &&
> -	test_path_is_file .git/logs/refs/heads/d1/d2/r1 &&
> -	git branch -d d1/d2/r1 &&
> -	test_must_fail git show-ref --verify -q refs/heads/d1/d2 &&
> -	test_must_fail git show-ref --verify -q logs/refs/heads/d1/d2 &&
> -	test_path_is_file .git/refs/heads/d1/r2 &&
> -	test_path_is_file .git/logs/refs/heads/d1/r2
> -'
> -
> -test_expect_success REFFILES 'symref empty directory removal' '
> -	git branch e1/e2/r1 HEAD &&
> -	git branch e1/r2 HEAD &&
> -	git checkout e1/e2/r1 &&
> -	test_when_finished "git checkout main" &&
> -	test_path_is_file .git/refs/heads/e1/e2/r1 &&
> -	test_path_is_file .git/logs/refs/heads/e1/e2/r1 &&
> -	git update-ref -d HEAD &&
> -	test_must_fail git show-ref --verify -q refs/heads/e1/e2 &&
> -	test_must_fail git show-ref --verify -q logs/refs/heads/e1/e2 &&
> -	test_path_is_file .git/refs/heads/e1/r2 &&
> -	test_path_is_file .git/logs/refs/heads/e1/r2 &&
> -	test_path_is_file .git/logs/HEAD
> -'
> -
>  cat >expect <<EOF
>  $Z $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000	Initial Creation
>  $A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150260 +0000	Switch
> @@ -1668,13 +1641,4 @@ test_expect_success PIPE 'transaction flushes status updates' '
>  	test_cmp expected actual
>  '
>  
> -test_expect_success REFFILES 'directory not created deleting packed ref' '
> -	git branch d1/d2/r1 HEAD &&
> -	git pack-refs --all &&
> -	test_path_is_missing .git/refs/heads/d1/d2 &&
> -	git update-ref -d refs/heads/d1/d2/r1 &&
> -	test_path_is_missing .git/refs/heads/d1/d2 &&
> -	test_path_is_missing .git/refs/heads/d1
> -'

I've covered how these removals should be safe for the remaining
tests in this file already.  Good.

>  test_done
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index de7d3014e4..e36f4d15f2 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -836,35 +836,6 @@ test_expect_success 'renaming a symref is not allowed' '
>  	test_ref_missing refs/heads/new-topic
>  '
>  
> -test_expect_success SYMLINKS,REFFILES 'git branch -m u v should fail when the reflog for u is a symlink' '
> -	git branch --create-reflog u &&
> -	mv .git/logs/refs/heads/u real-u &&
> -	ln -s real-u .git/logs/refs/heads/u &&
> -	test_must_fail git branch -m u v
> -'
> -
> -test_expect_success SYMLINKS,REFFILES 'git branch -m with symlinked .git/refs' '
> -	test_when_finished "rm -rf subdir" &&
> -	git init --bare subdir &&
> -
> -	rm -rfv subdir/refs subdir/objects subdir/packed-refs &&
> -	ln -s ../.git/refs subdir/refs &&
> -	ln -s ../.git/objects subdir/objects &&
> -	ln -s ../.git/packed-refs subdir/packed-refs &&
> -
> -	git -C subdir rev-parse --absolute-git-dir >subdir.dir &&
> -	git rev-parse --absolute-git-dir >our.dir &&
> -	! test_cmp subdir.dir our.dir &&
> -
> -	git -C subdir log &&
> -	git -C subdir branch rename-src &&
> -	git rev-parse rename-src >expect &&
> -	git -C subdir branch -m rename-src rename-dest &&
> -	git rev-parse rename-dest >actual &&
> -	test_cmp expect actual &&
> -	git branch -D rename-dest
> -'

Likewise.

> diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
> index 57f1392926..e1c8c5f701 100755
> --- a/t/t3400-rebase.sh
> +++ b/t/t3400-rebase.sh
> @@ -424,16 +424,6 @@ test_expect_success 'refuse to switch to branch checked out elsewhere' '
>  	test_grep "already used by worktree at" err
>  '
>  
> -test_expect_success REFFILES,MINGW,SYMLINKS_WINDOWS 'rebase when .git/logs is a symlink' '
> -	git checkout main &&
> -	mv .git/logs actual_logs &&
> -	cmd //c "mklink /D .git\logs ..\actual_logs" &&
> -	git rebase -f HEAD^ &&
> -	test -L .git/logs &&
> -	rm .git/logs &&
> -	mv actual_logs .git/logs
> -'
> -
>  test_expect_success 'rebase when inside worktree subdirectory' '
>  	git init main-wt &&
>  	(

Looking good.

Thanks.


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/7] t0410: enable tests with extensions with non-default repo format
  2024-02-09  7:23 ` [PATCH 2/7] t0410: enable tests with extensions with non-default repo format Patrick Steinhardt
@ 2024-02-14 22:57   ` Junio C Hamano
  2024-02-15  7:59     ` Patrick Steinhardt
  0 siblings, 1 reply; 28+ messages in thread
From: Junio C Hamano @ 2024-02-14 22:57 UTC (permalink / raw
  To: Patrick Steinhardt, Jonathan Nieder; +Cc: git

Patrick Steinhardt <ps@pks.im> writes:

> In t0410 we have two tests which exercise how partial clones behave in
> the context of a repository with extensions. These tests are marked to
> require a default repository using SHA1 and the "files" backend because
> we explicitly set the repository format version to 0.
>
> Changing the repository format version to 0 is not needed though. The
> "noop" extension is ignored as expected regardless of what the version
> is set to, same as the "nonsense" extension leads to failure regardless
> of the version.

Isn't the reason why 11664196 kept the forcing of the format version
because it wanted to see noop ignored and nonsense failed even if
the format version is 0 to ensure the regression it fixed will stay
fixed?  IOW, we force version 0 not because we do not want to test
with anything but SHA1 and REFFILES; we pretty much assume that with
the default version, noop and nonsense will be handled sensibly, and
we want to make sure they will be with version 0 as well.

And once we force to version 0, we have trouble running with
anything other than SHA1 and REFFILES, hence these prerequisites.

So, I dunno.

>
> Stop setting the version so that these tests can execute with SHA256 and
> "reftable" repositories.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  t/t0410-partial-clone.sh | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
> index 6b6424b3df..d913f3c453 100755
> --- a/t/t0410-partial-clone.sh
> +++ b/t/t0410-partial-clone.sh
> @@ -49,24 +49,22 @@ test_expect_success 'convert shallow clone to partial clone' '
>  	test_cmp_config -C client 1 core.repositoryformatversion
>  '
>  
> -test_expect_success SHA1,REFFILES 'convert to partial clone with noop extension' '
> +test_expect_success 'convert to partial clone with noop extension' '
>  	rm -fr server client &&
>  	test_create_repo server &&
>  	test_commit -C server my_commit 1 &&
>  	test_commit -C server my_commit2 1 &&
>  	git clone --depth=1 "file://$(pwd)/server" client &&
> -	test_cmp_config -C client 0 core.repositoryformatversion &&
>  	git -C client config extensions.noop true &&
>  	git -C client fetch --unshallow --filter="blob:none"
>  '
>  
> -test_expect_success SHA1,REFFILES 'converting to partial clone fails with unrecognized extension' '
> +test_expect_success 'converting to partial clone fails with unrecognized extension' '
>  	rm -fr server client &&
>  	test_create_repo server &&
>  	test_commit -C server my_commit 1 &&
>  	test_commit -C server my_commit2 1 &&
>  	git clone --depth=1 "file://$(pwd)/server" client &&
> -	test_cmp_config -C client 0 core.repositoryformatversion &&
>  	git -C client config extensions.nonsense true &&
>  	test_must_fail git -C client fetch --unshallow --filter="blob:none"
>  '


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 3/7] t1400: exercise reflog with gaps with reftable backend
  2024-02-09  7:23 ` [PATCH 3/7] t1400: exercise reflog with gaps with reftable backend Patrick Steinhardt
@ 2024-02-14 22:59   ` Junio C Hamano
  0 siblings, 0 replies; 28+ messages in thread
From: Junio C Hamano @ 2024-02-14 22:59 UTC (permalink / raw
  To: Patrick Steinhardt; +Cc: git

Patrick Steinhardt <ps@pks.im> writes:

> In t1400, we have a test that exercises whether we print a warning
> message as expected when the reflog contains entries which have a gap
> between the old entry's new object ID and the new entry's old object ID.
> While the logic should apply to all ref backends, the test setup writes
> into `.git/logs` directly and is thus "files"-backend specific.
>
> Refactor the test to instead use `git reflog delete` to create the gap
> and drop the REFFILES prerequisite.

This rewrite looks good.

Instead of mucking with the implementation detail, we achieve the
same with proper use of the tools provided.  Very nice.

Thanks.


> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  t/t1400-update-ref.sh | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
> index 3294b7ce08..3aeb103d34 100755
> --- a/t/t1400-update-ref.sh
> +++ b/t/t1400-update-ref.sh
> @@ -426,15 +426,15 @@ test_expect_success 'Query "main@{2005-05-28}" (past end of history)' '
>  rm -f expect
>  git update-ref -d $m
>  
> -test_expect_success REFFILES 'query reflog with gap' '
> +test_expect_success 'query reflog with gap' '
>  	test_when_finished "git update-ref -d $m" &&
>  
> -	git update-ref $m $F &&
> -	cat >.git/logs/$m <<-EOF &&
> -	$Z $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150320 -0500
> -	$A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150380 -0500
> -	$D $F $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150680 -0500
> -	EOF
> +	GIT_COMMITTER_DATE="1117150320 -0500" git update-ref $m $A &&
> +	GIT_COMMITTER_DATE="1117150380 -0500" git update-ref $m $B &&
> +	GIT_COMMITTER_DATE="1117150480 -0500" git update-ref $m $C &&
> +	GIT_COMMITTER_DATE="1117150580 -0500" git update-ref $m $D &&
> +	GIT_COMMITTER_DATE="1117150680 -0500" git update-ref $m $F &&
> +	git reflog delete $m@{2} &&
>  
>  	git rev-parse --verify "main@{2005-05-26 23:33:01}" >actual 2>stderr &&
>  	echo "$B" >expect &&


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 4/7] t1404: make D/F conflict tests compatible with reftable backend
  2024-02-09  7:23 ` [PATCH 4/7] t1404: make D/F conflict tests compatible " Patrick Steinhardt
@ 2024-02-14 23:11   ` Junio C Hamano
  0 siblings, 0 replies; 28+ messages in thread
From: Junio C Hamano @ 2024-02-14 23:11 UTC (permalink / raw
  To: Patrick Steinhardt; +Cc: git

Patrick Steinhardt <ps@pks.im> writes:

>  	test_must_fail git update-ref --stdin <commands 2>output.err &&
> -	test_cmp expected-err output.err &&
> +	grep "fatal:\( cannot lock ref $SQ$addname$SQ:\)\? $SQ$delref$SQ exists; cannot create $SQ$addref$SQ" output.err &&

OK, that's more thorough than I would have done (I am lazy and would
just check "cannot create"), but being more specific is better than
being lazy ;-)

> @@ -191,69 +188,69 @@ test_expect_success 'one new ref is a simple prefix of another' '
>  
>  '
>  
> -test_expect_success REFFILES 'D/F conflict prevents add long + delete short' '
> +test_expect_success 'D/F conflict prevents add long + delete short' '
>  	df_test refs/df-al-ds --add-del foo/bar foo
>  '

All the changes make sense here.  Thanks.


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 5/7] t1405: remove unneeded cleanup step
  2024-02-09  7:23 ` [PATCH 5/7] t1405: remove unneeded cleanup step Patrick Steinhardt
@ 2024-02-14 23:17   ` Junio C Hamano
  2024-02-15  7:59     ` Patrick Steinhardt
  0 siblings, 1 reply; 28+ messages in thread
From: Junio C Hamano @ 2024-02-14 23:17 UTC (permalink / raw
  To: Patrick Steinhardt; +Cc: git

Patrick Steinhardt <ps@pks.im> writes:

> In 5e00514745 (t1405: explictly delete reflogs for reftable, 2022-01-31)
> we have added a test that explicitly deletes the reflog when not using
> the "files" backend. This was required because back then, the "reftable"
> backend didn't yet delete reflogs when deleting their corresponding
> branches, and thus subsequent tests would fail because some unexpected
> reflogs still exist.
>
> The "reftable" backend was eventually changed though so that it behaves
> the same as the "files" backend and deletes reflogs when deleting refs.
> This was done to make the "reftable" backend behave like the "files"
> backend as closely as possible so that it can act as a drop-in
> replacement.
>
> The cleanup-style test is thus not required anymore. Remove it.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  t/t1405-main-ref-store.sh | 6 ------
>  1 file changed, 6 deletions(-)

Again, makes sense.

This is a tangent, but artificial limitations we imposed on reftable
to be more similar to files backend may be something we would want
to reconsider once reftable hits mainline and people actively start
using it.  Not having to lose the reflog only because you removed a
branch by mistake would be a powerful thing, as it would allow you
to resurrect the branch as well as its log.  Being able to have a
branch 'foo', with work related to 'foo' kept inbranches 'foo/arc1'
'foo/arc2', etc., would be a very nice organizational tool.

But it is a good idea to start limited and later making it looser.
These two limitations are something all users are already familiar
with, so it is not as cripplingly bad as it smells anyway.

Thanks.

>
> diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
> index 976bd71efb..1183232a72 100755
> --- a/t/t1405-main-ref-store.sh
> +++ b/t/t1405-main-ref-store.sh
> @@ -33,12 +33,6 @@ test_expect_success 'delete_refs(FOO, refs/tags/new-tag)' '
>  	test_must_fail git rev-parse refs/tags/new-tag --
>  '
>  
> -# In reftable, we keep the reflogs around for deleted refs.
> -test_expect_success !REFFILES 'delete-reflog(FOO, refs/tags/new-tag)' '
> -	$RUN delete-reflog FOO &&
> -	$RUN delete-reflog refs/tags/new-tag
> -'
> -
>  test_expect_success 'rename_refs(main, new-main)' '
>  	git rev-parse main >expected &&
>  	$RUN rename-ref refs/heads/main refs/heads/new-main &&


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 0/7] t: drop more REFFILES prereqs
  2024-02-09  7:23 [PATCH 0/7] t: drop more REFFILES prereqs Patrick Steinhardt
                   ` (7 preceding siblings ...)
  2024-02-11 14:00 ` [PATCH 0/7] t: drop more REFFILES prereqs Karthik Nayak
@ 2024-02-14 23:20 ` Junio C Hamano
  2024-02-15  8:14   ` Patrick Steinhardt
  2024-02-15  8:25 ` [PATCH v2 " Patrick Steinhardt
  9 siblings, 1 reply; 28+ messages in thread
From: Junio C Hamano @ 2024-02-14 23:20 UTC (permalink / raw
  To: Patrick Steinhardt; +Cc: git

Patrick Steinhardt <ps@pks.im> writes:

> this patch series is another step towards less tests with the REFFILES
> prerequisite. Some of the tests are simply moved into t0600 because they
> are inherently exercising low-level behaviour of the "files" backend.
> Other tests are adapted so that they work across both the "files" and
> the "reftable" backends.

I've read this through, and except for one of them (I left a comment
on it), they all made perfect sense.

Thanks.


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/7] t0410: enable tests with extensions with non-default repo format
  2024-02-14 22:57   ` Junio C Hamano
@ 2024-02-15  7:59     ` Patrick Steinhardt
  2024-02-15 17:18       ` Junio C Hamano
  0 siblings, 1 reply; 28+ messages in thread
From: Patrick Steinhardt @ 2024-02-15  7:59 UTC (permalink / raw
  To: Junio C Hamano; +Cc: Jonathan Nieder, git

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

On Wed, Feb 14, 2024 at 02:57:55PM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > In t0410 we have two tests which exercise how partial clones behave in
> > the context of a repository with extensions. These tests are marked to
> > require a default repository using SHA1 and the "files" backend because
> > we explicitly set the repository format version to 0.
> >
> > Changing the repository format version to 0 is not needed though. The
> > "noop" extension is ignored as expected regardless of what the version
> > is set to, same as the "nonsense" extension leads to failure regardless
> > of the version.
> 
> Isn't the reason why 11664196 kept the forcing of the format version
> because it wanted to see noop ignored and nonsense failed even if
> the format version is 0 to ensure the regression it fixed will stay
> fixed?  IOW, we force version 0 not because we do not want to test
> with anything but SHA1 and REFFILES; we pretty much assume that with
> the default version, noop and nonsense will be handled sensibly, and
> we want to make sure they will be with version 0 as well.
> 
> And once we force to version 0, we have trouble running with
> anything other than SHA1 and REFFILES, hence these prerequisites.
> 
> So, I dunno.

Hum, I guess that's fair. Let me adapt the test case to instead use the
DEFAULT_REPO_FORMAT prerequisite then.

Patrick

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 5/7] t1405: remove unneeded cleanup step
  2024-02-14 23:17   ` Junio C Hamano
@ 2024-02-15  7:59     ` Patrick Steinhardt
  0 siblings, 0 replies; 28+ messages in thread
From: Patrick Steinhardt @ 2024-02-15  7:59 UTC (permalink / raw
  To: Junio C Hamano; +Cc: git

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

On Wed, Feb 14, 2024 at 03:17:32PM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > In 5e00514745 (t1405: explictly delete reflogs for reftable, 2022-01-31)
> > we have added a test that explicitly deletes the reflog when not using
> > the "files" backend. This was required because back then, the "reftable"
> > backend didn't yet delete reflogs when deleting their corresponding
> > branches, and thus subsequent tests would fail because some unexpected
> > reflogs still exist.
> >
> > The "reftable" backend was eventually changed though so that it behaves
> > the same as the "files" backend and deletes reflogs when deleting refs.
> > This was done to make the "reftable" backend behave like the "files"
> > backend as closely as possible so that it can act as a drop-in
> > replacement.
> >
> > The cleanup-style test is thus not required anymore. Remove it.
> >
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> >  t/t1405-main-ref-store.sh | 6 ------
> >  1 file changed, 6 deletions(-)
> 
> Again, makes sense.
> 
> This is a tangent, but artificial limitations we imposed on reftable
> to be more similar to files backend may be something we would want
> to reconsider once reftable hits mainline and people actively start
> using it.  Not having to lose the reflog only because you removed a
> branch by mistake would be a powerful thing, as it would allow you
> to resurrect the branch as well as its log.  Being able to have a
> branch 'foo', with work related to 'foo' kept inbranches 'foo/arc1'
> 'foo/arc2', etc., would be a very nice organizational tool.
> 
> But it is a good idea to start limited and later making it looser.
> These two limitations are something all users are already familiar
> with, so it is not as cripplingly bad as it smells anyway.

Yeah, I very much agree with what you say here. We have it in our
backlog to change this behaviour once the initial dust has settled.

Patrick

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 0/7] t: drop more REFFILES prereqs
  2024-02-14 23:20 ` Junio C Hamano
@ 2024-02-15  8:14   ` Patrick Steinhardt
  0 siblings, 0 replies; 28+ messages in thread
From: Patrick Steinhardt @ 2024-02-15  8:14 UTC (permalink / raw
  To: Junio C Hamano; +Cc: git

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

On Wed, Feb 14, 2024 at 03:20:02PM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > this patch series is another step towards less tests with the REFFILES
> > prerequisite. Some of the tests are simply moved into t0600 because they
> > are inherently exercising low-level behaviour of the "files" backend.
> > Other tests are adapted so that they work across both the "files" and
> > the "reftable" backends.
> 
> I've read this through, and except for one of them (I left a comment
> on it), they all made perfect sense.

Thanks, also for being extra thorough with the first patch. I'll send a
v2.

Patrick

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2 0/7] t: drop more REFFILES prereqs
  2024-02-09  7:23 [PATCH 0/7] t: drop more REFFILES prereqs Patrick Steinhardt
                   ` (8 preceding siblings ...)
  2024-02-14 23:20 ` Junio C Hamano
@ 2024-02-15  8:25 ` Patrick Steinhardt
  2024-02-15  8:25   ` [PATCH v2 1/7] t: move tests exercising the "files" backend Patrick Steinhardt
                     ` (6 more replies)
  9 siblings, 7 replies; 28+ messages in thread
From: Patrick Steinhardt @ 2024-02-15  8:25 UTC (permalink / raw
  To: git; +Cc: Karthik Nayak, Junio C Hamano

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

Hi,

this is the second version of my patch series that drops the REFFILES
prereq from more tests, thereby increasing test coverage of the reftable
backend.

There's only a single change compared to v1, addressing Junio's
feedback. Please refer to the below range-diff.

Thanks!

Patrick

Patrick Steinhardt (7):
  t: move tests exercising the "files" backend
  t0410: convert tests to use DEFAULT_REPO_FORMAT prereq
  t1400: exercise reflog with gaps with reftable backend
  t1404: make D/F conflict tests compatible with reftable backend
  t1405: remove unneeded cleanup step
  t2011: exercise D/F conflicts with HEAD with the reftable backend
  t7003: ensure filter-branch prunes reflogs with the reftable backend

 t/t0410-partial-clone.sh         |  4 +-
 t/t0600-reffiles-backend.sh      | 91 ++++++++++++++++++++++++++++++++
 t/t1301-shared-repo.sh           | 16 ------
 t/t1400-update-ref.sh            | 50 +++---------------
 t/t1404-update-ref-errors.sh     | 37 ++++++-------
 t/t1405-main-ref-store.sh        |  6 ---
 t/t2011-checkout-invalid-head.sh | 17 +++---
 t/t3200-branch.sh                | 29 ----------
 t/t3400-rebase.sh                | 10 ----
 t/t7003-filter-branch.sh         |  5 +-
 10 files changed, 125 insertions(+), 140 deletions(-)

Range-diff against v1:
1:  2eca90234f = 1:  6891cdfdb3 t: move tests exercising the "files" backend
2:  feef6a3e6c ! 2:  2dd87f3126 t0410: enable tests with extensions with non-default repo format
    @@ Metadata
     Author: Patrick Steinhardt <ps@pks.im>
     
      ## Commit message ##
    -    t0410: enable tests with extensions with non-default repo format
    +    t0410: convert tests to use DEFAULT_REPO_FORMAT prereq
     
         In t0410 we have two tests which exercise how partial clones behave in
         the context of a repository with extensions. These tests are marked to
    -    require a default repository using SHA1 and the "files" backend because
    -    we explicitly set the repository format version to 0.
    +    require a repository using SHA1 and the "files" backend because we
    +    explicitly set the repository format version to 0, and setting up either
    +    the "objectFormat" or "refStorage" extensions requires a repository
    +    format version of 1.
     
    -    Changing the repository format version to 0 is not needed though. The
    -    "noop" extension is ignored as expected regardless of what the version
    -    is set to, same as the "nonsense" extension leads to failure regardless
    -    of the version.
    -
    -    Stop setting the version so that these tests can execute with SHA256 and
    -    "reftable" repositories.
    +    We have recently introduced a new DEFAULT_REPO_FORMAT prerequisite.
    +    Despite capturing the intent more directly, it also has the added
    +    benefit that it can easily be extended in the future in case we add new
    +    repository extensions. Adapt the tests to use it.
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
    @@ t/t0410-partial-clone.sh: test_expect_success 'convert shallow clone to partial
      '
      
     -test_expect_success SHA1,REFFILES 'convert to partial clone with noop extension' '
    -+test_expect_success 'convert to partial clone with noop extension' '
    ++test_expect_success DEFAULT_REPO_FORMAT 'convert to partial clone with noop extension' '
      	rm -fr server client &&
      	test_create_repo server &&
      	test_commit -C server my_commit 1 &&
    - 	test_commit -C server my_commit2 1 &&
    - 	git clone --depth=1 "file://$(pwd)/server" client &&
    --	test_cmp_config -C client 0 core.repositoryformatversion &&
    - 	git -C client config extensions.noop true &&
    +@@ t/t0410-partial-clone.sh: test_expect_success SHA1,REFFILES 'convert to partial clone with noop extension'
      	git -C client fetch --unshallow --filter="blob:none"
      '
      
     -test_expect_success SHA1,REFFILES 'converting to partial clone fails with unrecognized extension' '
    -+test_expect_success 'converting to partial clone fails with unrecognized extension' '
    ++test_expect_success DEFAULT_REPO_FORMAT 'converting to partial clone fails with unrecognized extension' '
      	rm -fr server client &&
      	test_create_repo server &&
      	test_commit -C server my_commit 1 &&
    - 	test_commit -C server my_commit2 1 &&
    - 	git clone --depth=1 "file://$(pwd)/server" client &&
    --	test_cmp_config -C client 0 core.repositoryformatversion &&
    - 	git -C client config extensions.nonsense true &&
    - 	test_must_fail git -C client fetch --unshallow --filter="blob:none"
    - '
3:  9d8eed354e = 3:  ed57913eb9 t1400: exercise reflog with gaps with reftable backend
4:  70c6f98012 = 4:  212949689f t1404: make D/F conflict tests compatible with reftable backend
5:  25cf00c36f = 5:  67d6aede63 t1405: remove unneeded cleanup step
6:  64d2548bbc = 6:  24051cc246 t2011: exercise D/F conflicts with HEAD with the reftable backend
7:  7adf510f73 = 7:  8fb6de37ce t7003: ensure filter-branch prunes reflogs with the reftable backend

base-commit: 4fc51f00ef18d2c0174ab2fd39d0ee473fd144bd
-- 
2.44.0-rc0


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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2 1/7] t: move tests exercising the "files" backend
  2024-02-15  8:25 ` [PATCH v2 " Patrick Steinhardt
@ 2024-02-15  8:25   ` Patrick Steinhardt
  2024-02-15  8:25   ` [PATCH v2 2/7] t0410: convert tests to use DEFAULT_REPO_FORMAT prereq Patrick Steinhardt
                     ` (5 subsequent siblings)
  6 siblings, 0 replies; 28+ messages in thread
From: Patrick Steinhardt @ 2024-02-15  8:25 UTC (permalink / raw
  To: git; +Cc: Karthik Nayak, Junio C Hamano

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

We still have a bunch of tests scattered across our test suites that
exercise on-disk files of the "files" backend directly:

  - t1301 exercises permissions of reflog files when the config
    "core.sharedRepository" is set.

  - t1400 exercises whether empty directories in the ref store are
    handled correctly.

  - t3200 exercises what happens when there are symlinks in the ref
    store.

  - t3400 also exercises what happens when ".git/logs" is a symlink.

All of these are inherently low-level tests specific to the "files"
backend. Move them into "t0600-reffiles-backend.sh" to reflect this.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0600-reffiles-backend.sh | 91 +++++++++++++++++++++++++++++++++++++
 t/t1301-shared-repo.sh      | 16 -------
 t/t1400-update-ref.sh       | 36 ---------------
 t/t3200-branch.sh           | 29 ------------
 t/t3400-rebase.sh           | 10 ----
 5 files changed, 91 insertions(+), 91 deletions(-)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index e6a5f1868f..485481d6b4 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -381,4 +381,95 @@ test_expect_success 'log diagnoses bogus HEAD symref' '
 	test_grep broken stderr
 '
 
+test_expect_success 'empty directory removal' '
+	git branch d1/d2/r1 HEAD &&
+	git branch d1/r2 HEAD &&
+	test_path_is_file .git/refs/heads/d1/d2/r1 &&
+	test_path_is_file .git/logs/refs/heads/d1/d2/r1 &&
+	git branch -d d1/d2/r1 &&
+	test_must_fail git show-ref --verify -q refs/heads/d1/d2 &&
+	test_must_fail git show-ref --verify -q logs/refs/heads/d1/d2 &&
+	test_path_is_file .git/refs/heads/d1/r2 &&
+	test_path_is_file .git/logs/refs/heads/d1/r2
+'
+
+test_expect_success 'symref empty directory removal' '
+	git branch e1/e2/r1 HEAD &&
+	git branch e1/r2 HEAD &&
+	git checkout e1/e2/r1 &&
+	test_when_finished "git checkout main" &&
+	test_path_is_file .git/refs/heads/e1/e2/r1 &&
+	test_path_is_file .git/logs/refs/heads/e1/e2/r1 &&
+	git update-ref -d HEAD &&
+	test_must_fail git show-ref --verify -q refs/heads/e1/e2 &&
+	test_must_fail git show-ref --verify -q logs/refs/heads/e1/e2 &&
+	test_path_is_file .git/refs/heads/e1/r2 &&
+	test_path_is_file .git/logs/refs/heads/e1/r2 &&
+	test_path_is_file .git/logs/HEAD
+'
+
+test_expect_success 'directory not created deleting packed ref' '
+	git branch d1/d2/r1 HEAD &&
+	git pack-refs --all &&
+	test_path_is_missing .git/refs/heads/d1/d2 &&
+	git update-ref -d refs/heads/d1/d2/r1 &&
+	test_path_is_missing .git/refs/heads/d1/d2 &&
+	test_path_is_missing .git/refs/heads/d1
+'
+
+test_expect_success SYMLINKS 'git branch -m u v should fail when the reflog for u is a symlink' '
+	git branch --create-reflog u &&
+	mv .git/logs/refs/heads/u real-u &&
+	ln -s real-u .git/logs/refs/heads/u &&
+	test_must_fail git branch -m u v
+'
+
+test_expect_success SYMLINKS 'git branch -m with symlinked .git/refs' '
+	test_when_finished "rm -rf subdir" &&
+	git init --bare subdir &&
+
+	rm -rfv subdir/refs subdir/objects subdir/packed-refs &&
+	ln -s ../.git/refs subdir/refs &&
+	ln -s ../.git/objects subdir/objects &&
+	ln -s ../.git/packed-refs subdir/packed-refs &&
+
+	git -C subdir rev-parse --absolute-git-dir >subdir.dir &&
+	git rev-parse --absolute-git-dir >our.dir &&
+	! test_cmp subdir.dir our.dir &&
+
+	git -C subdir log &&
+	git -C subdir branch rename-src &&
+	git rev-parse rename-src >expect &&
+	git -C subdir branch -m rename-src rename-dest &&
+	git rev-parse rename-dest >actual &&
+	test_cmp expect actual &&
+	git branch -D rename-dest
+'
+
+test_expect_success MINGW,SYMLINKS_WINDOWS 'rebase when .git/logs is a symlink' '
+	git checkout main &&
+	mv .git/logs actual_logs &&
+	cmd //c "mklink /D .git\logs ..\actual_logs" &&
+	git rebase -f HEAD^ &&
+	test -L .git/logs &&
+	rm .git/logs &&
+	mv actual_logs .git/logs
+'
+
+test_expect_success POSIXPERM 'git reflog expire honors core.sharedRepository' '
+	umask 077 &&
+	git config core.sharedRepository group &&
+	git reflog expire --all &&
+	actual="$(ls -l .git/logs/refs/heads/main)" &&
+	case "$actual" in
+	-rw-rw-*)
+		: happy
+		;;
+	*)
+		echo Ooops, .git/logs/refs/heads/main is not 066x [$actual]
+		false
+		;;
+	esac
+'
+
 test_done
diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
index 8e2c01e760..b1eb5c01b8 100755
--- a/t/t1301-shared-repo.sh
+++ b/t/t1301-shared-repo.sh
@@ -137,22 +137,6 @@ test_expect_success POSIXPERM 'info/refs respects umask in unshared repo' '
 	test_cmp expect actual
 '
 
-test_expect_success REFFILES,POSIXPERM 'git reflog expire honors core.sharedRepository' '
-	umask 077 &&
-	git config core.sharedRepository group &&
-	git reflog expire --all &&
-	actual="$(ls -l .git/logs/refs/heads/main)" &&
-	case "$actual" in
-	-rw-rw-*)
-		: happy
-		;;
-	*)
-		echo Ooops, .git/logs/refs/heads/main is not 066x [$actual]
-		false
-		;;
-	esac
-'
-
 test_expect_success POSIXPERM 'forced modes' '
 	test_when_finished "rm -rf new" &&
 	mkdir -p templates/hooks &&
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 78a09abc35..bf37763bd6 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -288,33 +288,6 @@ test_expect_success "set $m (logged by touch)" '
 	test $A = $(git show-ref -s --verify $m)
 '
 
-test_expect_success REFFILES 'empty directory removal' '
-	git branch d1/d2/r1 HEAD &&
-	git branch d1/r2 HEAD &&
-	test_path_is_file .git/refs/heads/d1/d2/r1 &&
-	test_path_is_file .git/logs/refs/heads/d1/d2/r1 &&
-	git branch -d d1/d2/r1 &&
-	test_must_fail git show-ref --verify -q refs/heads/d1/d2 &&
-	test_must_fail git show-ref --verify -q logs/refs/heads/d1/d2 &&
-	test_path_is_file .git/refs/heads/d1/r2 &&
-	test_path_is_file .git/logs/refs/heads/d1/r2
-'
-
-test_expect_success REFFILES 'symref empty directory removal' '
-	git branch e1/e2/r1 HEAD &&
-	git branch e1/r2 HEAD &&
-	git checkout e1/e2/r1 &&
-	test_when_finished "git checkout main" &&
-	test_path_is_file .git/refs/heads/e1/e2/r1 &&
-	test_path_is_file .git/logs/refs/heads/e1/e2/r1 &&
-	git update-ref -d HEAD &&
-	test_must_fail git show-ref --verify -q refs/heads/e1/e2 &&
-	test_must_fail git show-ref --verify -q logs/refs/heads/e1/e2 &&
-	test_path_is_file .git/refs/heads/e1/r2 &&
-	test_path_is_file .git/logs/refs/heads/e1/r2 &&
-	test_path_is_file .git/logs/HEAD
-'
-
 cat >expect <<EOF
 $Z $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000	Initial Creation
 $A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150260 +0000	Switch
@@ -1668,13 +1641,4 @@ test_expect_success PIPE 'transaction flushes status updates' '
 	test_cmp expected actual
 '
 
-test_expect_success REFFILES 'directory not created deleting packed ref' '
-	git branch d1/d2/r1 HEAD &&
-	git pack-refs --all &&
-	test_path_is_missing .git/refs/heads/d1/d2 &&
-	git update-ref -d refs/heads/d1/d2/r1 &&
-	test_path_is_missing .git/refs/heads/d1/d2 &&
-	test_path_is_missing .git/refs/heads/d1
-'
-
 test_done
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index de7d3014e4..e36f4d15f2 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -836,35 +836,6 @@ test_expect_success 'renaming a symref is not allowed' '
 	test_ref_missing refs/heads/new-topic
 '
 
-test_expect_success SYMLINKS,REFFILES 'git branch -m u v should fail when the reflog for u is a symlink' '
-	git branch --create-reflog u &&
-	mv .git/logs/refs/heads/u real-u &&
-	ln -s real-u .git/logs/refs/heads/u &&
-	test_must_fail git branch -m u v
-'
-
-test_expect_success SYMLINKS,REFFILES 'git branch -m with symlinked .git/refs' '
-	test_when_finished "rm -rf subdir" &&
-	git init --bare subdir &&
-
-	rm -rfv subdir/refs subdir/objects subdir/packed-refs &&
-	ln -s ../.git/refs subdir/refs &&
-	ln -s ../.git/objects subdir/objects &&
-	ln -s ../.git/packed-refs subdir/packed-refs &&
-
-	git -C subdir rev-parse --absolute-git-dir >subdir.dir &&
-	git rev-parse --absolute-git-dir >our.dir &&
-	! test_cmp subdir.dir our.dir &&
-
-	git -C subdir log &&
-	git -C subdir branch rename-src &&
-	git rev-parse rename-src >expect &&
-	git -C subdir branch -m rename-src rename-dest &&
-	git rev-parse rename-dest >actual &&
-	test_cmp expect actual &&
-	git branch -D rename-dest
-'
-
 test_expect_success 'test tracking setup via --track' '
 	git config remote.local.url . &&
 	git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 57f1392926..e1c8c5f701 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -424,16 +424,6 @@ test_expect_success 'refuse to switch to branch checked out elsewhere' '
 	test_grep "already used by worktree at" err
 '
 
-test_expect_success REFFILES,MINGW,SYMLINKS_WINDOWS 'rebase when .git/logs is a symlink' '
-	git checkout main &&
-	mv .git/logs actual_logs &&
-	cmd //c "mklink /D .git\logs ..\actual_logs" &&
-	git rebase -f HEAD^ &&
-	test -L .git/logs &&
-	rm .git/logs &&
-	mv actual_logs .git/logs
-'
-
 test_expect_success 'rebase when inside worktree subdirectory' '
 	git init main-wt &&
 	(
-- 
2.44.0-rc0


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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 2/7] t0410: convert tests to use DEFAULT_REPO_FORMAT prereq
  2024-02-15  8:25 ` [PATCH v2 " Patrick Steinhardt
  2024-02-15  8:25   ` [PATCH v2 1/7] t: move tests exercising the "files" backend Patrick Steinhardt
@ 2024-02-15  8:25   ` Patrick Steinhardt
  2024-02-15 18:19     ` Junio C Hamano
  2024-02-15  8:25   ` [PATCH v2 3/7] t1400: exercise reflog with gaps with reftable backend Patrick Steinhardt
                     ` (4 subsequent siblings)
  6 siblings, 1 reply; 28+ messages in thread
From: Patrick Steinhardt @ 2024-02-15  8:25 UTC (permalink / raw
  To: git; +Cc: Karthik Nayak, Junio C Hamano

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

In t0410 we have two tests which exercise how partial clones behave in
the context of a repository with extensions. These tests are marked to
require a repository using SHA1 and the "files" backend because we
explicitly set the repository format version to 0, and setting up either
the "objectFormat" or "refStorage" extensions requires a repository
format version of 1.

We have recently introduced a new DEFAULT_REPO_FORMAT prerequisite.
Despite capturing the intent more directly, it also has the added
benefit that it can easily be extended in the future in case we add new
repository extensions. Adapt the tests to use it.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0410-partial-clone.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
index 6b6424b3df..0f98b21be8 100755
--- a/t/t0410-partial-clone.sh
+++ b/t/t0410-partial-clone.sh
@@ -49,7 +49,7 @@ test_expect_success 'convert shallow clone to partial clone' '
 	test_cmp_config -C client 1 core.repositoryformatversion
 '
 
-test_expect_success SHA1,REFFILES 'convert to partial clone with noop extension' '
+test_expect_success DEFAULT_REPO_FORMAT 'convert to partial clone with noop extension' '
 	rm -fr server client &&
 	test_create_repo server &&
 	test_commit -C server my_commit 1 &&
@@ -60,7 +60,7 @@ test_expect_success SHA1,REFFILES 'convert to partial clone with noop extension'
 	git -C client fetch --unshallow --filter="blob:none"
 '
 
-test_expect_success SHA1,REFFILES 'converting to partial clone fails with unrecognized extension' '
+test_expect_success DEFAULT_REPO_FORMAT 'converting to partial clone fails with unrecognized extension' '
 	rm -fr server client &&
 	test_create_repo server &&
 	test_commit -C server my_commit 1 &&
-- 
2.44.0-rc0


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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 3/7] t1400: exercise reflog with gaps with reftable backend
  2024-02-15  8:25 ` [PATCH v2 " Patrick Steinhardt
  2024-02-15  8:25   ` [PATCH v2 1/7] t: move tests exercising the "files" backend Patrick Steinhardt
  2024-02-15  8:25   ` [PATCH v2 2/7] t0410: convert tests to use DEFAULT_REPO_FORMAT prereq Patrick Steinhardt
@ 2024-02-15  8:25   ` Patrick Steinhardt
  2024-02-15  8:25   ` [PATCH v2 4/7] t1404: make D/F conflict tests compatible " Patrick Steinhardt
                     ` (3 subsequent siblings)
  6 siblings, 0 replies; 28+ messages in thread
From: Patrick Steinhardt @ 2024-02-15  8:25 UTC (permalink / raw
  To: git; +Cc: Karthik Nayak, Junio C Hamano

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

In t1400, we have a test that exercises whether we print a warning
message as expected when the reflog contains entries which have a gap
between the old entry's new object ID and the new entry's old object ID.
While the logic should apply to all ref backends, the test setup writes
into `.git/logs` directly and is thus "files"-backend specific.

Refactor the test to instead use `git reflog delete` to create the gap
and drop the REFFILES prerequisite.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t1400-update-ref.sh | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index bf37763bd6..6ebc3ef945 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -426,15 +426,15 @@ test_expect_success 'Query "main@{2005-05-28}" (past end of history)' '
 rm -f expect
 git update-ref -d $m
 
-test_expect_success REFFILES 'query reflog with gap' '
+test_expect_success 'query reflog with gap' '
 	test_when_finished "git update-ref -d $m" &&
 
-	git update-ref $m $F &&
-	cat >.git/logs/$m <<-EOF &&
-	$Z $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150320 -0500
-	$A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150380 -0500
-	$D $F $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150680 -0500
-	EOF
+	GIT_COMMITTER_DATE="1117150320 -0500" git update-ref $m $A &&
+	GIT_COMMITTER_DATE="1117150380 -0500" git update-ref $m $B &&
+	GIT_COMMITTER_DATE="1117150480 -0500" git update-ref $m $C &&
+	GIT_COMMITTER_DATE="1117150580 -0500" git update-ref $m $D &&
+	GIT_COMMITTER_DATE="1117150680 -0500" git update-ref $m $F &&
+	git reflog delete $m@{2} &&
 
 	git rev-parse --verify "main@{2005-05-26 23:33:01}" >actual 2>stderr &&
 	echo "$B" >expect &&
-- 
2.44.0-rc0


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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 4/7] t1404: make D/F conflict tests compatible with reftable backend
  2024-02-15  8:25 ` [PATCH v2 " Patrick Steinhardt
                     ` (2 preceding siblings ...)
  2024-02-15  8:25   ` [PATCH v2 3/7] t1400: exercise reflog with gaps with reftable backend Patrick Steinhardt
@ 2024-02-15  8:25   ` Patrick Steinhardt
  2024-02-15  8:25   ` [PATCH v2 5/7] t1405: remove unneeded cleanup step Patrick Steinhardt
                     ` (2 subsequent siblings)
  6 siblings, 0 replies; 28+ messages in thread
From: Patrick Steinhardt @ 2024-02-15  8:25 UTC (permalink / raw
  To: git; +Cc: Karthik Nayak, Junio C Hamano

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

Some of the tests in t1404 exercise whether Git correctly aborts
transactions when there is a directory/file conflict with ref names.
While these tests are all marked to require the "files" backend, they do
in fact apply to the "reftable" backend as well.

This may not make much sense on the surface: D/F conflicts only exist
because the "files" backend uses the filesystem to store loose refs, and
thus the restriction theoretically shouldn't apply to the "reftable"
backend. But for now, the "reftable" backend artificially restricts the
creation of such conflicting refs so that it is a drop-in replacement
for the "files" backend. This also ensures that the "reftable" backend
can easily be used on the server side without causing issues for clients
which only know to use the "files" backend.

The only difference between the "files" and "reftable" backends is a
slightly different error message. Adapt the tests to accomodate for this
difference and remove the REFFILES prerequisite so that we start testing
with both backends.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t1404-update-ref-errors.sh | 37 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/t/t1404-update-ref-errors.sh b/t/t1404-update-ref-errors.sh
index 00b7013705..98e9158bd2 100755
--- a/t/t1404-update-ref-errors.sh
+++ b/t/t1404-update-ref-errors.sh
@@ -92,9 +92,6 @@ df_test() {
 	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
@@ -103,7 +100,7 @@ df_test() {
 		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 &&
+	grep "fatal:\( cannot lock ref $SQ$addname$SQ:\)\? $SQ$delref$SQ exists; cannot create $SQ$addref$SQ" 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
@@ -191,69 +188,69 @@ test_expect_success 'one new ref is a simple prefix of another' '
 
 '
 
-test_expect_success REFFILES 'D/F conflict prevents add long + delete short' '
+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 REFFILES 'D/F conflict prevents add short + delete long' '
+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 REFFILES 'D/F conflict prevents delete long + add short' '
+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 REFFILES 'D/F conflict prevents delete short + add long' '
+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 REFFILES 'D/F conflict prevents add long + delete short packed' '
+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 REFFILES 'D/F conflict prevents add short + delete long packed' '
+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 REFFILES 'D/F conflict prevents delete long packed + add short' '
+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 REFFILES 'D/F conflict prevents delete short packed + add long' '
+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 REFFILES 'D/F conflict prevents indirect add long + delete short' '
+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 REFFILES 'D/F conflict prevents indirect add long + indirect delete short' '
+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 REFFILES 'D/F conflict prevents indirect add short + indirect delete long' '
+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 REFFILES 'D/F conflict prevents indirect delete long + indirect add short' '
+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 REFFILES 'D/F conflict prevents indirect add long + delete short packed' '
+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 REFFILES 'D/F conflict prevents indirect add long + indirect delete short packed' '
+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 REFFILES 'D/F conflict prevents add long + indirect delete short packed' '
+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 REFFILES 'D/F conflict prevents indirect delete long packed + indirect add short' '
+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
 '
 
-- 
2.44.0-rc0


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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 5/7] t1405: remove unneeded cleanup step
  2024-02-15  8:25 ` [PATCH v2 " Patrick Steinhardt
                     ` (3 preceding siblings ...)
  2024-02-15  8:25   ` [PATCH v2 4/7] t1404: make D/F conflict tests compatible " Patrick Steinhardt
@ 2024-02-15  8:25   ` Patrick Steinhardt
  2024-02-15  8:25   ` [PATCH v2 6/7] t2011: exercise D/F conflicts with HEAD with the reftable backend Patrick Steinhardt
  2024-02-15  8:25   ` [PATCH v2 7/7] t7003: ensure filter-branch prunes reflogs " Patrick Steinhardt
  6 siblings, 0 replies; 28+ messages in thread
From: Patrick Steinhardt @ 2024-02-15  8:25 UTC (permalink / raw
  To: git; +Cc: Karthik Nayak, Junio C Hamano

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

In 5e00514745 (t1405: explictly delete reflogs for reftable, 2022-01-31)
we have added a test that explicitly deletes the reflog when not using
the "files" backend. This was required because back then, the "reftable"
backend didn't yet delete reflogs when deleting their corresponding
branches, and thus subsequent tests would fail because some unexpected
reflogs still exist.

The "reftable" backend was eventually changed though so that it behaves
the same as the "files" backend and deletes reflogs when deleting refs.
This was done to make the "reftable" backend behave like the "files"
backend as closely as possible so that it can act as a drop-in
replacement.

The cleanup-style test is thus not required anymore. Remove it.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t1405-main-ref-store.sh | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
index 976bd71efb..1183232a72 100755
--- a/t/t1405-main-ref-store.sh
+++ b/t/t1405-main-ref-store.sh
@@ -33,12 +33,6 @@ test_expect_success 'delete_refs(FOO, refs/tags/new-tag)' '
 	test_must_fail git rev-parse refs/tags/new-tag --
 '
 
-# In reftable, we keep the reflogs around for deleted refs.
-test_expect_success !REFFILES 'delete-reflog(FOO, refs/tags/new-tag)' '
-	$RUN delete-reflog FOO &&
-	$RUN delete-reflog refs/tags/new-tag
-'
-
 test_expect_success 'rename_refs(main, new-main)' '
 	git rev-parse main >expected &&
 	$RUN rename-ref refs/heads/main refs/heads/new-main &&
-- 
2.44.0-rc0


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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 6/7] t2011: exercise D/F conflicts with HEAD with the reftable backend
  2024-02-15  8:25 ` [PATCH v2 " Patrick Steinhardt
                     ` (4 preceding siblings ...)
  2024-02-15  8:25   ` [PATCH v2 5/7] t1405: remove unneeded cleanup step Patrick Steinhardt
@ 2024-02-15  8:25   ` Patrick Steinhardt
  2024-02-15  8:25   ` [PATCH v2 7/7] t7003: ensure filter-branch prunes reflogs " Patrick Steinhardt
  6 siblings, 0 replies; 28+ messages in thread
From: Patrick Steinhardt @ 2024-02-15  8:25 UTC (permalink / raw
  To: git; +Cc: Karthik Nayak, Junio C Hamano

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

Some of the tests in t2011 exercise whether it is possible to move away
from a symbolic HEAD ref whose target ref has a directory-file conflict
with another, preexisting ref. These tests don't use git-symbolic-ref(1)
but manually write HEAD. This is supposedly done to avoid using logic
that we're about to exercise, but it makes it impossible to verify
whether the logic also works for ref backends other than "files".

Refactor the code to use git-symbolic-ref(1) instead so that the tests
work with the "reftable" backend, as well. We already have lots of tests
in t1404 that ensure that both git-update-ref(1) and git-symbolic-ref(1)
work in such a scenario, so it should be safe to rely on it here.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t2011-checkout-invalid-head.sh | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/t/t2011-checkout-invalid-head.sh b/t/t2011-checkout-invalid-head.sh
index 3c8135831b..04f53b1ea1 100755
--- a/t/t2011-checkout-invalid-head.sh
+++ b/t/t2011-checkout-invalid-head.sh
@@ -29,36 +29,33 @@ test_expect_success REFFILES 'checkout notices failure to lock HEAD' '
 	test_must_fail git checkout -b other
 '
 
-test_expect_success REFFILES 'create ref directory/file conflict scenario' '
+test_expect_success 'create ref directory/file conflict scenario' '
 	git update-ref refs/heads/outer/inner main &&
-
-	# do not rely on symbolic-ref to get a known state,
-	# as it may use the same code we are testing
 	reset_to_df () {
-		echo "ref: refs/heads/outer" >.git/HEAD
+		git symbolic-ref HEAD refs/heads/outer
 	}
 '
 
-test_expect_success REFFILES 'checkout away from d/f HEAD (unpacked, to branch)' '
+test_expect_success 'checkout away from d/f HEAD (unpacked, to branch)' '
 	reset_to_df &&
 	git checkout main
 '
 
-test_expect_success REFFILES 'checkout away from d/f HEAD (unpacked, to detached)' '
+test_expect_success 'checkout away from d/f HEAD (unpacked, to detached)' '
 	reset_to_df &&
 	git checkout --detach main
 '
 
-test_expect_success REFFILES 'pack refs' '
+test_expect_success 'pack refs' '
 	git pack-refs --all --prune
 '
 
-test_expect_success REFFILES 'checkout away from d/f HEAD (packed, to branch)' '
+test_expect_success 'checkout away from d/f HEAD (packed, to branch)' '
 	reset_to_df &&
 	git checkout main
 '
 
-test_expect_success REFFILES 'checkout away from d/f HEAD (packed, to detached)' '
+test_expect_success 'checkout away from d/f HEAD (packed, to detached)' '
 	reset_to_df &&
 	git checkout --detach main
 '
-- 
2.44.0-rc0


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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 7/7] t7003: ensure filter-branch prunes reflogs with the reftable backend
  2024-02-15  8:25 ` [PATCH v2 " Patrick Steinhardt
                     ` (5 preceding siblings ...)
  2024-02-15  8:25   ` [PATCH v2 6/7] t2011: exercise D/F conflicts with HEAD with the reftable backend Patrick Steinhardt
@ 2024-02-15  8:25   ` Patrick Steinhardt
  6 siblings, 0 replies; 28+ messages in thread
From: Patrick Steinhardt @ 2024-02-15  8:25 UTC (permalink / raw
  To: git; +Cc: Karthik Nayak, Junio C Hamano

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

In t7003 we conditionally check whether the reflog for branches pruned
by git-filter-branch(1) get deleted based on whether or not we use the
"files" backend. Same as with the preceding commit, this condition was
added because in its initial iteration the "reftable" backend did not
delete reflogs when their corresponding ref was deleted. Since then, the
backend has been aligned to behave the same as the "files" backend
though, which makes this check unnecessary.

Remove it.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t7003-filter-branch.sh | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index f6aebe92ff..5ab4d41ee7 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -396,10 +396,7 @@ test_expect_success '--prune-empty is able to prune entire branch' '
 	git branch prune-entire B &&
 	git filter-branch -f --prune-empty --index-filter "git update-index --remove A.t B.t" prune-entire &&
 	test_must_fail git rev-parse refs/heads/prune-entire &&
-	if test_have_prereq REFFILES
-	then
-		test_must_fail git reflog exists refs/heads/prune-entire
-	fi
+	test_must_fail git reflog exists refs/heads/prune-entire
 '
 
 test_expect_success '--remap-to-ancestor with filename filters' '
-- 
2.44.0-rc0


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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/7] t0410: enable tests with extensions with non-default repo format
  2024-02-15  7:59     ` Patrick Steinhardt
@ 2024-02-15 17:18       ` Junio C Hamano
  0 siblings, 0 replies; 28+ messages in thread
From: Junio C Hamano @ 2024-02-15 17:18 UTC (permalink / raw
  To: Patrick Steinhardt; +Cc: Jonathan Nieder, git

Patrick Steinhardt <ps@pks.im> writes:

> On Wed, Feb 14, 2024 at 02:57:55PM -0800, Junio C Hamano wrote:
>> Patrick Steinhardt <ps@pks.im> writes:
>> 
>> > In t0410 we have two tests which exercise how partial clones behave in
>> > the context of a repository with extensions. These tests are marked to
>> > require a default repository using SHA1 and the "files" backend because
>> > we explicitly set the repository format version to 0.
>> >
>> > Changing the repository format version to 0 is not needed though. The
>> > "noop" extension is ignored as expected regardless of what the version
>> > is set to, same as the "nonsense" extension leads to failure regardless
>> > of the version.
>> 
>> Isn't the reason why 11664196 kept the forcing of the format version
>> because it wanted to see noop ignored and nonsense failed even if
>> the format version is 0 to ensure the regression it fixed will stay
>> fixed?  IOW, we force version 0 not because we do not want to test
>> with anything but SHA1 and REFFILES; we pretty much assume that with
>> the default version, noop and nonsense will be handled sensibly, and
>> we want to make sure they will be with version 0 as well.
>> 
>> And once we force to version 0, we have trouble running with
>> anything other than SHA1 and REFFILES, hence these prerequisites.
>> 
>> So, I dunno.
>
> Hum, I guess that's fair. Let me adapt the test case to instead use the
> DEFAULT_REPO_FORMAT prerequisite then.

If we expect that "git init" by default will create version 0
repository in the foreseeable future (and when the default gets
fliped to create version 1, we will add CI task that forces version
0 somehow, just like we have special CI task to run test with ref
backend set to reftable), then I think the patch as posted may
actually be a good idea.  As long as somebody is checking these with
a repository in version 0 format, the original motivation behind the
tests is satisified, and if we in addition make sure noop and
nonsense are handled sensibly with higher versions, that would also
be good, too.

So perhaps we need some in-code comment next to these tests to
remind us about that, while removing these prerequisites?

Thanks.


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/7] t0410: convert tests to use DEFAULT_REPO_FORMAT prereq
  2024-02-15  8:25   ` [PATCH v2 2/7] t0410: convert tests to use DEFAULT_REPO_FORMAT prereq Patrick Steinhardt
@ 2024-02-15 18:19     ` Junio C Hamano
  0 siblings, 0 replies; 28+ messages in thread
From: Junio C Hamano @ 2024-02-15 18:19 UTC (permalink / raw
  To: Patrick Steinhardt; +Cc: git, Karthik Nayak

Patrick Steinhardt <ps@pks.im> writes:

> We have recently introduced a new DEFAULT_REPO_FORMAT prerequisite.
> Despite capturing the intent more directly, it also has the added
> benefit that it can easily be extended in the future in case we add new
> repository extensions. Adapt the tests to use it.

That is a good explanation.  Instead of lifting the prerequisite and
forcing version 0 altogether, we can move by one step (I am puzzled
by "despite" though).

As I said in a separate message, as long as we make sure somebody
will test with version 0, we do not mind if other people test with
higher versions, so in that sense, the patch in v1 may be closer to
what we want to have in the longer term (but we need to figure out
how we "make sure somebody will test with version 0" first).

Thanks, will queue.

> -test_expect_success SHA1,REFFILES 'convert to partial clone with noop extension' '
> +test_expect_success DEFAULT_REPO_FORMAT 'convert to partial clone with noop extension' '
>  	rm -fr server client &&
>  	test_create_repo server &&
>  	test_commit -C server my_commit 1 &&
> @@ -60,7 +60,7 @@ test_expect_success SHA1,REFFILES 'convert to partial clone with noop extension'
>  	git -C client fetch --unshallow --filter="blob:none"
>  '
>  
> -test_expect_success SHA1,REFFILES 'converting to partial clone fails with unrecognized extension' '
> +test_expect_success DEFAULT_REPO_FORMAT 'converting to partial clone fails with unrecognized extension' '
>  	rm -fr server client &&
>  	test_create_repo server &&
>  	test_commit -C server my_commit 1 &&


^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2024-02-15 18:19 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-09  7:23 [PATCH 0/7] t: drop more REFFILES prereqs Patrick Steinhardt
2024-02-09  7:23 ` [PATCH 1/7] t: move tests exercising the "files" backend Patrick Steinhardt
2024-02-14 22:45   ` Junio C Hamano
2024-02-09  7:23 ` [PATCH 2/7] t0410: enable tests with extensions with non-default repo format Patrick Steinhardt
2024-02-14 22:57   ` Junio C Hamano
2024-02-15  7:59     ` Patrick Steinhardt
2024-02-15 17:18       ` Junio C Hamano
2024-02-09  7:23 ` [PATCH 3/7] t1400: exercise reflog with gaps with reftable backend Patrick Steinhardt
2024-02-14 22:59   ` Junio C Hamano
2024-02-09  7:23 ` [PATCH 4/7] t1404: make D/F conflict tests compatible " Patrick Steinhardt
2024-02-14 23:11   ` Junio C Hamano
2024-02-09  7:23 ` [PATCH 5/7] t1405: remove unneeded cleanup step Patrick Steinhardt
2024-02-14 23:17   ` Junio C Hamano
2024-02-15  7:59     ` Patrick Steinhardt
2024-02-09  7:23 ` [PATCH 6/7] t2011: exercise D/F conflicts with HEAD with the reftable backend Patrick Steinhardt
2024-02-09  7:23 ` [PATCH 7/7] t7003: ensure filter-branch prunes reflogs " Patrick Steinhardt
2024-02-11 14:00 ` [PATCH 0/7] t: drop more REFFILES prereqs Karthik Nayak
2024-02-14 23:20 ` Junio C Hamano
2024-02-15  8:14   ` Patrick Steinhardt
2024-02-15  8:25 ` [PATCH v2 " Patrick Steinhardt
2024-02-15  8:25   ` [PATCH v2 1/7] t: move tests exercising the "files" backend Patrick Steinhardt
2024-02-15  8:25   ` [PATCH v2 2/7] t0410: convert tests to use DEFAULT_REPO_FORMAT prereq Patrick Steinhardt
2024-02-15 18:19     ` Junio C Hamano
2024-02-15  8:25   ` [PATCH v2 3/7] t1400: exercise reflog with gaps with reftable backend Patrick Steinhardt
2024-02-15  8:25   ` [PATCH v2 4/7] t1404: make D/F conflict tests compatible " Patrick Steinhardt
2024-02-15  8:25   ` [PATCH v2 5/7] t1405: remove unneeded cleanup step Patrick Steinhardt
2024-02-15  8:25   ` [PATCH v2 6/7] t2011: exercise D/F conflicts with HEAD with the reftable backend Patrick Steinhardt
2024-02-15  8:25   ` [PATCH v2 7/7] t7003: ensure filter-branch prunes reflogs " Patrick Steinhardt

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).