On 3/9/2021 5:40 PM, Elijah Newren wrote:
On Tue, Mar 9, 2021 at 2:21 PM Derrick Stolee [off-list ref] wrote:
quoted
On 3/8/2021 7:09 PM, Elijah Newren via GitGitGadget wrote:
quoted
@@ -1167,9 +1178,10 @@ void diffcore_rename_extended(struct diff_options *options,
/*
* Cull sources, again:
* - remove ones involved in renames (found via basenames)
+ * - remove ones not found in relevant_sources
*/
trace2_region_enter("diff", "cull basename", options->repo);
- remove_unneeded_paths_from_src(want_copies);
+ remove_unneeded_paths_from_src(want_copies, relevant_sources);
trace2_region_leave("diff", "cull basename", options->repo);
This seems backwards from your cover letter. You are using the exact renames
_and_ basename matches to remove the unneeded paths. Why are we not stripping
out the relevant_sources in the call further up, before we call
find_basename_matches()?
Yeah, good flag. I should add a comment to the commit message about
how these are still needed in initialize_dir_rename_info() for
basename-guided directory rename detection...and will also play a role
in some of the special cases where renames are needed for more than
three-way content merges. And add a comment about how by the end of
the series, relevant_sources will be passed to find_basename_matches()
so that it can skip over those paths.
Ah, so by the end, find_basename_matches() will be coupled to the
relevant_sources set directory instead of relying on the culling
happening earlier? Interesting. It seems to me like it would be
better to have them be less coupled by relying on the culling
before calling find_basename_matches(), but I'll keep reading to
see your good reason for not doing that.
Thanks,
-Stolee