Re: [RFC/PATCH 0/5] Re: [PATCH v3 07/20] test-read-cache: print cache entries with --table

On Wed, Mar 17, 2021 at 6:28 AM Ævar Arnfjörð Bjarmason
[off-list ref] wrote:

quoted

From: Derrick Stolee <redacted>

This table is helpful for discovering data in the index to ensure it is
being written correctly, especially as we build and test the
sparse-index. This table includes an output format similar to 'git
ls-tree', but should not be compared to that directly. The biggest
reasons are that 'git ls-tree' includes a tree entry for every
subdirectory, even those that would not appear as a sparse directory in
a sparse-index. Further, 'git ls-tree' does not use a trailing directory
separator for its tree rows.

This does not print the stat() information for the blobs. That could be
added in a future change with another option. The tests that are added
in the next few changes care only about the object types and IDs.

To make the option parsing slightly more robust, wrap the string
comparisons in a loop adapted from test-dir-iterator.c.

Care must be taken with the final check for the 'cnt' variable. We
continue the expectation that the numerical value is the final argument.

Signed-off-by: Derrick Stolee <redacted>
---
 t/helper/test-read-cache.c | 55 +++++++++++++++++++++++++++++++-------
 1 file changed, 45 insertions(+), 10 deletions(-)

diff --git a/t/helper/test-read-cache.c b/t/helper/test-read-cache.c
index 244977a29bdf..6cfd8f2de71c 100644
--- a/t/helper/test-read-cache.c
+++ b/t/helper/test-read-cache.c

@@ -1,36 +1,71 @@
 #include "test-tool.h"
 #include "cache.h"
 #include "config.h"
+#include "blob.h"
+#include "commit.h"
+#include "tree.h"
+
+static void print_cache_entry(struct cache_entry *ce)
+{
+     const char *type;
+     printf("%06o ", ce->ce_mode & 0177777);
+
+     if (S_ISSPARSEDIR(ce->ce_mode))
+             type = tree_type;
+     else if (S_ISGITLINK(ce->ce_mode))
+             type = commit_type;
+     else
+             type = blob_type;
+
+     printf("%s %s\t%s\n",
+            type,
+            oid_to_hex(&ce->oid),
+            ce->name);
+}
+

So we have a test tool that's mostly ls-files but mocks the output
ls-tree would emit, won't these tests eventually care about what stage
things are in?

What follows is an RFC series on top that's the result of me wondering
why if we're adding new index constructs we aren't updating our
plumbing to emit that data, can we just add this to ls-files and drop
this test helper?

Turns out: Yes we can.

I like the idea of having ls-files be usable to show the entries that
are in the index; that seems great to me.  I very much dislike the
--sparse flag to ls-files, as noted on that commit.

Also, as a minor point, the first two patches seemed a bit confusing
to me.  The first commit said that it was there solely to make "the
next commit" easier, and the second was worded as just making the next
patch easier, which made me wonder if the wording in the first commit
message was referring to 3/5 when it said "the next commit".  Both of
the first two commits were so tiny that if they are both prep for 3/5,
maybe it makes sense to combine them (together or both to 3/5)?  If
not, maybe the commit messages could be cleaned up or clarified a bit?

Ævar Arnfjörð Bjarmason (5):
  ls-files: defer read_index() after parse_options() etc.
  ls-files: make "mode" in show_ce() loop a variable
  ls-files: add and use a new --sparse option
  test-tool read-cache: --table is redundant to ls-files
  test-tool: split up test-tool read-cache

 Documentation/git-ls-files.txt           |  4 ++
 Makefile                                 |  3 +-
 builtin/ls-files.c                       | 29 +++++++--
 t/helper/test-read-cache-again.c         | 31 +++++++++
 t/helper/test-read-cache-perf.c          | 21 ++++++
 t/helper/test-read-cache.c               | 82 ------------------------
 t/helper/test-tool.c                     |  3 +-
 t/helper/test-tool.h                     |  3 +-
 t/perf/p0002-read-cache.sh               |  2 +-
 t/t1091-sparse-checkout-builtin.sh       |  9 +--
 t/t1092-sparse-checkout-compatibility.sh | 57 ++++++++++------
 t/t7519-status-fsmonitor.sh              |  2 +-
 12 files changed, 131 insertions(+), 115 deletions(-)
 create mode 100644 t/helper/test-read-cache-again.c
 create mode 100644 t/helper/test-read-cache-perf.c
 delete mode 100644 t/helper/test-read-cache.c

--
2.31.0.260.g719c683c1d