Re: [PATCH 4/4] object-name: diagnose trees in index properly

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 12, 2022, 06:32 UTC
Message-ID: <xmqqlewaerqd.fsf@gitster.g>
In-Reply-To: <99c09ccc2406e4f54c620bd7fb2d1205386e23a6.1649349442.git.gitgitgadget@gmail.com>

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 18 quoted lines

> From: Derrick Stolee <derrickstolee@github.com>
>
> When running 'git show :<path>' where '<path>' is a directory, then
> there is a subtle difference between a full checkout and a sparse
> checkout. The error message from diagnose_invalid_index_path() reports
> whether the path is on disk or not. The full checkout will have the
> directory on disk, but the path will not be in the index. The sparse
> checokut could have the directory not exist, specifically when that
> directory is outside of the sparse-checkout cone.
>
> In the case of a sparse index, we have yet another state: the path can
> be a sparse directory in the index. In this case, the error message from
> diagnose_invalid_index_path() would erroneously say "path '<path>' is in
> the index, but not at stage 0", which is false.
>
> Add special casing around sparse directory entries so we get to the
> correct error message. This requires two checks in order to get parity
> with the normal sparse-checkout case.

That all makes sense, but let me ask a more basic (and possibly stupid) question and a half.

 - When running 'git cmd :<path>' where '<path>' is a directory,
   even before the "sparse-index" or "sparse-checkout" world, I
   sometimes wished that ":<path>" resolved to the object name of
   the tree recorded in the cache-tree, if we have one.  If we are
   living in the "sparse-index" world, and the paths underneath
   '<path>' are not in the index (because we are not interested in
   them), we should be able answer "git rev-parse :<path>" with the
   name of the tree object.  It is a "new feature" regardless of
   sparse-index, but I wonder if it is sensible to add more code to
   engrave in stone that we would not support it and keep treating
   the index as if it is a flat list of paths to blobs (and commits,
   for submodules)?

 - When running 'git cmd :<path>' where '<path>' is *not* a
   directory but is not in the index because of "sparse-index"
   (i.e. a leading directory of '<path>' is represented as a tree in
   the index), should ":<path>" index expand the "tree" index entry
   on-demand, so that <path> has its own entry in the index, as if
   no "sparse-index" is in effect?  The tests I saw in the series
   were mostly asserted with test_sparse_match, not test_all_match,
   so I couldn't tell what the expectations are.

 - More generally, if <leading> path is represented as a
   "sparse-dir" entry, should ":<leading>/<lower>" cause the index
   entry for <leading> path to be expanded on-demand?  I am guessing
   that the answer is yes, as we wouldn't be able to even tell if
   such a path <leading>/<lower> would exist in the index if the
   index were converted to full upfront.

Thanks.