Re: [PATCH v5 10/14] diff-lib: handle index diffs with sparse dirs

On 6/11/2021 8:57 AM, Derrick Stolee wrote:

On 6/10/2021 5:31 PM, Elijah Newren wrote:

quoted

On Thu, Jun 10, 2021 at 10:45 AM Derrick Stolee [off-list ref] wrote:

quoted

I'll send a new version with this patch tomorrow, as well as the
other recommended edits.

...still planning on this today, but...

So optimistic!
 

quoted

quoted

+       /*
+        * 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)) {
+               show_modified_sparse_directory(revs, old_entry, new_entry, report_missing, cached, match_missing);
+               return 0;
+       }

What if S_ISSPARSEDIR(old_entry->ce_mode) != S_ISSPARSEDIR(new_entry->ce_mode) ?

You make a good point that something different would happen
in the case of a directory/file conflict on the sparse checkout
boundary. This can be as simple as the trivial "only files at
root" cone-mode sparse-checkout definition, with "folder/" (tree)
changing to "folder" (blob).

I'll see what I can do to create a test scenario for
this and add the correct cases.

Creating a directory/file conflict in this way exposes a bug in
a different codepath in unpack_trees(), although it isn't visible
until 'git checkout' allows the index to stay sparse. It's due to
the code in unpack_callback() that handles blobs and trees
differently, and hence the blob/tree conflict isn't handled
appropriately there. The changes from Patch 8 are to blame for
these first errors.

At least, those are the first errors I have discovered with these
conflicts. There might be other scenarios that care about this
section of diff-lib.c, but I have not gotten to a point where
such behavior would be exposed.

I don't expect to succeed in squashing this bug today, so I'll
try again next week.

Thanks,
-Stolee