[PATCH 0/6] Fix direct filesystem access in various test files.

[PATCH 0/6] Fix direct filesystem access in various test files.

[Han-Wen Nienhuys via GitGitGadget]

This fixes a few test failures in the reftable series.

Han-Wen Nienhuys (6):
  t6050: use git-update-ref rather than filesystem access
  t1503: mark symlink test as REFFILES
  t6120: use git-update-ref rather than filesystem access
  t7509: use git-update-ref rather than filesystem access
  t3320: use git-symbolic-ref rather than filesystem access
  t2402: use ref-store test helper to create broken symlink

 t/t1503-rev-parse-verify.sh      |  2 +-
 t/t2402-worktree-list.sh         |  2 +-
 t/t3320-notes-merge-worktrees.sh | 12 +++++++-----
 t/t6050-replace.sh               |  2 +-
 t/t6120-describe.sh              |  6 ++++--
 t/t7509-commit-authorship.sh     |  4 ++--
 6 files changed, 16 insertions(+), 12 deletions(-)


base-commit: 75ae10bc75336db031ee58d13c5037b929235912
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1052%2Fhanwen%2Ftest-fixes-v3-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1052/hanwen/test-fixes-v3-v1
Pull-Request: https://github.com/git/git/pull/1052
-- 
gitgitgadget

[PATCH 1/6] t6050: use git-update-ref rather than filesystem access

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t6050-replace.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index e33d512ec11..2500acc2ef8 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -132,7 +132,7 @@ tagger T A Gger <> 0 +0000
 EOF
 
 test_expect_success 'tag replaced commit' '
-     git mktag <tag.sig >.git/refs/tags/mytag
+     git update-ref refs/tags/mytag $(git mktag <tag.sig)
 '
 
 test_expect_success '"git fsck" works' '
-- 
gitgitgadget

[PATCH 2/6] t1503: mark symlink test as REFFILES

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t1503-rev-parse-verify.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh
index bf081023917..40958615ebb 100755
--- a/t/t1503-rev-parse-verify.sh
+++ b/t/t1503-rev-parse-verify.sh
@@ -142,7 +142,7 @@ test_expect_success 'main@{n} for various n' '
 	test_must_fail git rev-parse --verify main@{$Np1}
 '
 
-test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '
+test_expect_success SYMLINKS,REFFILES 'ref resolution not confused by broken symlinks' '
 	ln -s does-not-exist .git/refs/heads/broken &&
 	test_must_fail git rev-parse --verify broken
 '
-- 
gitgitgadget

[PATCH 3/6] t6120: use git-update-ref rather than filesystem access

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t6120-describe.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 1a501ee09e1..bae2419150b 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -107,7 +107,8 @@ test_expect_success 'describe --contains defaults to HEAD without commit-ish' '
 check_describe tags/A --all A^0
 
 test_expect_success 'renaming tag A to Q locally produces a warning' "
-	mv .git/refs/tags/A .git/refs/tags/Q &&
+	git update-ref refs/tags/Q $(git rev-parse refs/tags/A) &&
+	git update-ref -d refs/tags/A &&
 	git describe HEAD 2>err >out &&
 	cat >expected <<-\EOF &&
 	warning: tag 'Q' is externally known as 'A'
@@ -135,7 +136,8 @@ test_expect_success 'abbrev=0 will not break misplaced tag (2)' '
 '
 
 test_expect_success 'rename tag Q back to A' '
-	mv .git/refs/tags/Q .git/refs/tags/A
+	git update-ref refs/tags/A $(git rev-parse refs/tags/Q) &&
+	git update-ref -d refs/tags/Q
 '
 
 test_expect_success 'pack tag refs' 'git pack-refs'
-- 
gitgitgadget

[PATCH 4/6] t7509: use git-update-ref rather than filesystem access

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t7509-commit-authorship.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t7509-commit-authorship.sh b/t/t7509-commit-authorship.sh
index ee6c47416ed..d568593382c 100755
--- a/t/t7509-commit-authorship.sh
+++ b/t/t7509-commit-authorship.sh
@@ -147,7 +147,7 @@ test_expect_success 'commit respects CHERRY_PICK_HEAD and MERGE_MSG' '
 	test_tick &&
 	git commit -am "cherry-pick 1" --author="Cherry <cherry@pick.er>" &&
 	git tag cherry-pick-head &&
-	git rev-parse cherry-pick-head >.git/CHERRY_PICK_HEAD &&
+	git update-ref CHERRY_PICK_HEAD $(git rev-parse cherry-pick-head) &&
 	echo "This is a MERGE_MSG" >.git/MERGE_MSG &&
 	echo "cherry-pick 1b" >>foo &&
 	test_tick &&
@@ -162,7 +162,7 @@ test_expect_success 'commit respects CHERRY_PICK_HEAD and MERGE_MSG' '
 '
 
 test_expect_success '--reset-author with CHERRY_PICK_HEAD' '
-	git rev-parse cherry-pick-head >.git/CHERRY_PICK_HEAD &&
+	git update-ref CHERRY_PICK_HEAD $(git rev-parse cherry-pick-head) &&
 	echo "cherry-pick 2" >>foo &&
 	test_tick &&
 	git commit -am "cherry-pick 2" --reset-author &&
-- 
gitgitgadget

[PATCH 5/6] t3320: use git-symbolic-ref rather than filesystem access

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t3320-notes-merge-worktrees.sh | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/t/t3320-notes-merge-worktrees.sh b/t/t3320-notes-merge-worktrees.sh
index 052516e6c6a..6b2d507f3e7 100755
--- a/t/t3320-notes-merge-worktrees.sh
+++ b/t/t3320-notes-merge-worktrees.sh
@@ -46,8 +46,9 @@ test_expect_success 'create some new worktrees' '
 test_expect_success 'merge z into y fails and sets NOTES_MERGE_REF' '
 	git config core.notesRef refs/notes/y &&
 	test_must_fail git notes merge z &&
-	echo "ref: refs/notes/y" >expect &&
-	test_cmp expect .git/NOTES_MERGE_REF
+	echo "refs/notes/y" >expect &&
+	git symbolic-ref NOTES_MERGE_REF >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success 'merge z into y while mid-merge in another workdir fails' '
@@ -57,7 +58,7 @@ test_expect_success 'merge z into y while mid-merge in another workdir fails' '
 		test_must_fail git notes merge z 2>err &&
 		test_i18ngrep "a notes merge into refs/notes/y is already in-progress at" err
 	) &&
-	test_path_is_missing .git/worktrees/worktree/NOTES_MERGE_REF
+	test_must_fail git -C worktree symbolic-ref NOTES_MERGE_REF
 '
 
 test_expect_success 'merge z into x while mid-merge on y succeeds' '
@@ -68,8 +69,9 @@ test_expect_success 'merge z into x while mid-merge on y succeeds' '
 		test_i18ngrep "Automatic notes merge failed" out &&
 		grep -v "A notes merge into refs/notes/x is already in-progress in" out
 	) &&
-	echo "ref: refs/notes/x" >expect &&
-	test_cmp expect .git/worktrees/worktree2/NOTES_MERGE_REF
+	echo "refs/notes/x" >expect &&
+	git -C worktree2 symbolic-ref NOTES_MERGE_REF >actual &&
+	test_cmp expect actual
 '
 
 test_done
-- 
gitgitgadget

[PATCH 6/6] t2402: use ref-store test helper to create broken symlink

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t2402-worktree-list.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh
index fedcefe8de3..4012bd67b04 100755
--- a/t/t2402-worktree-list.sh
+++ b/t/t2402-worktree-list.sh
@@ -230,7 +230,7 @@ test_expect_success 'broken main worktree still at the top' '
 		EOF
 		cd linked &&
 		echo "worktree $(pwd)" >expected &&
-		echo "ref: .broken" >../.git/HEAD &&
+		(cd ../ && test-tool ref-store main create-symref HEAD .broken ) &&
 		git worktree list --porcelain >out &&
 		head -n 3 out >actual &&
 		test_cmp ../expected actual &&
-- 
gitgitgadget

Re: [PATCH 4/6] t7509: use git-update-ref rather than filesystem access

[Junio C Hamano]

"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Han-Wen Nienhuys <hanwen@google.com>
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
>  t/t7509-commit-authorship.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

This seems identical to one of the commits in hn/refs-test-cleanup
that has already been merged to 'master', so I'll drop this copy.

Thanks.


> diff --git a/t/t7509-commit-authorship.sh b/t/t7509-commit-authorship.sh
> index ee6c47416ed..d568593382c 100755
> --- a/t/t7509-commit-authorship.sh
> +++ b/t/t7509-commit-authorship.sh
> @@ -147,7 +147,7 @@ test_expect_success 'commit respects CHERRY_PICK_HEAD and MERGE_MSG' '
>  	test_tick &&
>  	git commit -am "cherry-pick 1" --author="Cherry <cherry@pick.er>" &&
>  	git tag cherry-pick-head &&
> -	git rev-parse cherry-pick-head >.git/CHERRY_PICK_HEAD &&
> +	git update-ref CHERRY_PICK_HEAD $(git rev-parse cherry-pick-head) &&
>  	echo "This is a MERGE_MSG" >.git/MERGE_MSG &&
>  	echo "cherry-pick 1b" >>foo &&
>  	test_tick &&
> @@ -162,7 +162,7 @@ test_expect_success 'commit respects CHERRY_PICK_HEAD and MERGE_MSG' '
>  '
>  
>  test_expect_success '--reset-author with CHERRY_PICK_HEAD' '
> -	git rev-parse cherry-pick-head >.git/CHERRY_PICK_HEAD &&
> +	git update-ref CHERRY_PICK_HEAD $(git rev-parse cherry-pick-head) &&
>  	echo "cherry-pick 2" >>foo &&
>  	test_tick &&
>  	git commit -am "cherry-pick 2" --reset-author &&

[PATCH v2 00/11] Fix direct filesystem access in various test files.

[Han-Wen Nienhuys via GitGitGadget]

This fixes a few test failures in the reftable series.

Han-Wen Nienhuys (11):
  t6050: use git-update-ref rather than filesystem access
  t1503: mark symlink test as REFFILES
  t6120: use git-update-ref rather than filesystem access
  t3320: use git-symbolic-ref rather than filesystem access
  t2402: use ref-store test helper to create broken symlink
  t1405: use 'git reflog exists' to check reflog existence
  t1405: mark test for 'git pack-refs' as REFFILES
  t1410: mark test as REFFILES
  t7064: use update-ref -d to remove upstream branch
  t6500: use "ls -1" to snapshot ref database state
  t6001: avoid direct file system access

 t/t1405-main-ref-store.sh        | 12 ++++++++----
 t/t1410-reflog.sh                |  4 +++-
 t/t1503-rev-parse-verify.sh      |  2 +-
 t/t2402-worktree-list.sh         |  2 +-
 t/t3320-notes-merge-worktrees.sh | 12 +++++++-----
 t/t6001-rev-list-graft.sh        |  3 ++-
 t/t6050-replace.sh               |  2 +-
 t/t6120-describe.sh              |  6 ++++--
 t/t6500-gc.sh                    |  5 +++--
 t/t7064-wtstatus-pv2.sh          |  5 +----
 10 files changed, 31 insertions(+), 22 deletions(-)


base-commit: daab8a564f8bbac55f70f8bf86c070e001a9b006
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1052%2Fhanwen%2Ftest-fixes-v3-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1052/hanwen/test-fixes-v3-v2
Pull-Request: https://github.com/git/git/pull/1052

Range-diff vs v1:

  1:  61a5c3718df =  1:  d7616038d88 t6050: use git-update-ref rather than filesystem access
  2:  49994a0d154 =  2:  cbd2128c331 t1503: mark symlink test as REFFILES
  3:  695921cacb6 =  3:  cbf90c13e5b t6120: use git-update-ref rather than filesystem access
  4:  94d7e144f54 <  -:  ----------- t7509: use git-update-ref rather than filesystem access
  5:  cc400e9131d =  4:  7b98d092811 t3320: use git-symbolic-ref rather than filesystem access
  6:  4d83f70e133 =  5:  0047266de6b t2402: use ref-store test helper to create broken symlink
  -:  ----------- >  6:  eb2c53d19cf t1405: use 'git reflog exists' to check reflog existence
  -:  ----------- >  7:  05dead16f1c t1405: mark test for 'git pack-refs' as REFFILES
  -:  ----------- >  8:  f931a26de58 t1410: mark test as REFFILES
  -:  ----------- >  9:  3685ba90f60 t7064: use update-ref -d to remove upstream branch
  -:  ----------- > 10:  ccdbf3749fe t6500: use "ls -1" to snapshot ref database state
  -:  ----------- > 11:  81e46f95536 t6001: avoid direct file system access

-- 
gitgitgadget

[PATCH v2 01/11] t6050: use git-update-ref rather than filesystem access

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t6050-replace.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index e33d512ec11..2500acc2ef8 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -132,7 +132,7 @@ tagger T A Gger <> 0 +0000
 EOF
 
 test_expect_success 'tag replaced commit' '
-     git mktag <tag.sig >.git/refs/tags/mytag
+     git update-ref refs/tags/mytag $(git mktag <tag.sig)
 '
 
 test_expect_success '"git fsck" works' '
-- 
gitgitgadget

[PATCH v2 02/11] t1503: mark symlink test as REFFILES

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t1503-rev-parse-verify.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh
index bf081023917..40958615ebb 100755
--- a/t/t1503-rev-parse-verify.sh
+++ b/t/t1503-rev-parse-verify.sh
@@ -142,7 +142,7 @@ test_expect_success 'main@{n} for various n' '
 	test_must_fail git rev-parse --verify main@{$Np1}
 '
 
-test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '
+test_expect_success SYMLINKS,REFFILES 'ref resolution not confused by broken symlinks' '
 	ln -s does-not-exist .git/refs/heads/broken &&
 	test_must_fail git rev-parse --verify broken
 '
-- 
gitgitgadget

[PATCH v2 03/11] t6120: use git-update-ref rather than filesystem access

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t6120-describe.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 1a501ee09e1..bae2419150b 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -107,7 +107,8 @@ test_expect_success 'describe --contains defaults to HEAD without commit-ish' '
 check_describe tags/A --all A^0
 
 test_expect_success 'renaming tag A to Q locally produces a warning' "
-	mv .git/refs/tags/A .git/refs/tags/Q &&
+	git update-ref refs/tags/Q $(git rev-parse refs/tags/A) &&
+	git update-ref -d refs/tags/A &&
 	git describe HEAD 2>err >out &&
 	cat >expected <<-\EOF &&
 	warning: tag 'Q' is externally known as 'A'
@@ -135,7 +136,8 @@ test_expect_success 'abbrev=0 will not break misplaced tag (2)' '
 '
 
 test_expect_success 'rename tag Q back to A' '
-	mv .git/refs/tags/Q .git/refs/tags/A
+	git update-ref refs/tags/A $(git rev-parse refs/tags/Q) &&
+	git update-ref -d refs/tags/Q
 '
 
 test_expect_success 'pack tag refs' 'git pack-refs'
-- 
gitgitgadget

[PATCH v2 04/11] t3320: use git-symbolic-ref rather than filesystem access

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t3320-notes-merge-worktrees.sh | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/t/t3320-notes-merge-worktrees.sh b/t/t3320-notes-merge-worktrees.sh
index 052516e6c6a..6b2d507f3e7 100755
--- a/t/t3320-notes-merge-worktrees.sh
+++ b/t/t3320-notes-merge-worktrees.sh
@@ -46,8 +46,9 @@ test_expect_success 'create some new worktrees' '
 test_expect_success 'merge z into y fails and sets NOTES_MERGE_REF' '
 	git config core.notesRef refs/notes/y &&
 	test_must_fail git notes merge z &&
-	echo "ref: refs/notes/y" >expect &&
-	test_cmp expect .git/NOTES_MERGE_REF
+	echo "refs/notes/y" >expect &&
+	git symbolic-ref NOTES_MERGE_REF >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success 'merge z into y while mid-merge in another workdir fails' '
@@ -57,7 +58,7 @@ test_expect_success 'merge z into y while mid-merge in another workdir fails' '
 		test_must_fail git notes merge z 2>err &&
 		test_i18ngrep "a notes merge into refs/notes/y is already in-progress at" err
 	) &&
-	test_path_is_missing .git/worktrees/worktree/NOTES_MERGE_REF
+	test_must_fail git -C worktree symbolic-ref NOTES_MERGE_REF
 '
 
 test_expect_success 'merge z into x while mid-merge on y succeeds' '
@@ -68,8 +69,9 @@ test_expect_success 'merge z into x while mid-merge on y succeeds' '
 		test_i18ngrep "Automatic notes merge failed" out &&
 		grep -v "A notes merge into refs/notes/x is already in-progress in" out
 	) &&
-	echo "ref: refs/notes/x" >expect &&
-	test_cmp expect .git/worktrees/worktree2/NOTES_MERGE_REF
+	echo "refs/notes/x" >expect &&
+	git -C worktree2 symbolic-ref NOTES_MERGE_REF >actual &&
+	test_cmp expect actual
 '
 
 test_done
-- 
gitgitgadget

[PATCH v2 05/11] t2402: use ref-store test helper to create broken symlink

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t2402-worktree-list.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh
index fedcefe8de3..4012bd67b04 100755
--- a/t/t2402-worktree-list.sh
+++ b/t/t2402-worktree-list.sh
@@ -230,7 +230,7 @@ test_expect_success 'broken main worktree still at the top' '
 		EOF
 		cd linked &&
 		echo "worktree $(pwd)" >expected &&
-		echo "ref: .broken" >../.git/HEAD &&
+		(cd ../ && test-tool ref-store main create-symref HEAD .broken ) &&
 		git worktree list --porcelain >out &&
 		head -n 3 out >actual &&
 		test_cmp ../expected actual &&
-- 
gitgitgadget

[PATCH v2 06/11] t1405: use 'git reflog exists' to check reflog existence

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

This fixes a test failure for reftable.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t1405-main-ref-store.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
index a237d9880ea..92b04873247 100755
--- a/t/t1405-main-ref-store.sh
+++ b/t/t1405-main-ref-store.sh
@@ -98,12 +98,12 @@ test_expect_success 'reflog_exists(HEAD)' '
 
 test_expect_success 'delete_reflog(HEAD)' '
 	$RUN delete-reflog HEAD &&
-	! test -f .git/logs/HEAD
+	test_must_fail git reflog exists HEAD
 '
 
 test_expect_success 'create-reflog(HEAD)' '
 	$RUN create-reflog HEAD 1 &&
-	test -f .git/logs/HEAD
+	git reflog exists HEAD
 '
 
 test_expect_success 'delete_ref(refs/heads/foo)' '
-- 
gitgitgadget

[PATCH v2 07/11] t1405: mark test for 'git pack-refs' as REFFILES

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

The outcome of the pack-refs operation is not checked. This was apparently
forgotten in the commit introducing this test: 16feb99d (Mar 26 2017, "t1405:
some basic tests on main ref store").

I tried adding the obvious check this, but it seems to fail on the freebsd_12
builder.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t1405-main-ref-store.sh | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
index 92b04873247..3517f54961d 100755
--- a/t/t1405-main-ref-store.sh
+++ b/t/t1405-main-ref-store.sh
@@ -9,8 +9,12 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 RUN="test-tool ref-store main"
 
-test_expect_success 'pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE)' '
-	test_commit one &&
+
+test_expect_success 'setup' '
+	test_commit one
+'
+
+test_expect_success REFFILES 'pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE)' '
 	N=`find .git/refs -type f | wc -l` &&
 	test "$N" != 0 &&
 	$RUN pack-refs 3 &&
-- 
gitgitgadget

[PATCH v2 08/11] t1410: mark test as REFFILES

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

In reftable, one cannot take a lock on an individual ref.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t1410-reflog.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
index 27b9080251a..d42f067ff8c 100755
--- a/t/t1410-reflog.sh
+++ b/t/t1410-reflog.sh
@@ -374,7 +374,9 @@ test_expect_failure 'reflog with non-commit entries displays all entries' '
 	test_line_count = 3 actual
 '
 
-test_expect_success 'reflog expire operates on symref not referrent' '
+# This test takes a lock on an individual ref; this is not supported in
+# reftable.
+test_expect_success REFFILES 'reflog expire operates on symref not referrent' '
 	git branch --create-reflog the_symref &&
 	git branch --create-reflog referrent &&
 	git update-ref referrent HEAD &&
-- 
gitgitgadget

[PATCH v2 09/11] t7064: use update-ref -d to remove upstream branch

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

The previous code tested this by writing $ZERO_OID explicitly in the packed-refs
file. This is a type of corruption that doesn't reflect realistic use-cases. In
addition, even the ref-store test-tool refuses to write invalid OIDs.
(update-ref interprets $ZERO_OID is deleting the ref).

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t7064-wtstatus-pv2.sh | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh
index 4613882caff..eeb0534163d 100755
--- a/t/t7064-wtstatus-pv2.sh
+++ b/t/t7064-wtstatus-pv2.sh
@@ -373,10 +373,7 @@ test_expect_success 'verify upstream fields in branch header' '
 
 		## Test upstream-gone case. Fake this by pointing
 		## origin/initial-branch at a non-existing commit.
-		OLD=$(git rev-parse origin/initial-branch) &&
-		NEW=$ZERO_OID &&
-		mv .git/packed-refs .git/old-packed-refs &&
-		sed "s/$OLD/$NEW/g" <.git/old-packed-refs >.git/packed-refs &&
+		git update-ref -d refs/remotes/origin/initial-branch &&
 
 		HUF=$(git rev-parse HEAD) &&
 
-- 
gitgitgadget

[PATCH v2 10/11] t6500: use "ls -1" to snapshot ref database state

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

By doing ls -1 .git/{reftable,refs/heads}, we can capture changes to both
reftable and packed/loose ref storage.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t6500-gc.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index 10c7ae7f09c..c2021267f2c 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -241,7 +241,7 @@ test_expect_success 'background auto gc respects lock for all operations' '
 
 	# create a ref whose loose presence we can use to detect a pack-refs run
 	git update-ref refs/heads/should-be-loose HEAD &&
-	test_path_is_file .git/refs/heads/should-be-loose &&
+	(ls -1 .git/refs/heads .git/reftable >expect || true) &&
 
 	# now fake a concurrent gc that holds the lock; we can use our
 	# shell pid so that it looks valid.
@@ -258,7 +258,8 @@ test_expect_success 'background auto gc respects lock for all operations' '
 
 	# our gc should exit zero without doing anything
 	run_and_wait_for_auto_gc &&
-	test_path_is_file .git/refs/heads/should-be-loose
+	(ls -1 .git/refs/heads .git/reftable >actual || true) &&
+	test_cmp expect actual
 '
 
 # DO NOT leave a detached auto gc process running near the end of the
-- 
gitgitgadget

[PATCH v2 11/11] t6001: avoid direct file system access

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t6001-rev-list-graft.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/t/t6001-rev-list-graft.sh b/t/t6001-rev-list-graft.sh
index 90d93f77fa7..7294147334a 100755
--- a/t/t6001-rev-list-graft.sh
+++ b/t/t6001-rev-list-graft.sh
@@ -23,7 +23,8 @@ test_expect_success setup '
 	git commit -a -m "Third in one history." &&
 	A2=$(git rev-parse --verify HEAD) &&
 
-	rm -f .git/refs/heads/main .git/index &&
+	git update-ref -d refs/heads/main &&
+	rm -f .git/index &&
 
 	echo >fileA fileA again &&
 	echo >subdir/fileB fileB again &&
-- 
gitgitgadget

Re: [PATCH 0/6] Fix direct filesystem access in various test files.

[Junio C Hamano]

"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:

> This fixes a few test failures in the reftable series.
>
> Han-Wen Nienhuys (6):
>   t6050: use git-update-ref rather than filesystem access
>   t1503: mark symlink test as REFFILES
>   t6120: use git-update-ref rather than filesystem access
>   t7509: use git-update-ref rather than filesystem access
>   t3320: use git-symbolic-ref rather than filesystem access
>   t2402: use ref-store test helper to create broken symlink

Modulo for the duplicated one, the other 5 looked all sensible and
are now part of 'next'.

Thanks.

Re: [PATCH v2 06/11] t1405: use 'git reflog exists' to check reflog existence

[Junio C Hamano]

"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Han-Wen Nienhuys <hanwen@google.com>
>
> This fixes a test failure for reftable.
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
>  t/t1405-main-ref-store.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
> index a237d9880ea..92b04873247 100755
> --- a/t/t1405-main-ref-store.sh
> +++ b/t/t1405-main-ref-store.sh
> @@ -98,12 +98,12 @@ test_expect_success 'reflog_exists(HEAD)' '
>  
>  test_expect_success 'delete_reflog(HEAD)' '
>  	$RUN delete-reflog HEAD &&
> -	! test -f .git/logs/HEAD
> +	test_must_fail git reflog exists HEAD
>  '
>  
>  test_expect_success 'create-reflog(HEAD)' '
>  	$RUN create-reflog HEAD 1 &&
> -	test -f .git/logs/HEAD
> +	git reflog exists HEAD
>  '

Obviously more logical.  Thanks.

Re: [PATCH v2 07/11] t1405: mark test for 'git pack-refs' as REFFILES

[Junio C Hamano]

"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Han-Wen Nienhuys <hanwen@google.com>
>
> The outcome of the pack-refs operation is not checked. This was apparently
> forgotten in the commit introducing this test: 16feb99d (Mar 26 2017, "t1405:
> some basic tests on main ref store").
>
> I tried adding the obvious check this, but it seems to fail on the freebsd_12
> builder.

These two paragraphs may state the truth, but readers would
appreciate if it is clarified that they are both side notes that do
not necessarily help understanding the change being made with the
patch.  Something like this

    The number of loose ref files under .git/refs/ directory before
    and after running the "git pack-refs" command only matter when
    the files ref-backend is in use, so hide the test behind the
    REFFILES prerequisite.

would be needed before these sidenotes.

With a larger context, as you said, 

    diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
    index 92b0487324..3517f54961 100755
    --- a/t/t1405-main-ref-store.sh
    +++ b/t/t1405-main-ref-store.sh
    @@ -9,10 +9,14 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME

     RUN="test-tool ref-store main"

    -test_expect_success 'pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE)' '
    -	test_commit one &&
    +
    +test_expect_success 'setup' '
    +	test_commit one
    +'
    +
    +test_expect_success REFFILES 'pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE)' '
            N=`find .git/refs -type f | wc -l` &&
            test "$N" != 0 &&
            $RUN pack-refs 3 &&
            N=`find .git/refs -type f | wc -l`
     '

we do count the number of regular files in .git/refs hierarchy after
running "test-tool ref-store main pack-refs 3" (whatever that means).

I think your "obvious check" would have been something like the
attached?  How would that break?  It is not the "'wc -l' pads the
output with extra spaces" we often see on macOS, is it?

Thanks.

 t/t1405-main-ref-store.sh | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git c/t/t1405-main-ref-store.sh w/t/t1405-main-ref-store.sh
index 3517f54961..9dc88d7580 100755
--- c/t/t1405-main-ref-store.sh
+++ w/t/t1405-main-ref-store.sh
@@ -15,10 +15,11 @@ test_expect_success 'setup' '
 '
 
 test_expect_success REFFILES 'pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE)' '
-	N=`find .git/refs -type f | wc -l` &&
-	test "$N" != 0 &&
+	N=$(find .git/refs -type f | wc -l) &&
+	test $N -ne 0 &&
 	$RUN pack-refs 3 &&
-	N=`find .git/refs -type f | wc -l`
+	N=$(find .git/refs -type f | wc -l) &&
+	test $N -eq 0
 '
 
 test_expect_success 'create_symref(FOO, refs/heads/main)' '

Re: [PATCH v2 08/11] t1410: mark test as REFFILES

[Junio C Hamano]

"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Han-Wen Nienhuys <hanwen@google.com>
>
> In reftable, one cannot take a lock on an individual ref.
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
>  t/t1410-reflog.sh | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Makes sense, I guess, but it makes it sound as if you can only take
a big single lock to freeze everything (as opposed to being able to
work on independent refs in parallel).  Is that what you meant to
say?

A file somewhere in .git/refs/ directory shouldn't be a way to lock
a ref in the reftable world, either, so it doubly makes sense.

> diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
> index 27b9080251a..d42f067ff8c 100755
> --- a/t/t1410-reflog.sh
> +++ b/t/t1410-reflog.sh
> @@ -374,7 +374,9 @@ test_expect_failure 'reflog with non-commit entries displays all entries' '
>  	test_line_count = 3 actual
>  '
>  
> -test_expect_success 'reflog expire operates on symref not referrent' '
> +# This test takes a lock on an individual ref; this is not supported in
> +# reftable.
> +test_expect_success REFFILES 'reflog expire operates on symref not referrent' '
>  	git branch --create-reflog the_symref &&
>  	git branch --create-reflog referrent &&
>  	git update-ref referrent HEAD &&

Re: [PATCH v2 09/11] t7064: use update-ref -d to remove upstream branch

[Junio C Hamano]

"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:

>  		## Test upstream-gone case. Fake this by pointing
>  		## origin/initial-branch at a non-existing commit.
> -		OLD=$(git rev-parse origin/initial-branch) &&
> -		NEW=$ZERO_OID &&
> -		mv .git/packed-refs .git/old-packed-refs &&
> -		sed "s/$OLD/$NEW/g" <.git/old-packed-refs >.git/packed-refs &&
> +		git update-ref -d refs/remotes/origin/initial-branch &&

OK, so we now "fake" upstream-gone by making the upstream actually
gone ;-)  OK.

>  
>  		HUF=$(git rev-parse HEAD) &&

Re: [PATCH v2 11/11] t6001: avoid direct file system access

[Junio C Hamano]

"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Han-Wen Nienhuys <hanwen@google.com>
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
>  t/t6001-rev-list-graft.sh | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/t/t6001-rev-list-graft.sh b/t/t6001-rev-list-graft.sh
> index 90d93f77fa7..7294147334a 100755
> --- a/t/t6001-rev-list-graft.sh
> +++ b/t/t6001-rev-list-graft.sh
> @@ -23,7 +23,8 @@ test_expect_success setup '
>  	git commit -a -m "Third in one history." &&
>  	A2=$(git rev-parse --verify HEAD) &&
>  
> -	rm -f .git/refs/heads/main .git/index &&
> +	git update-ref -d refs/heads/main &&
> +	rm -f .git/index &&

Makes sense.  We might want a further clean-up to avoid removing the
index file by hand but that is totally outside the scope of the
"refs" series.

Thanks.

Re: [PATCH v2 00/11] Fix direct filesystem access in various test files.

[Junio C Hamano]

"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:

> This fixes a few test failures in the reftable series.

The early part is already in 'next', and the new ones looked mostly
good.

I didn't quite understand what was going on with "ls -1" though
(what does it even mean to "ls -1" enumerate only the filenames in
.git/reftable directory)?

Thanks.

Re: [PATCH v2 00/11] Fix direct filesystem access in various test files.

[Han-Wen Nienhuys]

On Fri, Jul 23, 2021 at 7:44 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > This fixes a few test failures in the reftable series.
>
> The early part is already in 'next', and the new ones looked mostly
> good.
>
> I didn't quite understand what was going on with "ls -1" though
> (what does it even mean to "ls -1" enumerate only the filenames in
> .git/reftable directory)?


By listing the files before and after, we can make sure that the ref
storage wasn't changed.

(perhaps "find -type f" is a better way to do this, though).

-- 
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--
Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado

Re: [PATCH v2 00/11] Fix direct filesystem access in various test files.

[Junio C Hamano]

Han-Wen Nienhuys <hanwen@google.com> writes:

> On Fri, Jul 23, 2021 at 7:44 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>
>> > This fixes a few test failures in the reftable series.
>>
>> The early part is already in 'next', and the new ones looked mostly
>> good.
>>
>> I didn't quite understand what was going on with "ls -1" though
>> (what does it even mean to "ls -1" enumerate only the filenames in
>> .git/reftable directory)?
>
>
> By listing the files before and after, we can make sure that the ref
> storage wasn't changed.

Meaning that we rely on the fact that we never append to an existing
file?

Thanks.

Re: [PATCH v2 00/11] Fix direct filesystem access in various test files.

[Junio C Hamano]

Han-Wen Nienhuys <hanwen@google.com> writes:

> On Fri, Jul 23, 2021 at 7:44 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>
>> > This fixes a few test failures in the reftable series.
>>
>> The early part is already in 'next', and the new ones looked mostly
>> good.
>>
>> I didn't quite understand what was going on with "ls -1" though
>> (what does it even mean to "ls -1" enumerate only the filenames in
>> .git/reftable directory)?
>
>
> By listing the files before and after, we can make sure that the ref
> storage wasn't changed.

Meaning that we rely on the fact that we never append to an existing
file?  A comment here would have helped.

Thanks.

[PATCH v3 00/11] Fix direct filesystem access in various test files.

[Han-Wen Nienhuys via GitGitGadget]

This fixes a few test failures in the reftable series.

Han-Wen Nienhuys (11):
  t6050: use git-update-ref rather than filesystem access
  t1503: mark symlink test as REFFILES
  t6120: use git-update-ref rather than filesystem access
  t3320: use git-symbolic-ref rather than filesystem access
  t2402: use ref-store test helper to create broken symlink
  t1405: use 'git reflog exists' to check reflog existence
  t1405: mark test for 'git pack-refs' as REFFILES
  t1410: mark test as REFFILES
  t7064: use update-ref -d to remove upstream branch
  t6500: use "ls -1" to snapshot ref database state
  t6001: avoid direct file system access

 t/t1405-main-ref-store.sh        | 18 ++++++++++++------
 t/t1410-reflog.sh                |  4 +++-
 t/t1503-rev-parse-verify.sh      |  2 +-
 t/t2402-worktree-list.sh         |  2 +-
 t/t3320-notes-merge-worktrees.sh | 12 +++++++-----
 t/t6001-rev-list-graft.sh        |  3 ++-
 t/t6050-replace.sh               |  2 +-
 t/t6120-describe.sh              |  6 ++++--
 t/t6500-gc.sh                    |  5 +++--
 t/t7064-wtstatus-pv2.sh          |  5 +----
 10 files changed, 35 insertions(+), 24 deletions(-)


base-commit: 940fe202adcbf9fa1825c648d97cbe1b90d26aec
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1052%2Fhanwen%2Ftest-fixes-v3-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1052/hanwen/test-fixes-v3-v3
Pull-Request: https://github.com/git/git/pull/1052

Range-diff vs v2:

  1:  d7616038d88 =  1:  ad9fd570024 t6050: use git-update-ref rather than filesystem access
  2:  cbd2128c331 =  2:  b03be9ede7f t1503: mark symlink test as REFFILES
  3:  cbf90c13e5b =  3:  b62c3175c4c t6120: use git-update-ref rather than filesystem access
  4:  7b98d092811 =  4:  22bff0b0791 t3320: use git-symbolic-ref rather than filesystem access
  5:  0047266de6b =  5:  ceca139bcfc t2402: use ref-store test helper to create broken symlink
  6:  eb2c53d19cf =  6:  add97ad8d0b t1405: use 'git reflog exists' to check reflog existence
  7:  05dead16f1c !  7:  5f9a01fd86a t1405: mark test for 'git pack-refs' as REFFILES
     @@ Metadata
       ## Commit message ##
          t1405: mark test for 'git pack-refs' as REFFILES
      
     -    The outcome of the pack-refs operation is not checked. This was apparently
     -    forgotten in the commit introducing this test: 16feb99d (Mar 26 2017, "t1405:
     -    some basic tests on main ref store").
     +    The tests verifies that "pack-refs" causes loose refs to be packed. As both
     +    loose and packed refs are concepts specific to the files backend, mark the test
     +    as REFFILES.
      
     -    I tried adding the obvious check this, but it seems to fail on the freebsd_12
     -    builder.
     +    Check the outcome of the pack-refs operation. This was apparently forgotten in
     +    the commit introducing this test: 16feb99d (Mar 26 2017, "t1405: some basic
     +    tests on main ref store").
      
          Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
      
     @@ t/t1405-main-ref-store.sh: export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
      +test_expect_success REFFILES 'pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE)' '
       	N=`find .git/refs -type f | wc -l` &&
       	test "$N" != 0 &&
     - 	$RUN pack-refs 3 &&
     +-	$RUN pack-refs 3 &&
     +-	N=`find .git/refs -type f | wc -l`
     ++	ALL_OR_PRUNE_FLAG=3 &&
     ++	$RUN pack-refs ${ALL_OR_PRUNE_FLAG} &&
     ++	N=`find .git/refs -type f` &&
     ++	test -z "$N"
     + '
     + 
     + test_expect_success 'create_symref(FOO, refs/heads/main)' '
  8:  f931a26de58 !  8:  45bc0772c64 t1410: mark test as REFFILES
     @@ Metadata
       ## Commit message ##
          t1410: mark test as REFFILES
      
     -    In reftable, one cannot take a lock on an individual ref.
     +    This test takes a lock on the target of a symref, and then verifies that it is
     +    possible to expire the symref's reflog. In reftable, one can only take a global
     +    lock (which would prevent the symref reflog from being expired altogether.)
      
          Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
      
  9:  3685ba90f60 =  9:  1326e99a1fd t7064: use update-ref -d to remove upstream branch
 10:  ccdbf3749fe ! 10:  55eb2094811 t6500: use "ls -1" to snapshot ref database state
     @@ Commit message
          By doing ls -1 .git/{reftable,refs/heads}, we can capture changes to both
          reftable and packed/loose ref storage.
      
     +    This relies on the fact that git-pack-refs (which we're looking for here)
     +    changes the number (loose/packed storage) and/or names (reftable) files used for
     +    ref storage.
     +
          Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
      
       ## t/t6500-gc.sh ##
 11:  81e46f95536 = 11:  4a3418b7954 t6001: avoid direct file system access

-- 
gitgitgadget

[PATCH v3 01/11] t6050: use git-update-ref rather than filesystem access

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t6050-replace.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index e33d512ec11..2500acc2ef8 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -132,7 +132,7 @@ tagger T A Gger <> 0 +0000
 EOF
 
 test_expect_success 'tag replaced commit' '
-     git mktag <tag.sig >.git/refs/tags/mytag
+     git update-ref refs/tags/mytag $(git mktag <tag.sig)
 '
 
 test_expect_success '"git fsck" works' '
-- 
gitgitgadget

[PATCH v3 02/11] t1503: mark symlink test as REFFILES

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t1503-rev-parse-verify.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh
index bf081023917..40958615ebb 100755
--- a/t/t1503-rev-parse-verify.sh
+++ b/t/t1503-rev-parse-verify.sh
@@ -142,7 +142,7 @@ test_expect_success 'main@{n} for various n' '
 	test_must_fail git rev-parse --verify main@{$Np1}
 '
 
-test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '
+test_expect_success SYMLINKS,REFFILES 'ref resolution not confused by broken symlinks' '
 	ln -s does-not-exist .git/refs/heads/broken &&
 	test_must_fail git rev-parse --verify broken
 '
-- 
gitgitgadget

[PATCH v3 03/11] t6120: use git-update-ref rather than filesystem access

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t6120-describe.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 1a501ee09e1..bae2419150b 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -107,7 +107,8 @@ test_expect_success 'describe --contains defaults to HEAD without commit-ish' '
 check_describe tags/A --all A^0
 
 test_expect_success 'renaming tag A to Q locally produces a warning' "
-	mv .git/refs/tags/A .git/refs/tags/Q &&
+	git update-ref refs/tags/Q $(git rev-parse refs/tags/A) &&
+	git update-ref -d refs/tags/A &&
 	git describe HEAD 2>err >out &&
 	cat >expected <<-\EOF &&
 	warning: tag 'Q' is externally known as 'A'
@@ -135,7 +136,8 @@ test_expect_success 'abbrev=0 will not break misplaced tag (2)' '
 '
 
 test_expect_success 'rename tag Q back to A' '
-	mv .git/refs/tags/Q .git/refs/tags/A
+	git update-ref refs/tags/A $(git rev-parse refs/tags/Q) &&
+	git update-ref -d refs/tags/Q
 '
 
 test_expect_success 'pack tag refs' 'git pack-refs'
-- 
gitgitgadget

[PATCH v3 04/11] t3320: use git-symbolic-ref rather than filesystem access

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t3320-notes-merge-worktrees.sh | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/t/t3320-notes-merge-worktrees.sh b/t/t3320-notes-merge-worktrees.sh
index 052516e6c6a..6b2d507f3e7 100755
--- a/t/t3320-notes-merge-worktrees.sh
+++ b/t/t3320-notes-merge-worktrees.sh
@@ -46,8 +46,9 @@ test_expect_success 'create some new worktrees' '
 test_expect_success 'merge z into y fails and sets NOTES_MERGE_REF' '
 	git config core.notesRef refs/notes/y &&
 	test_must_fail git notes merge z &&
-	echo "ref: refs/notes/y" >expect &&
-	test_cmp expect .git/NOTES_MERGE_REF
+	echo "refs/notes/y" >expect &&
+	git symbolic-ref NOTES_MERGE_REF >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success 'merge z into y while mid-merge in another workdir fails' '
@@ -57,7 +58,7 @@ test_expect_success 'merge z into y while mid-merge in another workdir fails' '
 		test_must_fail git notes merge z 2>err &&
 		test_i18ngrep "a notes merge into refs/notes/y is already in-progress at" err
 	) &&
-	test_path_is_missing .git/worktrees/worktree/NOTES_MERGE_REF
+	test_must_fail git -C worktree symbolic-ref NOTES_MERGE_REF
 '
 
 test_expect_success 'merge z into x while mid-merge on y succeeds' '
@@ -68,8 +69,9 @@ test_expect_success 'merge z into x while mid-merge on y succeeds' '
 		test_i18ngrep "Automatic notes merge failed" out &&
 		grep -v "A notes merge into refs/notes/x is already in-progress in" out
 	) &&
-	echo "ref: refs/notes/x" >expect &&
-	test_cmp expect .git/worktrees/worktree2/NOTES_MERGE_REF
+	echo "refs/notes/x" >expect &&
+	git -C worktree2 symbolic-ref NOTES_MERGE_REF >actual &&
+	test_cmp expect actual
 '
 
 test_done
-- 
gitgitgadget

[PATCH v3 05/11] t2402: use ref-store test helper to create broken symlink

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t2402-worktree-list.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh
index fedcefe8de3..4012bd67b04 100755
--- a/t/t2402-worktree-list.sh
+++ b/t/t2402-worktree-list.sh
@@ -230,7 +230,7 @@ test_expect_success 'broken main worktree still at the top' '
 		EOF
 		cd linked &&
 		echo "worktree $(pwd)" >expected &&
-		echo "ref: .broken" >../.git/HEAD &&
+		(cd ../ && test-tool ref-store main create-symref HEAD .broken ) &&
 		git worktree list --porcelain >out &&
 		head -n 3 out >actual &&
 		test_cmp ../expected actual &&
-- 
gitgitgadget

[PATCH v3 06/11] t1405: use 'git reflog exists' to check reflog existence

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

This fixes a test failure for reftable.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t1405-main-ref-store.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
index a237d9880ea..92b04873247 100755
--- a/t/t1405-main-ref-store.sh
+++ b/t/t1405-main-ref-store.sh
@@ -98,12 +98,12 @@ test_expect_success 'reflog_exists(HEAD)' '
 
 test_expect_success 'delete_reflog(HEAD)' '
 	$RUN delete-reflog HEAD &&
-	! test -f .git/logs/HEAD
+	test_must_fail git reflog exists HEAD
 '
 
 test_expect_success 'create-reflog(HEAD)' '
 	$RUN create-reflog HEAD 1 &&
-	test -f .git/logs/HEAD
+	git reflog exists HEAD
 '
 
 test_expect_success 'delete_ref(refs/heads/foo)' '
-- 
gitgitgadget

[PATCH v3 07/11] t1405: mark test for 'git pack-refs' as REFFILES

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

The tests verifies that "pack-refs" causes loose refs to be packed. As both
loose and packed refs are concepts specific to the files backend, mark the test
as REFFILES.

Check the outcome of the pack-refs operation. This was apparently forgotten in
the commit introducing this test: 16feb99d (Mar 26 2017, "t1405: some basic
tests on main ref store").

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t1405-main-ref-store.sh | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
index 92b04873247..49718b7ea7f 100755
--- a/t/t1405-main-ref-store.sh
+++ b/t/t1405-main-ref-store.sh
@@ -9,12 +9,18 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 RUN="test-tool ref-store main"
 
-test_expect_success 'pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE)' '
-	test_commit one &&
+
+test_expect_success 'setup' '
+	test_commit one
+'
+
+test_expect_success REFFILES 'pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE)' '
 	N=`find .git/refs -type f | wc -l` &&
 	test "$N" != 0 &&
-	$RUN pack-refs 3 &&
-	N=`find .git/refs -type f | wc -l`
+	ALL_OR_PRUNE_FLAG=3 &&
+	$RUN pack-refs ${ALL_OR_PRUNE_FLAG} &&
+	N=`find .git/refs -type f` &&
+	test -z "$N"
 '
 
 test_expect_success 'create_symref(FOO, refs/heads/main)' '
-- 
gitgitgadget

[PATCH v3 08/11] t1410: mark test as REFFILES

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

This test takes a lock on the target of a symref, and then verifies that it is
possible to expire the symref's reflog. In reftable, one can only take a global
lock (which would prevent the symref reflog from being expired altogether.)

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t1410-reflog.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
index 27b9080251a..d42f067ff8c 100755
--- a/t/t1410-reflog.sh
+++ b/t/t1410-reflog.sh
@@ -374,7 +374,9 @@ test_expect_failure 'reflog with non-commit entries displays all entries' '
 	test_line_count = 3 actual
 '
 
-test_expect_success 'reflog expire operates on symref not referrent' '
+# This test takes a lock on an individual ref; this is not supported in
+# reftable.
+test_expect_success REFFILES 'reflog expire operates on symref not referrent' '
 	git branch --create-reflog the_symref &&
 	git branch --create-reflog referrent &&
 	git update-ref referrent HEAD &&
-- 
gitgitgadget

[PATCH v3 09/11] t7064: use update-ref -d to remove upstream branch

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

The previous code tested this by writing $ZERO_OID explicitly in the packed-refs
file. This is a type of corruption that doesn't reflect realistic use-cases. In
addition, even the ref-store test-tool refuses to write invalid OIDs.
(update-ref interprets $ZERO_OID is deleting the ref).

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t7064-wtstatus-pv2.sh | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh
index 4613882caff..eeb0534163d 100755
--- a/t/t7064-wtstatus-pv2.sh
+++ b/t/t7064-wtstatus-pv2.sh
@@ -373,10 +373,7 @@ test_expect_success 'verify upstream fields in branch header' '
 
 		## Test upstream-gone case. Fake this by pointing
 		## origin/initial-branch at a non-existing commit.
-		OLD=$(git rev-parse origin/initial-branch) &&
-		NEW=$ZERO_OID &&
-		mv .git/packed-refs .git/old-packed-refs &&
-		sed "s/$OLD/$NEW/g" <.git/old-packed-refs >.git/packed-refs &&
+		git update-ref -d refs/remotes/origin/initial-branch &&
 
 		HUF=$(git rev-parse HEAD) &&
 
-- 
gitgitgadget

[PATCH v3 10/11] t6500: use "ls -1" to snapshot ref database state

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

By doing ls -1 .git/{reftable,refs/heads}, we can capture changes to both
reftable and packed/loose ref storage.

This relies on the fact that git-pack-refs (which we're looking for here)
changes the number (loose/packed storage) and/or names (reftable) files used for
ref storage.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t6500-gc.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index 10c7ae7f09c..c2021267f2c 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -241,7 +241,7 @@ test_expect_success 'background auto gc respects lock for all operations' '
 
 	# create a ref whose loose presence we can use to detect a pack-refs run
 	git update-ref refs/heads/should-be-loose HEAD &&
-	test_path_is_file .git/refs/heads/should-be-loose &&
+	(ls -1 .git/refs/heads .git/reftable >expect || true) &&
 
 	# now fake a concurrent gc that holds the lock; we can use our
 	# shell pid so that it looks valid.
@@ -258,7 +258,8 @@ test_expect_success 'background auto gc respects lock for all operations' '
 
 	# our gc should exit zero without doing anything
 	run_and_wait_for_auto_gc &&
-	test_path_is_file .git/refs/heads/should-be-loose
+	(ls -1 .git/refs/heads .git/reftable >actual || true) &&
+	test_cmp expect actual
 '
 
 # DO NOT leave a detached auto gc process running near the end of the
-- 
gitgitgadget

[PATCH v3 11/11] t6001: avoid direct file system access

[Han-Wen Nienhuys via GitGitGadget]

From: Han-Wen Nienhuys <hanwen@google.com>

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t6001-rev-list-graft.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/t/t6001-rev-list-graft.sh b/t/t6001-rev-list-graft.sh
index 90d93f77fa7..7294147334a 100755
--- a/t/t6001-rev-list-graft.sh
+++ b/t/t6001-rev-list-graft.sh
@@ -23,7 +23,8 @@ test_expect_success setup '
 	git commit -a -m "Third in one history." &&
 	A2=$(git rev-parse --verify HEAD) &&
 
-	rm -f .git/refs/heads/main .git/index &&
+	git update-ref -d refs/heads/main &&
+	rm -f .git/index &&
 
 	echo >fileA fileA again &&
 	echo >subdir/fileB fileB again &&
-- 
gitgitgadget

Re: [PATCH v2 00/11] Fix direct filesystem access in various test files.

[Han-Wen Nienhuys]

On Mon, Aug 2, 2021 at 6:28 PM Junio C Hamano <gitster@pobox.com> wrote:
> > By listing the files before and after, we can make sure that the ref
> > storage wasn't changed.
>
> Meaning that we rely on the fact that we never append to an existing
> file?  A comment here would have helped.

Added some more explanation.

I addressed your comments for the other changes (but forgot to update
the cover letter :-/ )

-- 
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--

Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich

Registergericht und -nummer: Hamburg, HRB 86891

Sitz der Gesellschaft: Hamburg

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado

Re: [PATCH v3 00/11] Fix direct filesystem access in various test files.

[Junio C Hamano]

"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:

> This fixes a few test failures in the reftable series.
>
> Han-Wen Nienhuys (11):
>   t6050: use git-update-ref rather than filesystem access
>   t1503: mark symlink test as REFFILES
>   t6120: use git-update-ref rather than filesystem access
>   t3320: use git-symbolic-ref rather than filesystem access
>   t2402: use ref-store test helper to create broken symlink
>   t1405: use 'git reflog exists' to check reflog existence
>   t1405: mark test for 'git pack-refs' as REFFILES
>   t1410: mark test as REFFILES
>   t7064: use update-ref -d to remove upstream branch
>   t6500: use "ls -1" to snapshot ref database state
>   t6001: avoid direct file system access

Thanks.

Unfortunately all of these are already in 'next', so I'll revert the
merge(s) to eliminate what has been in 'next' and queue this round
to 'seen', hoping that we can merge the new one back to 'next'
soonish.

Re: [PATCH v3 00/11] Fix direct filesystem access in various test files.

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> This fixes a few test failures in the reftable series.
>>
>> Han-Wen Nienhuys (11):
>>   t6050: use git-update-ref rather than filesystem access
>>   t1503: mark symlink test as REFFILES
>>   t6120: use git-update-ref rather than filesystem access
>>   t3320: use git-symbolic-ref rather than filesystem access
>>   t2402: use ref-store test helper to create broken symlink
>>   t1405: use 'git reflog exists' to check reflog existence
>>   t1405: mark test for 'git pack-refs' as REFFILES
>>   t1410: mark test as REFFILES
>>   t7064: use update-ref -d to remove upstream branch
>>   t6500: use "ls -1" to snapshot ref database state
>>   t6001: avoid direct file system access
>
> Thanks.
>
> Unfortunately all of these are already in 'next', so I'll revert the
> merge(s) to eliminate what has been in 'next' and queue this round
> to 'seen', hoping that we can merge the new one back to 'next'
> soonish.

Sorry, but I changed my mind.  On the day for -rc0, I do not have
time to deal with this right now, so I'll keep a bookmark on this
round for now, but will keep what has been on 'next' there for now.

Will revisit this new round of patches later.

Thanks.

Re: [PATCH v3 00/11] Fix direct filesystem access in various test files.

[Han-Wen Nienhuys]

On Mon, Aug 2, 2021 at 11:32 PM Junio C Hamano <gitster@pobox.com> wrote:
> > Unfortunately all of these are already in 'next', so I'll revert the
> > merge(s) to eliminate what has been in 'next' and queue this round
> > to 'seen', hoping that we can merge the new one back to 'next'
> > soonish.
>
> Sorry, but I changed my mind.  On the day for -rc0, I do not have
> time to deal with this right now, so I'll keep a bookmark on this
> round for now, but will keep what has been on 'next' there for now.
>
> Will revisit this new round of patches later.

No worries: I'll fix up whatever is necessary once the changes hit master.

-- 
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--

Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich

Registergericht und -nummer: Hamburg, HRB 86891

Sitz der Gesellschaft: Hamburg

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado