git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[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 filenames
Published-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_done
 diffcore-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
Previous: Junio C Hamano
Message 10 of 10 in “diffcore-delta: avoid ignoring final 'line' of file”
  1. diffcore-delta: avoid ignoring final 'line' of fileElijah Newren via GitGitGadget, Jan 11, 2024
  2. Taylor BlauJan 11, 2024
  3. Junio C HamanoJan 11, 2024
  4. Elijah NewrenJan 13, 2024
  5. Junio C HamanoJan 13, 2024
  6. Elijah NewrenJan 19, 2024
  7. Junio C HamanoJan 19, 2024
  8. Elijah NewrenJan 19, 2024
  9. Junio C HamanoJan 19, 2024
  10. diffcore-delta: avoid ignoring final 'line' of fileElijah Newren via GitGitGadget, Jan 13, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.