Re: [PATCH v5 1/2] t1092: add tests for `git diff-files`

> I'd be interested in seeing an additional test case for a pathspec with
> wildcards or other "magic" [1], e.g. 'git diff-files "deep/*"'. In past
> sparse index integrations, there has occasionally been a need for special
> handling of those types of pathspecs [2][3], so it would be good for the
> test to cover cases like that.

> From the comment you've added here, it looks like the state you want to test
> is "file outside sparse checkout definition exists on disk". However, since
> one of the goals of this test is to verify sparse index behavior once its
> integrated with the 'diff-files' command, whether the "outside of sparse-
> checkout definition" file belongs to a sparse directory entry is an
> important (and fairly nuanced) factor to consider.
>
> The main difference between a "regular" sparse-checkout and a sparse
> index-enabled sparse-checkout is the existence of "sparse directory"
> entries: index entries with 'SKIP_WORKTREE' that represent directories and
> their contents. In the context of these tests, the thing we really want to
> verify is that the sparse index-enabled case produces the same results as
> the full index when an operation needs to get some information out of a
> sparse directory.
>
> Coming back to this test, the 'newdirectory/testfile' you create, while
> outside the sparse-checkout definition, never actually belongs to a sparse
> directory because 'newdirectory' is never collapsed - in fact, I don't
> think it even gets the SKIP_WORKTREE bit in the index. To ensure you have a
> sparse directory & SKIP_WORKTREE, you'd need to run 'git sparse-checkout
> reapply' after removing 'newdirectory/testfile' from disk - which doesn't
> help if you want to test what happens when the file exists on disk!

> In fact, because of built-in safeguards around on-disk files and
> sparse-checkout, there isn't really a way in Git to materialize a file
> without also expanding its sparse directory and removing SKIP_WORKTREE. If
> you want to preserve a sparse directory, you should write the contents of an
> existing file inside that sparse directory to disk manually:
>
>         run_on_sparse mkdir folder1 &&
>         run_on_sparse cp a folder1/a &&  # `folder1/a` is identical to `a` in the base commit
>
> Git's index will not be touched by doing this, meaning the next command you
> invoke (in this case, 'diff-files') will need to reconcile the file on disk
> with what it sees in the index.
>
> > +     test_sparse_match git diff-files &&
> > +     test_must_be_empty sparse-checkout-out &&
> > +     test_must_be_empty sparse-checkout-err &&
> > +     test_sparse_match git diff-files newdirectory/testfile &&
> > +     test_must_be_empty sparse-checkout-out &&
> > +     test_must_be_empty sparse-checkout-err &&
>
> These checks should be 'test_all_match' rather than 'test_sparse_match'.
> Since (through other tests) we're confident that 'git diff-files' on an
> unmodified, non-sparse-checkout repository will give us an empty result, you
> wouldn't need the additional 'test_must_be_empty' checks.
>
> A bit of a "spoiler": when I tested this out locally, I found that the diff
> was *not* empty for the sparse-checkout cases, until I ran 'git status'
> (which strips the 'SKIP_WORKTREE' bit from the file and writes it to the
> index). That's not the desired behavior, so there's a bug in the
> sparse-checkout logic used in 'diff-files' that needs to be fixed (my first
> guess would be that 'clear_skip_worktree_from_present_files()' is not being
> applied when the index is read).

> > +     run_on_sparse ../edit-contents newdirectory/testfile &&
> > +     test_sparse_match git diff-files &&
> > +     test_cmp expect sparse-checkout-out &&
> > +     test_sparse_match git diff-files newdirectory/testfile &&
> > +     test_cmp expect sparse-checkout-out
>
> Similarly, you should use 'run_on_all' to modify the file & 'test_all_match'
> to verify that they all match here. It would demonstrate that we don't
> expect any "special" behavior from sparse-checkout, meaning you can probably
> avoid checking the result verbatim.