# [PATCH 0/6] Fix direct filesystem access in various test files. 46 messages from 2021-07-19 to 2021-08-03. Participants: Han-Wen Nienhuys via GitGitGadget, Junio C Hamano, Han-Wen Nienhuys. Thread: https://gitlist.dev/t/56128 ## Han-Wen Nienhuys via GitGitGadget, 2021-07-19 18:07 Subject: [PATCH 0/6] Fix direct filesystem access in various test files. Message-ID: URL: https://gitlist.dev/e/pull.1052.git.git.1626718050.gitgitgadget%40gmail.com ``` 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 ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-07-19 18:07 Subject: [PATCH 5/6] t3320: use git-symbolic-ref rather than filesystem access Message-ID: URL: https://gitlist.dev/e/cc400e9131dfc9a3df1b56401ab964eb5f1eb31d.1626718050.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys Signed-off-by: Han-Wen Nienhuys --- 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 ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-07-19 18:07 Subject: [PATCH 1/6] t6050: use git-update-ref rather than filesystem access Message-ID: <61a5c3718dfbba7aae6aee75b3030d7d744d2664.1626718050.git.gitgitgadget@gmail.com> URL: https://gitlist.dev/e/61a5c3718dfbba7aae6aee75b3030d7d744d2664.1626718050.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys Signed-off-by: Han-Wen Nienhuys --- 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 .git/refs/tags/mytag + git update-ref refs/tags/mytag $(git mktag URL: https://gitlist.dev/e/49994a0d154abd3e4cc7d8cf64044b8ab738819f.1626718050.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys Signed-off-by: Han-Wen Nienhuys --- 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 ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-07-19 18:07 Subject: [PATCH 6/6] t2402: use ref-store test helper to create broken symlink Message-ID: <4d83f70e13323c97bbae685c3ee6f5c94876cf78.1626718050.git.gitgitgadget@gmail.com> URL: https://gitlist.dev/e/4d83f70e13323c97bbae685c3ee6f5c94876cf78.1626718050.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys Signed-off-by: Han-Wen Nienhuys --- 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 ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-07-19 18:07 Subject: [PATCH 3/6] t6120: use git-update-ref rather than filesystem access Message-ID: <695921cacb68dede0deef3a197bc9cd89c9cf2e3.1626718050.git.gitgitgadget@gmail.com> URL: https://gitlist.dev/e/695921cacb68dede0deef3a197bc9cd89c9cf2e3.1626718050.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys Signed-off-by: Han-Wen Nienhuys --- 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 ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-07-19 18:07 Subject: [PATCH 4/6] t7509: use git-update-ref rather than filesystem access Message-ID: <94d7e144f54c423c865dcd475b042469f07f4221.1626718050.git.gitgitgadget@gmail.com> URL: https://gitlist.dev/e/94d7e144f54c423c865dcd475b042469f07f4221.1626718050.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys Signed-off-by: Han-Wen Nienhuys --- 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 " && 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 ``` ## Junio C Hamano, 2021-07-19 21:35 Subject: Re: [PATCH 4/6] t7509: use git-update-ref rather than filesystem access Message-ID: URL: https://gitlist.dev/e/xmqqv956j8c2.fsf%40gitster.g In-Reply-To: <94d7e144f54c423c865dcd475b042469f07f4221.1626718050.git.gitgitgadget@gmail.com> ``` "Han-Wen Nienhuys via GitGitGadget" writes: > From: Han-Wen Nienhuys > > Signed-off-by: Han-Wen Nienhuys > --- > 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 " && > 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 && ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-07-22 21:28 Subject: [PATCH v2 00/11] Fix direct filesystem access in various test files. Message-ID: URL: https://gitlist.dev/e/pull.1052.v2.git.git.1626989327.gitgitgadget%40gmail.com In-Reply-To: ``` 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 ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-07-22 21:28 Subject: [PATCH v2 01/11] t6050: use git-update-ref rather than filesystem access Message-ID: URL: https://gitlist.dev/e/d7616038d88f0f3bca9fec2a57abc2c9ca896e78.1626989327.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys Signed-off-by: Han-Wen Nienhuys --- 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 .git/refs/tags/mytag + git update-ref refs/tags/mytag $(git mktag URL: https://gitlist.dev/e/cbf90c13e5beb0849f4e112c6d4e8e0f8675b508.1626989327.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys Signed-off-by: Han-Wen Nienhuys --- 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 ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-07-22 21:28 Subject: [PATCH v2 02/11] t1503: mark symlink test as REFFILES Message-ID: URL: https://gitlist.dev/e/cbd2128c3313ea7627f16d3758ed402a280d3ea9.1626989327.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys Signed-off-by: Han-Wen Nienhuys --- 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 ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-07-22 21:28 Subject: [PATCH v2 05/11] t2402: use ref-store test helper to create broken symlink Message-ID: <0047266de6bc2f7f198cbc73cbb1e8adf2581535.1626989327.git.gitgitgadget@gmail.com> URL: https://gitlist.dev/e/0047266de6bc2f7f198cbc73cbb1e8adf2581535.1626989327.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys Signed-off-by: Han-Wen Nienhuys --- 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 ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-07-22 21:28 Subject: [PATCH v2 04/11] t3320: use git-symbolic-ref rather than filesystem access Message-ID: <7b98d092811c3b479b41e5ef3327d795d794fde2.1626989327.git.gitgitgadget@gmail.com> URL: https://gitlist.dev/e/7b98d092811c3b479b41e5ef3327d795d794fde2.1626989327.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys Signed-off-by: Han-Wen Nienhuys --- 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 ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-07-22 21:28 Subject: [PATCH v2 06/11] t1405: use 'git reflog exists' to check reflog existence Message-ID: URL: https://gitlist.dev/e/eb2c53d19cf90432ff4fd5af9ca6f4abc636cd7d.1626989327.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys This fixes a test failure for reftable. Signed-off-by: Han-Wen Nienhuys --- 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 ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-07-22 21:28 Subject: [PATCH v2 07/11] t1405: mark test for 'git pack-refs' as REFFILES Message-ID: <05dead16f1cbc8d967edf101e170049bbe121cb4.1626989327.git.gitgitgadget@gmail.com> URL: https://gitlist.dev/e/05dead16f1cbc8d967edf101e170049bbe121cb4.1626989327.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys 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 --- 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 ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-07-22 21:28 Subject: [PATCH v2 08/11] t1410: mark test as REFFILES Message-ID: URL: https://gitlist.dev/e/f931a26de58e3cc7bb6522fab47e934aca7dc776.1626989327.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys In reftable, one cannot take a lock on an individual ref. Signed-off-by: Han-Wen Nienhuys --- 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 ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-07-22 21:28 Subject: [PATCH v2 09/11] t7064: use update-ref -d to remove upstream branch Message-ID: <3685ba90f60ef95892a22c8549c9a3c0717d6662.1626989327.git.gitgitgadget@gmail.com> URL: https://gitlist.dev/e/3685ba90f60ef95892a22c8549c9a3c0717d6662.1626989327.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys 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 --- 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 ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-07-22 21:28 Subject: [PATCH v2 10/11] t6500: use "ls -1" to snapshot ref database state Message-ID: URL: https://gitlist.dev/e/ccdbf3749fe6444e2638a62fdb594836e67450af.1626989327.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys 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 --- 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 ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-07-22 21:28 Subject: [PATCH v2 11/11] t6001: avoid direct file system access Message-ID: <81e46f955363fee8576b378f22273436e28b3675.1626989327.git.gitgitgadget@gmail.com> URL: https://gitlist.dev/e/81e46f955363fee8576b378f22273436e28b3675.1626989327.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys Signed-off-by: Han-Wen Nienhuys --- 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 ``` ## Junio C Hamano, 2021-07-22 22:24 Subject: Re: [PATCH 0/6] Fix direct filesystem access in various test files. Message-ID: URL: https://gitlist.dev/e/xmqqk0li7zsk.fsf%40gitster.g In-Reply-To: ``` "Han-Wen Nienhuys via GitGitGadget" 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. ``` ## Junio C Hamano, 2021-07-23 17:15 Subject: Re: [PATCH v2 06/11] t1405: use 'git reflog exists' to check reflog existence Message-ID: URL: https://gitlist.dev/e/xmqqy29xne7m.fsf%40gitster.g In-Reply-To: ``` "Han-Wen Nienhuys via GitGitGadget" writes: > From: Han-Wen Nienhuys > > This fixes a test failure for reftable. > > Signed-off-by: Han-Wen Nienhuys > --- > 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. ``` ## Junio C Hamano, 2021-07-23 17:28 Subject: Re: [PATCH v2 07/11] t1405: mark test for 'git pack-refs' as REFFILES Message-ID: URL: https://gitlist.dev/e/xmqqsg05ndn9.fsf%40gitster.g In-Reply-To: <05dead16f1cbc8d967edf101e170049bbe121cb4.1626989327.git.gitgitgadget@gmail.com> ``` "Han-Wen Nienhuys via GitGitGadget" writes: > From: Han-Wen Nienhuys > > 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)' ' ``` ## Junio C Hamano, 2021-07-23 17:32 Subject: Re: [PATCH v2 08/11] t1410: mark test as REFFILES Message-ID: URL: https://gitlist.dev/e/xmqqo8atndgu.fsf%40gitster.g In-Reply-To: ``` "Han-Wen Nienhuys via GitGitGadget" writes: > From: Han-Wen Nienhuys > > In reftable, one cannot take a lock on an individual ref. > > Signed-off-by: Han-Wen Nienhuys > --- > 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 && ``` ## Junio C Hamano, 2021-07-23 17:37 Subject: Re: [PATCH v2 09/11] t7064: use update-ref -d to remove upstream branch Message-ID: URL: https://gitlist.dev/e/xmqqk0lhnd7x.fsf%40gitster.g In-Reply-To: <3685ba90f60ef95892a22c8549c9a3c0717d6662.1626989327.git.gitgitgadget@gmail.com> ``` "Han-Wen Nienhuys via GitGitGadget" 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) && ``` ## Junio C Hamano, 2021-07-23 17:40 Subject: Re: [PATCH v2 11/11] t6001: avoid direct file system access Message-ID: URL: https://gitlist.dev/e/xmqqfsw5nd3c.fsf%40gitster.g In-Reply-To: <81e46f955363fee8576b378f22273436e28b3675.1626989327.git.gitgitgadget@gmail.com> ``` "Han-Wen Nienhuys via GitGitGadget" writes: > From: Han-Wen Nienhuys > > Signed-off-by: Han-Wen Nienhuys > --- > 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. ``` ## Junio C Hamano, 2021-07-23 17:44 Subject: Re: [PATCH v2 00/11] Fix direct filesystem access in various test files. Message-ID: URL: https://gitlist.dev/e/xmqqbl6tncw7.fsf%40gitster.g In-Reply-To: ``` "Han-Wen Nienhuys via GitGitGadget" 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. ``` ## Han-Wen Nienhuys, 2021-08-02 13:38 Subject: Re: [PATCH v2 00/11] Fix direct filesystem access in various test files. Message-ID: URL: https://gitlist.dev/e/CAFQ2z_PjEuy2Ngr_i7c8rT6xb4w2HBEUARqwR7azZO4zor-w7g%40mail.gmail.com In-Reply-To: ``` On Fri, Jul 23, 2021 at 7:44 PM Junio C Hamano wrote: > > "Han-Wen Nienhuys via GitGitGadget" 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 ``` ## Junio C Hamano, 2021-08-02 16:27 Subject: Re: [PATCH v2 00/11] Fix direct filesystem access in various test files. Message-ID: URL: https://gitlist.dev/e/xmqqsfzrx14q.fsf%40gitster.g In-Reply-To: ``` Han-Wen Nienhuys writes: > On Fri, Jul 23, 2021 at 7:44 PM Junio C Hamano wrote: >> >> "Han-Wen Nienhuys via GitGitGadget" 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. ``` ## Junio C Hamano, 2021-08-02 16:28 Subject: Re: [PATCH v2 00/11] Fix direct filesystem access in various test files. Message-ID: URL: https://gitlist.dev/e/xmqqr1fbx13k.fsf%40gitster.g In-Reply-To: ``` Han-Wen Nienhuys writes: > On Fri, Jul 23, 2021 at 7:44 PM Junio C Hamano wrote: >> >> "Han-Wen Nienhuys via GitGitGadget" 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. ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-08-02 16:53 Subject: [PATCH v3 00/11] Fix direct filesystem access in various test files. Message-ID: URL: https://gitlist.dev/e/pull.1052.v3.git.git.1627923216.gitgitgadget%40gmail.com In-Reply-To: ``` 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 @@ 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 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 ## t/t6500-gc.sh ## 11: 81e46f95536 = 11: 4a3418b7954 t6001: avoid direct file system access -- gitgitgadget ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-08-02 16:53 Subject: [PATCH v3 01/11] t6050: use git-update-ref rather than filesystem access Message-ID: URL: https://gitlist.dev/e/ad9fd5700242a991edcd2801b85914da2fa54f44.1627923216.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys Signed-off-by: Han-Wen Nienhuys --- 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 .git/refs/tags/mytag + git update-ref refs/tags/mytag $(git mktag URL: https://gitlist.dev/e/b62c3175c4c86a3ae3a2a748dd1311306f1220ca.1627923216.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys Signed-off-by: Han-Wen Nienhuys --- 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 ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-08-02 16:53 Subject: [PATCH v3 02/11] t1503: mark symlink test as REFFILES Message-ID: URL: https://gitlist.dev/e/b03be9ede7f1462f35e5dcaf24a8c82cfc4d780f.1627923216.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys Signed-off-by: Han-Wen Nienhuys --- 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 ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-08-02 16:53 Subject: [PATCH v3 04/11] t3320: use git-symbolic-ref rather than filesystem access Message-ID: <22bff0b0791ffa402b6cfb79ecaa589bf0617d85.1627923216.git.gitgitgadget@gmail.com> URL: https://gitlist.dev/e/22bff0b0791ffa402b6cfb79ecaa589bf0617d85.1627923216.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys Signed-off-by: Han-Wen Nienhuys --- 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 ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-08-02 16:53 Subject: [PATCH v3 05/11] t2402: use ref-store test helper to create broken symlink Message-ID: URL: https://gitlist.dev/e/ceca139bcfcb695a09fc20c4aa0acb5aa85494f4.1627923216.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys Signed-off-by: Han-Wen Nienhuys --- 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 ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-08-02 16:53 Subject: [PATCH v3 07/11] t1405: mark test for 'git pack-refs' as REFFILES Message-ID: <5f9a01fd86aa52db48176ee347abdca394af5f14.1627923216.git.gitgitgadget@gmail.com> URL: https://gitlist.dev/e/5f9a01fd86aa52db48176ee347abdca394af5f14.1627923216.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys 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 --- 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 ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-08-02 16:53 Subject: [PATCH v3 08/11] t1410: mark test as REFFILES Message-ID: <45bc0772c64ee09a69122d3721271f827a21f909.1627923216.git.gitgitgadget@gmail.com> URL: https://gitlist.dev/e/45bc0772c64ee09a69122d3721271f827a21f909.1627923216.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys 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 --- 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 ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-08-02 16:53 Subject: [PATCH v3 09/11] t7064: use update-ref -d to remove upstream branch Message-ID: <1326e99a1fd1704e44b4f2d1a5731fe9da4d0a8a.1627923216.git.gitgitgadget@gmail.com> URL: https://gitlist.dev/e/1326e99a1fd1704e44b4f2d1a5731fe9da4d0a8a.1627923216.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys 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 --- 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 ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-08-02 16:53 Subject: [PATCH v3 06/11] t1405: use 'git reflog exists' to check reflog existence Message-ID: URL: https://gitlist.dev/e/add97ad8d0bd72178db3aede79c6d34331d3d1da.1627923216.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys This fixes a test failure for reftable. Signed-off-by: Han-Wen Nienhuys --- 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 ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-08-02 16:53 Subject: [PATCH v3 11/11] t6001: avoid direct file system access Message-ID: <4a3418b795451aabf6b79b7d7f09e09fa9e5bbc5.1627923216.git.gitgitgadget@gmail.com> URL: https://gitlist.dev/e/4a3418b795451aabf6b79b7d7f09e09fa9e5bbc5.1627923216.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys Signed-off-by: Han-Wen Nienhuys --- 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 ``` ## Han-Wen Nienhuys via GitGitGadget, 2021-08-02 16:53 Subject: [PATCH v3 10/11] t6500: use "ls -1" to snapshot ref database state Message-ID: <55eb2094811cfdc7a9387f1b582784f11a1d16eb.1627923216.git.gitgitgadget@gmail.com> URL: https://gitlist.dev/e/55eb2094811cfdc7a9387f1b582784f11a1d16eb.1627923216.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Han-Wen Nienhuys 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 --- 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 ``` ## Han-Wen Nienhuys, 2021-08-02 16:53 Subject: Re: [PATCH v2 00/11] Fix direct filesystem access in various test files. Message-ID: URL: https://gitlist.dev/e/CAFQ2z_P%3D54uk-9r8rTcSvXd5g5Mj8S1GjziszanC6iNdTp4q8w%40mail.gmail.com In-Reply-To: ``` On Mon, Aug 2, 2021 at 6:28 PM Junio C Hamano 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 ``` ## Junio C Hamano, 2021-08-02 20:07 Subject: Re: [PATCH v3 00/11] Fix direct filesystem access in various test files. Message-ID: URL: https://gitlist.dev/e/xmqqsfzrvcex.fsf%40gitster.g In-Reply-To: ``` "Han-Wen Nienhuys via GitGitGadget" 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. ``` ## Junio C Hamano, 2021-08-02 21:32 Subject: Re: [PATCH v3 00/11] Fix direct filesystem access in various test files. Message-ID: URL: https://gitlist.dev/e/xmqqo8afv8gn.fsf%40gitster.g In-Reply-To: ``` Junio C Hamano writes: > "Han-Wen Nienhuys via GitGitGadget" 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. ``` ## Han-Wen Nienhuys, 2021-08-03 09:37 Subject: Re: [PATCH v3 00/11] Fix direct filesystem access in various test files. Message-ID: URL: https://gitlist.dev/e/CAFQ2z_PALayJYY%3Dcjhae25jimvF_xevLdYQw-w_cZ4D3nBswmw%40mail.gmail.com In-Reply-To: ``` On Mon, Aug 2, 2021 at 11:32 PM Junio C Hamano 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 ```