On 6/10/2021 5:31 PM, Elijah Newren wrote: > On Thu, Jun 10, 2021 at 10:45 AM Derrick Stolee <stolee@xxxxxxxxx> wrote: >> >> On 6/9/2021 4:33 PM, Derrick Stolee wrote: >>> On 6/9/2021 4:11 AM, Elijah Newren wrote: >>>> On Tue, Jun 8, 2021 at 11:32 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: >>>>> >>>>> Elijah Newren <newren@xxxxxxxxx> 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! > >> I'll send a new version with this patch tomorrow, as well as the >> other recommended edits. ...still planning on this today, but... >> + /* >> + * 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