[PATCH v2 0/9] t: reduce direct disk access to data structures

[Patrick Steinhardt <ps@pks.im>]

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

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


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