[PATCH v2 0/9] t: reduce direct disk access to data structures
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Oct 24, 2023, 14:04 UTC
- Message-ID
- <cover.1698156169.git.ps@pks.im>
- In-Reply-To
- <cover.1697607222.git.ps@pks.im>
Hi,
this is the second version of my patch series that aims to reduce access to on-disk data structures in favor of using plumbing tools where possible.
Changes compared to v1:
- Patches 1, 3: I've dropped these patches that introduced and
started to use the test helper for reference existence. This has
been split out into a separate patch series now that instead
implements the logic as part of git-show-ref(1), see [1]. - Patch 4: I've made it more explicit that tests in t1450 are all
ran in detached HEAD mode via a new `orig_head` variable that is
set in the test setup. This variable is later used to reset HEAD
back to that original state. - Patch 4, 5: I've reordered some of the logic such that we schedule
`test_when_finished` before doing the actual mutation of the repo. - Patch 8: I've adopted the proposal of a `remove_replace_refs()`
helper function to clean up replace refs.- Now comes with a base commit. Unbelievable.
Thanks for all the feedback so far!
Patrick
Patrick Steinhardt (9): t: allow skipping expected object ID in `ref-store update-ref` t: convert tests to not write references via the filesystem t: convert tests to not access symrefs via the filesystem t: convert tests to not access reflog via the filesystem t1450: convert tests to remove worktrees via git-worktree(1) t4207: delete replace references via git-update-ref(1) t7300: assert exact states of repo t7900: assert the absence of refs via git-for-each-ref(1) t: mark several tests that assume the files backend with REFFILES
t/helper/test-ref-store.c | 11 ++++--- t/t1400-update-ref.sh | 50 ++++++++++++++++-------------- t/t1430-bad-ref-name.sh | 12 +++---- t/t1450-fsck.sh | 44 +++++++++++++------------- t/t2011-checkout-invalid-head.sh | 16 +++++----- t/t3200-branch.sh | 41 ++++++++++++------------ t/t3400-rebase.sh | 2 +- t/t3404-rebase-interactive.sh | 2 +- t/t4013-diff-various.sh | 2 +- t/t4202-log.sh | 2 +- t/t4207-log-decoration-colors.sh | 10 ++++-- t/t5526-fetch-submodules.sh | 2 +- t/t5605-clone-local.sh | 4 +-- t/t5702-protocol-v2.sh | 24 ++++++++++---- t/t7300-clean.sh | 23 ++++++++------ t/t7900-maintenance.sh | 3 +- t/t9133-git-svn-nested-git-repo.sh | 2 +- 17 files changed, 142 insertions(+), 108 deletions(-)
Range-diff against v1:
1: e947feb1c77 < -: ----------- t: add helpers to test for reference existence
2: 1f615d62f99 = 1: c868198f8c1 t: allow skipping expected object ID in `ref-store update-ref`
3: ac6a49c7c84 < -: ----------- t: convert tests to use helpers for reference existence
4: c79431c0bf1 ! 2: 4c0939d868e t: convert tests to not write references via the filesystem
@@ Commit message
test tool. The latter is required in some cases where safety checks of
git-update-ref(1) would otherwise reject a reference update.
+ While at it, refactor some of the tests to schedule the cleanup command
+ via `test_when_finished` before modifying the repository.
+
Signed-off-by: Patrick Steinhardt <ps@pks.im>
## t/t1400-update-ref.sh ##
@@ t/t1400-update-ref.sh: test_expect_success "delete $m without oldvalue verificat
test_expect_success "create $m (by HEAD)" '
@@ t/t1400-update-ref.sh: test_expect_success 'delete symref without dereference when the referred ref is
+ '
test_expect_success 'update-ref -d is not confused by self-reference' '
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF refs/heads/self" &&
git symbolic-ref refs/heads/self refs/heads/self &&
- test_when_finished "rm -f .git/refs/heads/self" &&
-+ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF refs/heads/self" &&
test_path_is_file .git/refs/heads/self &&
test_must_fail git update-ref -d refs/heads/self &&
test_path_is_file .git/refs/heads/self
-@@ t/t1400-update-ref.sh: test_expect_success 'update-ref -d is not confused by self-reference' '
+ '
test_expect_success 'update-ref --no-deref -d can delete self-reference' '
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF refs/heads/self" &&
git symbolic-ref refs/heads/self refs/heads/self &&
- test_when_finished "rm -f .git/refs/heads/self" &&
-+ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF refs/heads/self" &&
test_path_is_file .git/refs/heads/self &&
git update-ref --no-deref -d refs/heads/self &&
test_must_fail git show-ref --verify -q refs/heads/self
@@ t/t1400-update-ref.sh: test_expect_success 'Query "main@{2005-05-28}" (past end
'
-rm -f .git/$m .git/logs/$m expect
++rm -f expect
+git update-ref -d $m
test_expect_success 'creating initial files' '
test_when_finished rm -f M &&
## t/t1450-fsck.sh ##
+@@ t/t1450-fsck.sh: test_expect_success setup '
+ git config --unset i18n.commitencoding &&
+ git checkout HEAD^0 &&
+ test_commit B fileB two &&
++ orig_head=$(git rev-parse HEAD) &&
+ git tag -d A B &&
+ git reflog expire --expire=now --all
+ '
@@ t/t1450-fsck.sh: test_expect_success 'zlib corrupt loose object output ' '
'
test_expect_success 'branch pointing to non-commit' '
- git rev-parse HEAD^{tree} >.git/refs/heads/invalid &&
+ tree_oid=$(git rev-parse --verify HEAD^{tree}) &&
-+ test-tool ref-store main update-ref msg refs/heads/invalid $tree_oid $ZERO_OID REF_SKIP_OID_VERIFICATION &&
test_when_finished "git update-ref -d refs/heads/invalid" &&
++ test-tool ref-store main update-ref msg refs/heads/invalid $tree_oid $ZERO_OID REF_SKIP_OID_VERIFICATION &&
test_must_fail git fsck 2>out &&
test_i18ngrep "not a commit" out
'
@@ t/t1450-fsck.sh: test_expect_success 'zlib corrupt loose object output ' '
test_expect_success 'HEAD link pointing at a funny object' '
- test_when_finished "mv .git/SAVED_HEAD .git/HEAD" &&
- mv .git/HEAD .git/SAVED_HEAD &&
-+ saved_head=$(git rev-parse --verify HEAD) &&
-+ test_when_finished "git update-ref HEAD ${saved_head}" &&
++ test_when_finished "git update-ref HEAD $orig_head" &&
echo $ZERO_OID >.git/HEAD &&
# avoid corrupt/broken HEAD from interfering with repo discovery
test_must_fail env GIT_DIR=.git git fsck 2>out &&
@@ t/t1450-fsck.sh: test_expect_success 'HEAD link pointing at a funny object' '
test_expect_success 'HEAD link pointing at a funny place' '
- test_when_finished "mv .git/SAVED_HEAD .git/HEAD" &&
- mv .git/HEAD .git/SAVED_HEAD &&
-+ saved_head=$(git rev-parse --verify HEAD) &&
-+ test_when_finished "git update-ref --no-deref HEAD ${saved_head}" &&
++ test_when_finished "git update-ref --no-deref HEAD $orig_head" &&
echo "ref: refs/funny/place" >.git/HEAD &&
# avoid corrupt/broken HEAD from interfering with repo discovery
test_must_fail env GIT_DIR=.git git fsck 2>out &&
@@ t/t1450-fsck.sh: test_expect_success 'HEAD link pointing at a funny place' '
test_expect_success 'HEAD link pointing at a funny object (from different wt)' '
- test_when_finished "mv .git/SAVED_HEAD .git/HEAD" &&
-+ saved_head=$(git rev-parse --verify HEAD) &&
-+ test_when_finished "git update-ref HEAD $saved_head" &&
++ test_when_finished "git update-ref HEAD $orig_head" &&
test_when_finished "rm -rf .git/worktrees wt" &&
git worktree add wt &&
- mv .git/HEAD .git/SAVED_HEAD &&
5: 1ac120368c6 ! 3: 048583ed2c3 t: convert tests to not access symrefs via the filesystem
@@ Commit message
## t/t1400-update-ref.sh ##
@@ t/t1400-update-ref.sh: test_expect_success 'delete symref without dereference when the referred ref is
test_expect_success 'update-ref -d is not confused by self-reference' '
- git symbolic-ref refs/heads/self refs/heads/self &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF refs/heads/self" &&
+ git symbolic-ref refs/heads/self refs/heads/self &&
- test_path_is_file .git/refs/heads/self &&
+ git symbolic-ref --no-recurse refs/heads/self &&
test_must_fail git update-ref -d refs/heads/self &&
@@ t/t1400-update-ref.sh: test_expect_success 'delete symref without dereference wh
'
test_expect_success 'update-ref --no-deref -d can delete self-reference' '
- git symbolic-ref refs/heads/self refs/heads/self &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF refs/heads/self" &&
+ git symbolic-ref refs/heads/self refs/heads/self &&
- test_path_is_file .git/refs/heads/self &&
+ git symbolic-ref --no-recurse refs/heads/self &&
git update-ref --no-deref -d refs/heads/self &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref -d can delete broken na
- printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref &&
+ test-tool ref-store main create-symref refs/heads/broken...symref refs/heads/main &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
- test_ref_exists refs/heads/broken...symref &&
git update-ref --no-deref -d refs/heads/broken...symref >output 2>error &&
+ test_path_is_missing .git/refs/heads/broken...symref &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref --no-deref -d can delete symref with broken name
'
@@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref --no-deref -d can delet
- printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref &&
+ test-tool ref-store main create-symref refs/heads/broken...symref refs/heads/main &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
- test_ref_exists refs/heads/broken...symref &&
git branch -d broken...symref >output 2>error &&
+ test_path_is_missing .git/refs/heads/broken...symref &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -d can delete symref with broken name' '
'
@@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -d can delete symref with b
- printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref &&
+ test-tool ref-store main create-symref refs/heads/broken...symref refs/heads/idonotexist &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
- test_ref_exists refs/heads/broken...symref &&
git update-ref --no-deref -d refs/heads/broken...symref >output 2>error &&
+ test_path_is_missing .git/refs/heads/broken...symref &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref --no-deref -d can delete dangling symref with br
'
@@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref --no-deref -d can delet
- printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref &&
+ test-tool ref-store main create-symref refs/heads/broken...symref refs/heads/idonotexist &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
- test_ref_exists refs/heads/broken...symref &&
git branch -d broken...symref >output 2>error &&
+ test_path_is_missing .git/refs/heads/broken...symref &&
## t/t1450-fsck.sh ##
@@ t/t1450-fsck.sh: test_expect_success 'HEAD link pointing at a funny object' '
+
test_expect_success 'HEAD link pointing at a funny place' '
- saved_head=$(git rev-parse --verify HEAD) &&
- test_when_finished "git update-ref --no-deref HEAD ${saved_head}" &&
+ test_when_finished "git update-ref --no-deref HEAD $orig_head" &&
- echo "ref: refs/funny/place" >.git/HEAD &&
+ test-tool ref-store main create-symref HEAD refs/funny/place &&
# avoid corrupt/broken HEAD from interfering with repo discovery
@@ t/t3200-branch.sh: test_expect_success 'git branch -M should leave orphaned HEAD
+ git symbolic-ref HEAD >expect &&
+ echo refs/heads/lonely >actual &&
+ test_cmp expect actual &&
- test_ref_missing refs/head/lonely &&
+ test_path_is_missing .git/refs/head/lonely &&
git branch -M main mistress &&
- grep lonely .git/HEAD
+ git symbolic-ref HEAD >expect &&
@@ t/t3200-branch.sh: test_expect_success 'deleting a symref' '
+ git symbolic-ref --no-recurse refs/heads/dangling-symref &&
echo "Deleted branch dangling-symref (was nowhere)." >expect &&
git branch -d dangling-symref >actual &&
- test_ref_missing refs/heads/dangling-symref &&
+ test_path_is_missing .git/refs/heads/dangling-symref &&
## t/t4013-diff-various.sh ##
@@ t/t4013-diff-various.sh: test_expect_success 'log -S requires an argument' '
6: eaac658bbfd ! 4: 5e7937e7904 t: convert tests to not access reflog via the filesystem
@@ t/t3200-branch.sh: test_expect_success 'git branch HEAD should fail' '
test_expect_success 'git branch --create-reflog d/e/f should create a branch and a log' '
GIT_COMMITTER_DATE="2005-05-26 23:30" \
git -c core.logallrefupdates=false branch --create-reflog d/e/f &&
- test_ref_exists refs/heads/d/e/f &&
+ test_path_is_file .git/refs/heads/d/e/f &&
- test_path_is_file .git/logs/refs/heads/d/e/f &&
- test_cmp expect .git/logs/refs/heads/d/e/f
+ git reflog show --no-abbrev-commit refs/heads/d/e/f >actual &&
@@ t/t3200-branch.sh: test_expect_success '--set-upstream-to notices an error to se
test_expect_success 'git checkout -b g/h/i -l should create a branch and a log' '
GIT_COMMITTER_DATE="2005-05-26 23:30" \
git checkout -b g/h/i -l main &&
- test_ref_exists refs/heads/g/h/i &&
+ test_path_is_file .git/refs/heads/g/h/i &&
- test_path_is_file .git/logs/refs/heads/g/h/i &&
- test_cmp expect .git/logs/refs/heads/g/h/i
+ git reflog show --no-abbrev-commit refs/heads/g/h/i >actual &&
7: 3dc65a80074 ! 5: 089565a358e t1450: convert tests to remove worktrees via git-worktree(1)
@@ Commit message
## t/t1450-fsck.sh ##
@@ t/t1450-fsck.sh: test_expect_success 'HEAD link pointing at a funny place' '
+
test_expect_success 'HEAD link pointing at a funny object (from different wt)' '
- saved_head=$(git rev-parse --verify HEAD) &&
- test_when_finished "git update-ref HEAD $saved_head" &&
+ test_when_finished "git update-ref HEAD $orig_head" &&
- test_when_finished "rm -rf .git/worktrees wt" &&
+ test_when_finished "git worktree remove -f wt" &&
git worktree add wt &&
8: c4d09e3e5db ! 6: cb738888ed7 t4207: delete replace references via git-update-ref(1)
@@ Commit message
## t/t4207-log-decoration-colors.sh ##
@@ t/t4207-log-decoration-colors.sh: ${c_tag}tag: ${c_reset}${c_tag}A${c_reset}${c_commit})${c_reset} A
+ cmp_filtered_decorations
'
++remove_replace_refs () {
++ git for-each-ref 'refs/replace*/**' --format='delete %(refname)' >in &&
++ git update-ref --stdin <in &&
++ rm in
++}
++
test_expect_success 'test coloring with replace-objects' '
- test_when_finished rm -rf .git/refs/replace* &&
-+ test_when_finished "git for-each-ref refs/replace*/** --format=${SQ}delete %(refname)${SQ} | git update-ref --stdin" &&
++ test_when_finished remove_replace_refs &&
test_commit C &&
test_commit D &&
@@ t/t4207-log-decoration-colors.sh: EOF
test_expect_success 'test coloring with grafted commit' '
- test_when_finished rm -rf .git/refs/replace* &&
-+ test_when_finished "git for-each-ref refs/replace*/** --format=${SQ}delete %(refname)${SQ} | git update-ref --stdin" &&
++ test_when_finished remove_replace_refs &&
git replace --graft HEAD HEAD~2 &&
9: 153b5b199c8 = 7: e730e011de4 t7300: assert exact states of repo
10: b99d98b00a3 = 8: a1bdea52397 t7900: assert the absence of refs via git-for-each-ref(1)
11: 67cb282a414 ! 9: 497e43ae5c3 t: mark several tests that assume the files backend with REFFILES
@@ t/t1450-fsck.sh: test_expect_success 'branch pointing to non-commit' '
-test_expect_success 'HEAD link pointing at a funny object' '
+test_expect_success REFFILES 'HEAD link pointing at a funny object' '
- saved_head=$(git rev-parse --verify HEAD) &&
- test_when_finished "git update-ref HEAD ${saved_head}" &&
+ test_when_finished "git update-ref HEAD $orig_head" &&
echo $ZERO_OID >.git/HEAD &&
+ # avoid corrupt/broken HEAD from interfering with repo discovery
@@ t/t1450-fsck.sh: test_expect_success 'HEAD link pointing at a funny place' '
test_i18ngrep "HEAD points to something strange" out
'
-test_expect_success 'HEAD link pointing at a funny object (from different wt)' '
+test_expect_success REFFILES 'HEAD link pointing at a funny object (from different wt)' '
- saved_head=$(git rev-parse --verify HEAD) &&
- test_when_finished "git update-ref HEAD $saved_head" &&
+ test_when_finished "git update-ref HEAD $orig_head" &&
test_when_finished "git worktree remove -f wt" &&
+ git worktree add wt &&
@@ t/t1450-fsck.sh: test_expect_success 'HEAD link pointing at a funny object (from different wt)' '
test_i18ngrep "main-worktree/HEAD: detached HEAD points" out
'
@@ t/t2011-checkout-invalid-head.sh: test_expect_success 'create ref directory/file
## t/t3200-branch.sh ##
@@ t/t3200-branch.sh: test_expect_success 'git branch --help should not have created a bogus branch' '
- test_ref_missing refs/heads/--help
+ test_path_is_missing .git/refs/heads/--help
'
-test_expect_success 'branch -h in broken repository' '
@@ t/t3200-branch.sh: test_expect_success 'git branch -M baz bam should succeed whe
git worktree add -f bazdir2 baz &&
touch .git/worktrees/bazdir1/HEAD.lock &&
@@ t/t3200-branch.sh: test_expect_success 'renaming a symref is not allowed' '
- test_ref_missing refs/heads/new-topic
+ test_path_is_missing .git/refs/heads/new-topic
'
-test_expect_success SYMLINKS 'git branch -m u v should fail when the reflog for u is a symlink' 'base-commit: a9ecda2788e229afc9b611acaa26d0d9d4da53ed
-- 2.42.0