t: drop more REFFILES prereqs

PATCH Superseded Patrick Steinhardt 3 participants 28 messages Feb 9, 2024 – Feb 15, 2024

lore

Share & tools

lore

Patrick Steinhardt

author

2024-02-09 07:23 UTC #0 lore raw

cover t: drop more REFFILES prereqs

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

Signature · 1 line

2.43.GIT

Patrick Steinhardt

author

2024-02-09 07:23 UTC #1 lore raw

PATCH 1/7 t: move tests exercising the "files" backend

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 <[email protected]>

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

t/t0600-reffiles-backend.sh +91 -0

index e6a5f1868f..485481d6b4 100755
@@ -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

t/t1301-shared-repo.sh +0 ~~-16~~

index 8e2c01e760..b1eb5c01b8 100755
@@ -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 &&

t/t1400-update-ref.sh +0 ~~-36~~

index f18843bf7a..3294b7ce08 100755
@@ -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

t/t3200-branch.sh +0 ~~-29~~

index de7d3014e4..e36f4d15f2 100755
@@ -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/* &&

t/t3400-rebase.sh +0 ~~-10~~

index 57f1392926..e1c8c5f701 100755
@@ -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 &&
 	(

Signature · 1 line

2.43.GIT

Junio C Hamano

2024-02-14 22:48 UTC #2 lore raw

Re: [PATCH 1/7] t: move tests exercising the "files" backend

Quoted 9 lines · Patrick Steinhardt

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.

Quoted 7 lines

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

t/t0600-reffiles-backend.sh test_expect_success 'log diagnoses bogus HEAD symref' ' · line 382

Quoted 7 lines

t/t0600-reffiles-backend.sh +0 -0

index e6a5f1868f..485481d6b4 100755
@@ -381,4 +381,95 @@ test_expect_success 'log diagnoses bogus HEAD symref' '
 	test_grep broken stderr
 '

These two are from t1400.

Quoted 16 lines

t/t0600-reffiles-backend.sh +16 -0

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

t/t0600-reffiles-backend.sh test_expect_success 'log diagnoses bogus HEAD symref' ' · line 409

Quoted 10 lines

t/t0600-reffiles-backend.sh +10 -0

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

t/t0600-reffiles-backend.sh test_expect_success 'log diagnoses bogus HEAD symref' ' · line 418

Quoted 8 lines

t/t0600-reffiles-backend.sh +8 -0

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

t/t0600-reffiles-backend.sh test_expect_success 'log diagnoses bogus HEAD symref' ' · line 425

Quoted 6 lines

t/t0600-reffiles-backend.sh +6 -0

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

Quoted 11 lines

t/t0600-reffiles-backend.sh +11 -0

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

t/t0600-reffiles-backend.sh test_expect_success 'log diagnoses bogus HEAD symref' ' · line 447

Quoted 10 lines

t/t0600-reffiles-backend.sh +10 -0

+	! 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.

t/t0600-reffiles-backend.sh test_expect_success 'log diagnoses bogus HEAD symref' ' · line 457

Quoted 9 lines

t/t0600-reffiles-backend.sh +9 -0

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

Quoted 5 lines

t/t0600-reffiles-backend.sh +5 -0

+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)" &&

t/t0600-reffiles-backend.sh test_expect_success 'log diagnoses bogus HEAD symref' ' · line 473

Quoted 10 lines

t/t0600-reffiles-backend.sh +10 -0

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

Quoted 15 lines

t/t0600-reffiles-backend.sh +0 -0

 test_done

t/t1301-shared-repo.sh +0 -6

index 8e2c01e760..b1eb5c01b8 100755
@@ -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

t/t1301-shared-repo.sh test_expect_success POSIXPERM 'info/refs respects umask in unshared repo' ' · line 155

Quoted 10 lines

t/t1301-shared-repo.sh +0 ~~-10~~

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

Quoted 43 lines

t/t1301-shared-repo.sh +0 -0

 test_expect_success POSIXPERM 'forced modes' '
 	test_when_finished "rm -rf new" &&
 	mkdir -p templates/hooks &&

t/t1400-update-ref.sh +0 ~~-27~~

index f18843bf7a..3294b7ce08 100755
@@ -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

t/t1400-update-ref.sh test_expect_success PIPE 'transaction flushes status updates' ' · line 1678

Quoted 10 lines

t/t1400-update-ref.sh +0 -8

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

Quoted 27 lines

t/t1400-update-ref.sh +0 -0

 test_done

t/t3200-branch.sh +0 ~~-18~~

index de7d3014e4..e36f4d15f2 100755
@@ -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 &&

t/t3200-branch.sh test_expect_success 'renaming a symref is not allowed' ' · line 866

Quoted 10 lines

t/t3200-branch.sh +0 ~~-10~~

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

Quoted 11 lines

t/t3400-rebase.sh +0 -3

index 57f1392926..e1c8c5f701 100755
@@ -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 &&

t/t3400-rebase.sh test_expect_success 'refuse to switch to branch checked out elsewhere' ' · line 429

Quoted 10 lines

t/t3400-rebase.sh +0 -7

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

Patrick Steinhardt

author

2024-02-09 07:23 UTC #3 lore raw

PATCH 2/7 t0410: enable tests with extensions with non-default repo format

In reply to Patrick Steinhardt

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 <[email protected]>

---
 t/t0410-partial-clone.sh | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

t/t0410-partial-clone.sh +2 -4

index 6b6424b3df..d913f3c453 100755
@@ -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"
 '

Signature · 1 line

2.43.GIT

Junio C Hamano

2024-02-14 22:58 UTC #4 lore raw

Re: [PATCH 2/7] t0410: enable tests with extensions with non-default repo format

Quoted 9 lines · Patrick Steinhardt

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.

Quoted 40 lines

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

Signed-off-by Patrick Steinhardt <[email protected]>

---
 t/t0410-partial-clone.sh | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

t/t0410-partial-clone.sh +2 -4

index 6b6424b3df..d913f3c453 100755
@@ -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"
 '

Patrick Steinhardt

author

2024-02-15 07:59 UTC #5 lore raw

Re: [PATCH 2/7] t0410: enable tests with extensions with non-default repo format

Quoted 20 lines · Junio C Hamano

Quoted 9 lines · Patrick Steinhardt
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.

Quoted 4 lines

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

Junio C Hamano

2024-02-15 17:18 UTC #6 lore raw

Re: [PATCH 2/7] t0410: enable tests with extensions with non-default repo format

Quoted 21 lines · Patrick Steinhardt

Quoted 20 lines · Junio C Hamano
Quoted 9 lines · Patrick Steinhardt
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.

Quoted 7 lines

Quoted 4 lines
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.

Patrick Steinhardt

author

2024-02-09 07:23 UTC #7 lore raw

PATCH 3/7 t1400: exercise reflog with gaps with reftable backend

In reply to Patrick Steinhardt

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 <[email protected]>

---
 t/t1400-update-ref.sh | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

t/t1400-update-ref.sh +7 -7

index 3294b7ce08..3aeb103d34 100755
@@ -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 &&

Signature · 1 line

2.43.GIT

Junio C Hamano

2024-02-14 22:59 UTC #8 lore raw

Re: [PATCH 3/7] t1400: exercise reflog with gaps with reftable backend

Quoted 8 lines · Patrick Steinhardt

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.

Quoted 32 lines

Signed-off-by Patrick Steinhardt <[email protected]>

---
 t/t1400-update-ref.sh | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

t/t1400-update-ref.sh +7 -7

index 3294b7ce08..3aeb103d34 100755
@@ -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 &&

Patrick Steinhardt

author

2024-02-09 07:23 UTC #9 lore raw

PATCH 4/7 t1404: make D/F conflict tests compatible with reftable backend

In reply to Patrick Steinhardt

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 <[email protected]>

---
 t/t1404-update-ref-errors.sh | 37 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 20 deletions(-)

t/t1404-update-ref-errors.sh +17 ~~-20~~

index 00b7013705..98e9158bd2 100755
@@ -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
 '

Signature · 1 line

2.43.GIT

Junio C Hamano

2024-02-14 23:11 UTC #10 lore raw

Re: [PATCH 4/7] t1404: make D/F conflict tests compatible with reftable backend

t/t1404-update-ref-errors.sh df_test() { · line 103

Quoted 3 lines · Patrick Steinhardt

t/t1404-update-ref-errors.sh +1 -1

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

t/t1404-update-ref-errors.sh test_expect_success 'one new ref is a simple prefix of another' ' · line 193

Quoted 8 lines

t/t1404-update-ref-errors.sh +1 -1

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

Patrick Steinhardt

author

2024-02-09 07:23 UTC #11 lore raw

PATCH 5/7 t1405: remove unneeded cleanup step

In reply to Patrick Steinhardt

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 <[email protected]>

---
 t/t1405-main-ref-store.sh | 6 ------
 1 file changed, 6 deletions(-)

t/t1405-main-ref-store.sh +0 -6

index 976bd71efb..1183232a72 100755
@@ -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 &&

Signature · 1 line

2.43.GIT

Junio C Hamano

2024-02-14 23:17 UTC #12 lore raw

Re: [PATCH 5/7] t1405: remove unneeded cleanup step

Quoted 13 lines · Patrick Steinhardt

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.

Quoted 6 lines

The cleanup-style test is thus not required anymore. Remove it.
Signed-off-by Patrick Steinhardt <[email protected]>
---
 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.

Quoted 18 lines

t/t1405-main-ref-store.sh +0 -6

index 976bd71efb..1183232a72 100755
@@ -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 &&

Patrick Steinhardt

author

2024-02-15 07:59 UTC #13 lore raw

Re: [PATCH 5/7] t1405: remove unneeded cleanup step

Quoted 33 lines · Junio C Hamano

Quoted 13 lines · Patrick Steinhardt
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.
Quoted 6 lines
The cleanup-style test is thus not required anymore. Remove it.
Signed-off-by Patrick Steinhardt <[email protected]>
---
 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.

Quoted 3 lines

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

Patrick Steinhardt

author

2024-02-09 07:23 UTC #14 lore raw

PATCH 6/7 t2011: exercise D/F conflicts with HEAD with the reftable backend

In reply to Patrick Steinhardt

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 <[email protected]>

---
 t/t2011-checkout-invalid-head.sh | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

t/t2011-checkout-invalid-head.sh +7 ~~-10~~

index 3c8135831b..04f53b1ea1 100755
@@ -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
 '

Signature · 1 line

2.43.GIT

Patrick Steinhardt

author

2024-02-09 07:23 UTC #15 lore raw

PATCH 7/7 t7003: ensure filter-branch prunes reflogs with the reftable backend

In reply to Patrick Steinhardt

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 <[email protected]>

---
 t/t7003-filter-branch.sh | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

t/t7003-filter-branch.sh +1 -4

index f6aebe92ff..5ab4d41ee7 100755
@@ -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' '

Signature · 1 line

2.43.GIT

Karthik Nayak

2024-02-11 14:00 UTC #16 lore raw

In reply to Patrick Steinhardt

Hello,

Quoted 10 lines · Patrick Steinhardt

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

Junio C Hamano

2024-02-14 23:20 UTC #17 lore raw

In reply to Patrick Steinhardt

Quoted 5 lines · Patrick Steinhardt

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.

Patrick Steinhardt

author

2024-02-15 08:15 UTC #18 lore raw

Quoted 10 lines · Junio C Hamano

Quoted 5 lines · Patrick Steinhardt
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

Patrick Steinhardt

author

2024-02-15 08:25 UTC #19 lore raw

v2 cover t: drop more REFFILES prereqs

In reply to Patrick Steinhardt

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 <[email protected]>

## 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 <[email protected]>

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

Signature · 1 line

2.44.0-rc0

Patrick Steinhardt

author

2024-02-15 08:25 UTC #20 lore raw

PATCH v2 1/7 t: move tests exercising the "files" backend

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 <[email protected]>

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

t/t0600-reffiles-backend.sh +91 -0

index e6a5f1868f..485481d6b4 100755
@@ -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

t/t1301-shared-repo.sh +0 ~~-16~~

index 8e2c01e760..b1eb5c01b8 100755
@@ -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 &&

t/t1400-update-ref.sh +0 ~~-36~~

index 78a09abc35..bf37763bd6 100755
@@ -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

t/t3200-branch.sh +0 ~~-29~~

index de7d3014e4..e36f4d15f2 100755
@@ -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/* &&

t/t3400-rebase.sh +0 ~~-10~~

index 57f1392926..e1c8c5f701 100755
@@ -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 &&
 	(

Signature · 1 line

2.44.0-rc0

Patrick Steinhardt

author

2024-02-15 08:25 UTC #21 lore raw

PATCH v2 2/7 t0410: convert tests to use DEFAULT_REPO_FORMAT prereq

In reply to Patrick Steinhardt

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 <[email protected]>

---
 t/t0410-partial-clone.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

t/t0410-partial-clone.sh +2 -2

index 6b6424b3df..0f98b21be8 100755
@@ -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 &&

Signature · 1 line

2.44.0-rc0

Junio C Hamano

2024-02-15 18:19 UTC #22 lore raw

Re: [PATCH v2 2/7] t0410: convert tests to use DEFAULT_REPO_FORMAT prereq

Quoted 4 lines · Patrick Steinhardt

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.

Quoted 14 lines

t/t0410-partial-clone.sh +2 -2

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

Patrick Steinhardt

author

2024-02-15 08:25 UTC #23 lore raw

PATCH v2 3/7 t1400: exercise reflog with gaps with reftable backend

In reply to Patrick Steinhardt

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

Signed-off-by Patrick Steinhardt <[email protected]>

---
 t/t1400-update-ref.sh | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

t/t1400-update-ref.sh +7 -7

index bf37763bd6..6ebc3ef945 100755
@@ -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 &&

Signature · 1 line

2.44.0-rc0

Patrick Steinhardt

author

2024-02-15 08:25 UTC #24 lore raw

PATCH v2 4/7 t1404: make D/F conflict tests compatible with reftable backend

In reply to Patrick Steinhardt

Signed-off-by Patrick Steinhardt <[email protected]>

---
 t/t1404-update-ref-errors.sh | 37 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 20 deletions(-)

t/t1404-update-ref-errors.sh +17 ~~-20~~

index 00b7013705..98e9158bd2 100755
@@ -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
 '

Signature · 1 line

2.44.0-rc0

Patrick Steinhardt

author

2024-02-15 08:25 UTC #25 lore raw

PATCH v2 5/7 t1405: remove unneeded cleanup step

In reply to Patrick Steinhardt

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

Signed-off-by Patrick Steinhardt <[email protected]>

---
 t/t1405-main-ref-store.sh | 6 ------
 1 file changed, 6 deletions(-)

t/t1405-main-ref-store.sh +0 -6

index 976bd71efb..1183232a72 100755
@@ -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 &&

Signature · 1 line

2.44.0-rc0

Patrick Steinhardt

author

2024-02-15 08:25 UTC #26 lore raw

PATCH v2 6/7 t2011: exercise D/F conflicts with HEAD with the reftable backend

In reply to Patrick Steinhardt

Signed-off-by Patrick Steinhardt <[email protected]>

---
 t/t2011-checkout-invalid-head.sh | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

t/t2011-checkout-invalid-head.sh +7 ~~-10~~

index 3c8135831b..04f53b1ea1 100755
@@ -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
 '

Signature · 1 line

2.44.0-rc0

Patrick Steinhardt

author

2024-02-15 08:25 UTC #27 lore raw

PATCH v2 7/7 t7003: ensure filter-branch prunes reflogs with the reftable backend

In reply to Patrick Steinhardt

Remove it.

Signed-off-by Patrick Steinhardt <[email protected]>

---
 t/t7003-filter-branch.sh | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

t/t7003-filter-branch.sh +1 -4

index f6aebe92ff..5ab4d41ee7 100755
@@ -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' '

Signature · 1 line

2.44.0-rc0