[PATCH v2] diffcore-delta: avoid ignoring final 'line' of file
- From
Elijah Newren via GitGitGadget <gitgitgadget@gmail.com>
- Date
- Jan 13, 2024, 04:26 UTC
- Message-ID
- <pull.1637.v2.git.1705119973690.gitgitgadget@gmail.com>
- In-Reply-To
- <pull.1637.git.1705006074626.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>
hash_chars() would hash lines to integers, and store them in a spanhash, but cut lines at 64 characters. Thus, whenever it reached 64 characters or a newline, it would create a new spanhash. The problem is, the final part of the file might not end 64 characters after the previous 'line' and might not end with a newline. This could, for example, cause an 85-byte file with 12 lines and only the first character in the file differing to appear merely 23% similar rather than the expected 97%. Ensure the last line is included, and add a testcase that would have caught this problem.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
diffcore-delta: avoid ignoring final 'line' of file
Found while experimenting with converting portions of diffcore-delta to
Rust.
Changes since v1:
* Removed the unnecessary similarity threshold specification
* Munged the discovered similarity percentage so we are only checking
that a rename is detected
* Fixed up filenamesPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1637%2Fnewren%2Ffix-diffcore-final-line-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1637/newren/fix-diffcore-final-line-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/1637
Range-diff vs v1:
1: f62b22a54c3 ! 1: e0223bbfeb5 diffcore-delta: avoid ignoring final 'line' of file
@@ t/t4001-diff-rename.sh: test_expect_success 'basename similarity vs best similar
'
+test_expect_success 'last line matters too' '
-+ test_write_lines a 0 1 2 3 4 5 6 7 8 9 >nonewline &&
-+ printf "git ignores final up to 63 characters if not newline terminated" >>nonewline &&
-+ git add nonewline &&
++ {
++ test_write_lines a 0 1 2 3 4 5 6 7 8 9 &&
++ printf "git ignores final up to 63 characters if not newline terminated"
++ } >no-final-lf &&
++ git add no-final-lf &&
+ git commit -m "original version of file with no final newline" &&
+
+ # Change ONLY the first character of the whole file
-+ test_write_lines b 0 1 2 3 4 5 6 7 8 9 >nonewline &&
-+ printf "git ignores final up to 63 characters if not newline terminated" >>nonewline &&
-+ git add nonewline &&
-+ git mv nonewline still-no-newline &&
-+ git commit -a -m "rename nonewline -> still-no-newline" &&
-+ git diff-tree -r -M01 --name-status HEAD^ HEAD >actual &&
++ {
++ test_write_lines b 0 1 2 3 4 5 6 7 8 9 &&
++ printf "git ignores final up to 63 characters if not newline terminated"
++ } >no-final-lf &&
++ git add no-final-lf &&
++ git mv no-final-lf still-absent-final-lf &&
++ git commit -a -m "rename no-final-lf -> still-absent-final-lf" &&
++ git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
++ sed -e "s/^R[0-9]* /R /" actual >actual.munged &&
+ cat >expected <<-\EOF &&
-+ R097 nonewline still-no-newline
++ R no-final-lf still-absent-final-lf
+ EOF
-+ test_cmp expected actual
++ test_cmp expected actual.munged
+'
+
test_donediffcore-delta.c | 4 ++++ t/t4001-diff-rename.sh | 24 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+)
diff --git a/diffcore-delta.c b/diffcore-delta.c index c30b56e983b..7136c3dd203 100644 --- a/diffcore-delta.c +++ b/diffcore-delta.c @@ -159,6 +159,10 @@ static struct spanhash_top *hash_chars(struct repository *r, n = 0; accum1 = accum2 = 0; } + if (n > 0) { + hashval = (accum1 + accum2 * 0x61) % HASHBASE; + hash = add_spanhash(hash, hashval, n); + } QSORT(hash->data, (size_t)1ul << hash->alloc_log2, spanhash_cmp); return hash; } diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh index 85be1367de6..49c042a38ae 100755 --- a/t/t4001-diff-rename.sh +++ b/t/t4001-diff-rename.sh @@ -286,4 +286,28 @@ test_expect_success 'basename similarity vs best similarity' ' test_cmp expected actual ' +test_expect_success 'last line matters too' ' + { + test_write_lines a 0 1 2 3 4 5 6 7 8 9 && + printf "git ignores final up to 63 characters if not newline terminated" + } >no-final-lf && + git add no-final-lf && + git commit -m "original version of file with no final newline" && + + # Change ONLY the first character of the whole file + { + test_write_lines b 0 1 2 3 4 5 6 7 8 9 && + printf "git ignores final up to 63 characters if not newline terminated" + } >no-final-lf && + git add no-final-lf && + git mv no-final-lf still-absent-final-lf && + git commit -a -m "rename no-final-lf -> still-absent-final-lf" && + git diff-tree -r -M --name-status HEAD^ HEAD >actual && + sed -e "s/^R[0-9]* /R /" actual >actual.munged && + cat >expected <<-\EOF && + R no-final-lf still-absent-final-lf + EOF + test_cmp expected actual.munged +' + test_done base-commit: 055bb6e9969085777b7fab83e3fee0017654f134
-- gitgitgadget