Re: [PATCH v2 0/2] combine-diff: honor relative paths consistently
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 7, 2026, 17:53 UTC
- Message-ID
- <xmqqh5ix8kji.fsf@gitster.g>
- In-Reply-To
- <cover.1791390459.git.dilsheddilu123@gmail.com>
Muhammed Dilshad A <dilsheddilu123@gmail.com> writes:
> The revised patch prints there/file as ../there/file when the prefix is > here/. All displayed names then use the same base. The tests cover both > discovery paths, including raw and NUL-separated output.
This sounds like the most sensible behaviour, within the constraint that --relative must give a relative path to the prefix.
Show 5 quoted lines
> The index rejects repeated separators, but tree entry parsing does not > enforce the same check. The helper now skips all separators at the prefix > boundary, so it does not rely on there being only one. Explicit prefix > arguments stay literal, matching ordinary diff's filtering behavior. > I also wrapped the added C lines to fit the coding guidelines.
Do *not* respond to review comments in your cover letter. Nobody reading the above, other than those who have seen our earlier exchange of you sending v1 patch with I commenting on it, would not know what you are talking about in the above, and especially what is so special about "repeated separators" without context. The cover letter should aim to welcome even those late-comming reviewers who missed an earlier round.
Review response should be done as a response to a review message, unrelated to your rerolled patches.
> While checking the two discovery paths, I found that the fast multi-tree > scan bypasses the relative-prefix filter entirely. Patch 2 fixes that > separately and adds tests for outside paths and repeated separators in > an explicit prefix.
Great.
Show 24 quoted lines
> > Changes since v1: > > * Use relative_path() for parent names outside the prefix while keeping > ordinary diff's literal-prefix behavior for matching names. > * Preserve /dev/null, skip all boundary separators, and wrap long lines. > * Add cross-directory rename tests and the separate fast-scan fix. > > The developer build with SANITIZE=leak succeeds. The affected suites pass > all 77 normal tests with SHA-1 and SHA-256. A separate run with > LSAN_OPTIONS=detect_leaks=1 also passes without a leak report. The existing > three-parent coalescing failure in t4038 remains an expected failure. > > Muhammed Dilshad A (2): > combine-diff: honor --relative when printing paths > combine-diff: filter the fast scan by the relative prefix > > combine-diff.c | 70 ++++++++++++++++++--- > t/t4038-diff-combined.sh | 130 +++++++++++++++++++++++++++++++++++++++ > t/t4045-diff-relative.sh | 62 ++++++++++++++++++- > 3 files changed, 251 insertions(+), 11 deletions(-) > > > base-commit: 6de20f6092dcf9bdb1c8efe03db4b70c82b423dd