[PATCH v2 0/8] More index cleanups

From: "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: newren@gmail.com, Derrick Stolee <stolee@gmail.com>,
	gitster@pobox.com, Derrick Stolee <derrickstolee@github.com>
Subject: [PATCH v2 0/8] More index cleanups
Date: Fri, 22 Jan 2021 13:03:51 +0000	[thread overview]
Message-ID: <pull.839.v2.git.1611320639.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.839.git.1611161639.gitgitgadget@gmail.com>

This is based on ds/cache-tree-basics.

Here are a few more cleanups that are vaguely related to the index. I
discovered these while preparing my sparse-index RFC that I intend to send
early next week.

The biggest patch is the final one, which creates a test script for
comparing sparse-checkouts to full checkouts. There are some commands that
do not behave similarly. This script will be the backbone of my testing
strategy for the sparse-index by adding a new mode to compare
sparse-checkouts with the two index types (full and sparse).

UPDATES IN V2
=============

 * Fixed duplicated test in t1092.

 * Changed the implementation of 'test_region' after I discovered the
   negation doesn't work correctly. (I updated the test to use what was in
   t0500-progress-display.sh at the last minute before v1, but that
   implementation was wrong.) The use of it in t0500-progress-display.sh was
   incorrect, as well.

 * Updated commit messages to be more informative and have fewer typos.

 * I dropped the patch that placed the sparse-checkout patterns in struct
   index_state. I'll re-introduce that in time for the actual use of the
   member.

Thanks, -Stolee

Derrick Stolee (8):
  cache-tree: clean up cache_tree_update()
  cache-tree: extract subtree_pos()
  fsmonitor: de-duplicate BUG()s around dirty bits
  repository: add repo reference to index_state
  name-hash: use trace2 regions for init
  sparse-checkout: load sparse-checkout patterns
  test-lib: test_region looks for trace2 regions
  t1092: test interesting sparse-checkout scenarios

 builtin/sparse-checkout.c                |   5 -
 cache-tree.c                             |  20 +-
 cache-tree.h                             |   2 +
 cache.h                                  |   1 +
 dir.c                                    |  17 ++
 dir.h                                    |   2 +
 fsmonitor.c                              |  27 +-
 name-hash.c                              |   3 +
 repository.c                             |   4 +
 t/t0500-progress-display.sh              |   3 +-
 t/t1092-sparse-checkout-compatibility.sh | 298 +++++++++++++++++++++++
 t/test-lib-functions.sh                  |  40 +++
 unpack-trees.c                           |   6 +-
 13 files changed, 394 insertions(+), 34 deletions(-)
 create mode 100755 t/t1092-sparse-checkout-compatibility.sh

base-commit: a4b6d202caad83c6dc29abe9b17e53a1b3fb54a0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-839%2Fderrickstolee%2Fmore-index-cleanups-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-839/derrickstolee/more-index-cleanups-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/839

Range-diff vs v1:

  1:  0bccfd34ae5 !  1:  f9dccaed0ac cache-tree: clean up cache_tree_update()
     @@ Commit message
          cache-tree: clean up cache_tree_update()

          Make the method safer by allocating a cache_tree member for the given
     -    index_state if it is not already present.
     +    index_state if it is not already present. This is preferrable to a
     +    BUG() statement or returning with an error because future callers will
     +    want to populate an empty cache-tree using this method.

          Also drop local variables that are used exactly once and can be found
          directly from the 'istate' parameter.
  2:  a6f2406a795 =  2:  84323e04d08 cache-tree: extract subtree_pos()
  3:  838922de2e9 =  3:  31095f9aa0e fsmonitor: de-duplicate BUG()s around dirty bits
  4:  d4ff0468fc0 !  4:  a0d89d7a973 repository: add repo reference to index_state
     @@ Metadata
       ## Commit message ##
          repository: add repo reference to index_state

     -    It will be helpful to add behavior to index opertations that might
     +    It will be helpful to add behavior to index operations that might
          trigger an object lookup. Since each index belongs to a specific
          repository, add a 'repo' pointer to struct index_state that allows
          access to this repository.
  5:  3ba4b35f09c =  5:  bc092f5c703 name-hash: use trace2 regions for init
  6:  64358ec7ea2 =  6:  04d1daf7222 sparse-checkout: load sparse-checkout patterns
  7:  91344f5108c <  -:  ----------- sparse-checkout: hold pattern list in index
  8:  8326a9b5320 !  7:  8832ce84623 test-lib: test_region looks for trace2 regions
     @@ Commit message
          entered and left in a given trace2 event log.

          There is one existing test (t0500-progress-display.sh) that performs
     -    this check already, so use the helper function instead. More uses will
     -    be added in a later change.
     +    this check already, so use the helper function instead. Note that this
     +    changes the expectations slightly. The old test (incorrectly) used two
     +    patterns for the 'grep' invocation, but this performs an OR of the
     +    patterns, not an AND. This means that as long as one region_enter event
     +    was logged, the test would succeed, even if it was not due to the
     +    progress category.
     +
     +    More uses will be added in a later change.

          t6423-merge-rename-directories.sh also greps for region_enter lines, but
          it verifies the number of such lines, which is not the same as an
     @@ t/t0500-progress-display.sh: test_expect_success 'progress generates traces' '
       	# t0212/parse_events.perl intentionally omits regions and data.
      -	grep -e "region_enter" -e "\"category\":\"progress\"" trace.event &&
      -	grep -e "region_leave" -e "\"category\":\"progress\"" trace.event &&
     -+	test_region category progress trace.event &&
     ++	test_region progress "Working hard" trace.event &&
       	grep "\"key\":\"total_objects\",\"value\":\"40\"" trace.event &&
       	grep "\"key\":\"total_bytes\",\"value\":\"409600\"" trace.event
       '
     @@ t/test-lib-functions.sh: test_subcommand () {
      +		shift
      +	fi
      +
     -+	grep -e "region_enter" -e "\"category\":\"$1\",\"label\":\"$2\"" "$3"
     ++	grep -e "\"region_enter\".*\"category\":\"$1\",\"label\":\"$2\"" "$3"
      +	exitcode=$?
      +
      +	if test $exitcode != $expect_exit
     @@ t/test-lib-functions.sh: test_subcommand () {
      +		return 1
      +	fi
      +
     -+	grep -e "region_leave" -e "\"category\":\"$1\",\"label\":\"$2\"" "$3"
     ++	grep -e "\"region_leave\".*\"category\":\"$1\",\"label\":\"$2\"" "$3"
      +	exitcode=$?
      +
      +	if test $exitcode != $expect_exit
  9:  555e210dc03 !  8:  984458007ed t1092: test interesting sparse-checkout scenarios
     @@ Commit message
          These also document some behaviors that differ from a full checkout, and
          possibly in a way that is not intended.

     +    The test is designed to be run with "--run=1,X" where 'X' is an
     +    interesting test case. Each test uses 'init_repos' to reset the full and
     +    sparse copies of the initial-repo that is created by the first test
     +    case. This also makes it possible to have test cases leave the working
     +    directory or index in unusual states without disturbing later cases.
     +
          Signed-off-by: Derrick Stolee <dstolee@microsoft.com>

       ## t/t1092-sparse-checkout-compatibility.sh (new) ##
     @@ t/t1092-sparse-checkout-compatibility.sh (new)
      +	init_repos &&
      +
      +	write_script edit-contents <<-\EOF &&
     -+	echo text >>README.md
     -+	EOF
     -+	run_on_all "../edit-contents" &&
     -+
     -+	test_all_match git add README.md &&
     -+	test_all_match git status --porcelain=v2 &&
     -+	test_all_match git commit -m "Add README.md" &&
     -+
     -+	test_all_match git checkout HEAD~1 &&
     -+	test_all_match git checkout - &&
     -+
     -+	run_on_all "../edit-contents" &&
     -+
     -+	test_all_match git add -A &&
     -+	test_all_match git status --porcelain=v2 &&
     -+	test_all_match git commit -m "Extend README.md" &&
     -+
     -+	test_all_match git checkout HEAD~1 &&
     -+	test_all_match git checkout -
     -+'
     -+
     -+test_expect_success 'add, commit, checkout' '
     -+	init_repos &&
     -+
     -+	write_script edit-contents <<-\EOF &&
      +	echo text >>$1
      +	EOF
      +	run_on_all "../edit-contents README.md" &&
     @@ t/t1092-sparse-checkout-compatibility.sh (new)
      +	do
      +		test_all_match git checkout rename-base &&
      +		test_all_match git checkout $branch -- .&&
     -+		test_all_match git diff --staged &&
     ++		test_all_match git diff --staged --no-renames &&
      +		test_all_match git diff --staged --find-renames || return 1
      +	done
      +'

-- 
gitgitgadget