Re: [PATCH v2 1/8] diffcore-rename: enable filtering possible rename sources
- From
Derrick Stolee <stolee@gmail.com>
- Date
- Mar 9, 2021, 22:21 UTC
- Message-ID
- <c646e320-566b-3b53-63d8-a97c6d17c5bd@gmail.com>
- In-Reply-To
- <dab8e3c6aee5a852ad46c569c0be729d64310ad9.1615248599.git.gitgitgadget@gmail.com>
On 3/8/2021 7:09 PM, Elijah Newren via GitGitGadget wrote:> @@ -1127,9 +1137,10 @@ void diffcore_rename_extended(struct diff_options *options,
Show 9 quoted lines
> /*
> * Cull sources:
> * - remove ones corresponding to exact renames
> + * - remove ones not found in relevant_sources
> */
> trace2_region_enter("diff", "cull after exact", options->repo);
> - remove_unneeded_paths_from_src(want_copies);
> + remove_unneeded_paths_from_src(want_copies, relevant_sources);
> trace2_region_leave("diff", "cull after exact", options->repo);In this case, we are checking for copies. Perhaps there is a reason why we want to include relevant_sources in that case, so I'll look for it later.
Show 21 quoted lines
> } else {
> /* Determine minimum score to match basenames */
> @@ -1148,7 +1159,7 @@ void diffcore_rename_extended(struct diff_options *options,
> * - remove ones involved in renames (found via exact match)
> */
> trace2_region_enter("diff", "cull after exact", options->repo);
> - remove_unneeded_paths_from_src(want_copies);
> + remove_unneeded_paths_from_src(want_copies, NULL);
> trace2_region_leave("diff", "cull after exact", options->repo);
>
> /* Preparation for basename-driven matching. */
> @@ -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()?
Thanks, -Stolee