Re: [PATCH v7 12/16] diff-lib: handle index diffs with sparse dirs

>
> On Mon, Jun 28, 2021 at 7:05 PM Derrick Stolee via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> >
> > From: Derrick Stolee <dstolee@microsoft.com>
> >
> > While comparing an index to a tree, we may see a sparse directory entry.
> > In this case, we should compare that portion of the tree to the tree
> > represented by that entry. This could include a new tree which needs to
> > be expanded to a full list of added files. It could also include an
> > existing tree, in which case all of the changes inside are important to
> > describe, including the modifications, additions, and deletions. Note
> > that the case where the tree has a path and the index does not remains
> > identical to before: the lack of a cache entry is the same with a sparse
> > index.
> >
> > Use diff_tree_oid() appropriately to compute the diff.
> >
> > Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> > ---
> >  diff-lib.c | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/diff-lib.c b/diff-lib.c
> > index c2ac9250fe9..3f32f038371 100644
> > --- a/diff-lib.c
> > +++ b/diff-lib.c
> > @@ -325,6 +325,11 @@ static void show_new_file(struct rev_info *revs,
> >         unsigned dirty_submodule = 0;
> >         struct index_state *istate = revs->diffopt.repo->index;
> >
> > +       if (new_file && S_ISSPARSEDIR(new_file->ce_mode)) {
> > +               diff_tree_oid(NULL, &new_file->oid, new_file->name, &revs->diffopt);
> > +               return;
> > +       }
> > +
> >         /*
> >          * New file in the index: it might actually be different in
> >          * the working tree.
> > @@ -347,6 +352,17 @@ static int show_modified(struct rev_info *revs,
> >         unsigned dirty_submodule = 0;
> >         struct index_state *istate = revs->diffopt.repo->index;
> >
> > +       /*
> > +        * If both are sparse directory entries, then expand the
> > +        * modifications to the file level.
> > +        */
> > +       if (old_entry && new_entry &&
> > +           S_ISSPARSEDIR(old_entry->ce_mode) &&
> > +           S_ISSPARSEDIR(new_entry->ce_mode)) {
> > +               diff_tree_oid(&old_entry->oid, &new_entry->oid, new_entry->name, &revs->diffopt);
> > +               return 0;
> > +       }
> > +
> >         if (get_stat_data(istate, new_entry, &oid, &mode, cached, match_missing,
> >                           &dirty_submodule, &revs->diffopt) < 0) {
> >                 if (report_missing)
>
> Love the simpler patch.
>
> I'm curious about the case where S_ISSPARSEDIR(old_entry->ce_mode) !=
> S_ISSPARSEDIR(new_entry->ce_mode), though; how is that handled?