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

Re: [PATCH 1/2] diff: fix handling of binary rewrite diffs

From
Junio C Hamano <gitster@pobox.com>
Date
Dec 10, 2008, 08:34 UTC
Message-ID
<7vvdts1l92.fsf@gitster.siamese.dyndns.org>
In-Reply-To
<20081209081227.GA19626@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 8 quoted lines
> Instead, if we have binary files, then let's just skip emit_rewrite_diff
> altogether. We will already have shown the "dissimilarity index" line,
> so it is really about the diff contents. If binary diffs are turned off,
> the "Binary files a/file and b/file differ" message should be the same
> in either case. If we do have binary patches turned on, there isn't much
> point in making a less-efficient binary patch that does a total rewrite;
> no human is going to read it, and since binary patches don't apply with
> any fuzz anyway, the result of application should be the same.
Makes sense.
Show 20 quoted lines
>  diff.c                         |    4 ++-
>  t/t4031-diff-rewrite-binary.sh |   42 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 45 insertions(+), 1 deletions(-)
>  create mode 100755 t/t4031-diff-rewrite-binary.sh
>
> diff --git a/diff.c b/diff.c
> index f644947..ea958a2 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -1376,7 +1376,9 @@ static void builtin_diff(const char *name_a,
>  		 */
>  		if ((one->mode ^ two->mode) & S_IFMT)
>  			goto free_ab_and_return;
> -		if (complete_rewrite) {
> +		if (complete_rewrite &&
> +		    !diff_filespec_is_binary(one) &&
> +		    !diff_filespec_is_binary(two)) {
>  			emit_rewrite_diff(name_a, name_b, one, two, o);
>  			o->found_changes = 1;
>  			goto free_ab_and_return;
And looks correct.
Show 29 quoted lines
> diff --git a/t/t4031-diff-rewrite-binary.sh b/t/t4031-diff-rewrite-binary.sh
> new file mode 100755
> index 0000000..4b522f7
> --- /dev/null
> +++ b/t/t4031-diff-rewrite-binary.sh
> @@ -0,0 +1,42 @@
> +#!/bin/sh
> +
> +test_description='rewrite diff on binary file'
> +
> +. ./test-lib.sh
> +
> +# We must be large enough to meet the MINIMUM_BREAK_SIZE
> +# requirement.
> +make_file() {
> +	for i in 1 2 3 4 5 6 7 8 9 10; do
> +		for j in 1 2 3 4 5 6 7 9 10; do
> +			for k in 1 2 3 4 5; do
> +				printf "$1\n"
> +			done
> +		done
> +	done >file
> +}
> +
> +test_expect_success 'create binary file with changes' '
> +	make_file "\\0" &&
> +	git add file &&
> +	make_file "\\01"
> +'
Hmm... "1 2 3 4 5 6 7 9 10"?
Previous: Junio C HamanoNext: Jeff King
Message 6 of 7 in “diff: fix handling of binary rewrite diffs”
  1. 1/2 diff: fix handling of binary rewrite diffsJeff King, Dec 9, 2008
  2. 2/2 diff: respect textconv in rewrite diffsJeff King, Dec 9, 2008
  3. Junio C HamanoDec 10, 2008
  4. Jeff KingDec 10, 2008
  5. Junio C HamanoDec 10, 2008
  6. Junio C HamanoDec 10, 2008
  7. Jeff KingDec 10, 2008

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.