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

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

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

quoted

On 6/9/2021 4:33 PM, Derrick Stolee wrote:

quoted

On 6/9/2021 4:11 AM, Elijah Newren wrote:

quoted

On Tue, Jun 8, 2021 at 11:32 PM Junio C Hamano [off-list ref] wrote:

quoted

Elijah Newren [off-list ref] writes:

The tree-diff machinery takes two trees, walks them in parallel and
repeatedly calls either diff_addremove() or diff_change(), which
appends diff_filepair() to the diff_queue[] structure.  If you see
an unexpanded tree on the index side, you should be able to pass
that tree with the subtree you are comparing against to the tree-diff
machinery to come up with a series of filepairs, and then tweak the
pathnames of these filepairs (as such a two-tree comparison would be
comparing two trees representing a single subdirectory of two different
vintages) before adding them to the diff_queue[] you are collecting
the index-vs-tree diff, for example.

Good to know it seems my idea might be reasonable.

I agree that this is reasonable. I just didn't look hard enough
to find existing code for this, since I found traverse_trees and
thought that _was_ the library for this.

This was surprisingly simple, since most of the complicated stuff
is built into diff_tree_oid() and its use of revs->diffopt. The
new patch works as shown below the cut-line.

I was incredibly suspicious of how quickly this came together,
but it passes all the tests I have for it (including Scalar
functional tests with the commit, checkout, and add integrations).

Nice!

quoted

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

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

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.

Thanks,
-Stolee