Re: [PATCH v5 10/14] diff-lib: handle index diffs with sparse dirs
From: Derrick Stolee <hidden>
Date: 2021-06-10 17:46:16
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2021-06-09 · Re: [PATCH v5 10/14] diff-lib: handle index diffs with sparse dirs · Elijah Newren <hidden>
- 2021-06-08 · Re: [PATCH v5 10/14] diff-lib: handle index diffs with sparse dirs · Derrick Stolee <hidden>
- 2021-06-08 · Re: [PATCH v5 10/14] diff-lib: handle index diffs with sparse dirs · Junio C Hamano <hidden>
- 2021-06-07 · Re: [PATCH v5 10/14] diff-lib: handle index diffs with sparse dirs · Derrick Stolee <hidden>
- 2021-06-07 · [PATCH v5 10/14] diff-lib: handle index diffs with sparse dirs · Derrick Stolee via GitGitGadget <hidden>
On 6/9/2021 4:33 PM, Derrick Stolee wrote:
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). I'll send a new version with this patch tomorrow, as well as the other recommended edits. Thanks, -Stolee
--- >8 ---
diff --git a/diff-lib.c b/diff-lib.c
index c2ac9250fe9..b631df89343 100644
--- a/diff-lib.c
+++ b/diff-lib.c@@ -316,6 +316,13 @@ static int get_stat_data(const struct index_state *istate, return 0; } +static void show_directory(struct rev_info *revs, + const struct cache_entry *new_dir, + int added) +{ + diff_tree_oid(NULL, &new_dir->oid, new_dir->name, &revs->diffopt); +} + static void show_new_file(struct rev_info *revs, const struct cache_entry *new_file, int cached, int match_missing)
@@ -325,6 +332,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)) { + show_directory(revs, new_file, /*added */ 1); + return; + } + /* * New file in the index: it might actually be different in * the working tree.
@@ -336,6 +348,15 @@ static void show_new_file(struct rev_info *revs, diff_index_show_file(revs, "+", new_file, oid, !is_null_oid(oid), mode, dirty_submodule); } +static void show_modified_sparse_directory(struct rev_info *revs, + const struct cache_entry *old_entry, + const struct cache_entry *new_entry, + int report_missing, + int cached, int match_missing) +{ + diff_tree_oid(&old_entry->oid, &new_entry->oid, new_entry->name, &revs->diffopt); +} + static int show_modified(struct rev_info *revs, const struct cache_entry *old_entry, const struct cache_entry *new_entry,
@@ -347,6 +368,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)) { + show_modified_sparse_directory(revs, old_entry, new_entry, report_missing, cached, match_missing); + return 0; + } + if (get_stat_data(istate, new_entry, &oid, &mode, cached, match_missing, &dirty_submodule, &revs->diffopt) < 0) { if (report_missing)