Re: [PATCH v4 07/20] test-read-cache: print cache entries with --table

On 3/23/21 9:24 PM, Ævar Arnfjörð Bjarmason wrote:

On Tue, Mar 23 2021, Derrick Stolee via GitGitGadget 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 will 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.
However, this future need for full index information justifies the need
for this test helper over extending a user-facing feature, such as 'git
ls-files'.

Is that stat() information that's going to be essential to grab in the
same process that runs the "for (i = 0; i < istate->cache_nr; i++)"
for-loop, or stat() information that could be grabbed as:

    git ls-files -z --stage | some-program-that-stats-all-listed-blobs

The point is not to find the stat() data from disk, but to ensure that
the stat() data is correctly stored in the index (say, after converting
an existing index from another format). This pipe strategy does not
allow for that scenario.

It's not so much that I still disagree as I feel like I'm missing
something. I haven't gone through this topic with a fine toothed comb,
so ...

If and when these patches land and I'm using this nascent sparse
checkout support why wouldn't I want ls-files or another not-a-test-tool
to support extracting this new information that's in the index?

That's why I sent the RFC patches at
https://lore.kernel.org/git/20210317132814.30175-2-avarab@gmail.com/ (local) to
roll this functionality into ls-files.

And I recommend that you continue to pursue them as an independent
series, but I'm not going to incorporate them into this one. I'm
not going to distract from this internal data structure with changes
to user-facing commands until I think it's ready to use. As the design
document describes the plan, I don't expect this to be something I
will recommend to users until most of "Phase 3" is complete, making
the most common Git commands aware of a sparse index. (I expect to
fast-track a prototype to willing users that covers that functionality
while review continues on the mailing list.)

Making a change to a builtin is _forever_, and since the only
purpose right now is to expose the data in a test environment, I
don't want to adjust the builtin until either there is a real user
need or the feature has otherwise stabilized. If you want to take on
that responsibility, then please do.

Otherwise, I will need to eventually handle "git ls-files" being
sparse-aware when eventually removing 'command_requires_full_index',
(Phase 4) so that would be a good opportunity to adjust the
expectations.

Thanks,
-Stolee