From c2907575ec050d4cbc667544f7e6114e09b7b123 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 15 Jan 2013 09:24:17 -0800 Subject: [PATCH 1/3] Add FORCE_TEXT check into git_diff_blobs code path `git_diff_blobs` and `git_diff_blob_to_buffer` skip the step where we check file attributes because they don't have a filename associated with the data. Unfortunately, this meant they were also skipping the check for the GIT_DIFF_FORCE_TEXT option and so you could not force a diff of an apparent binary file. This adds the force text check into their code path. --- src/diff_output.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/diff_output.c b/src/diff_output.c index c2c259161..2e0be63c2 100644 --- a/src/diff_output.c +++ b/src/diff_output.c @@ -137,15 +137,22 @@ static int diff_delta_is_binary_by_content( git_diff_file *file, const git_map *map) { - const git_buf search = { map->data, 0, min(map->len, 4000) }; - - GIT_UNUSED(ctxt); + /* check if user is forcing us to text diff these files */ + if (ctxt->opts && (ctxt->opts->flags & GIT_DIFF_FORCE_TEXT) != 0) { + delta->old_file.flags |= GIT_DIFF_FILE_NOT_BINARY; + delta->new_file.flags |= GIT_DIFF_FILE_NOT_BINARY; + delta->binary = 0; + return 0; + } if ((file->flags & KNOWN_BINARY_FLAGS) == 0) { + const git_buf search = { map->data, 0, min(map->len, 4000) }; + /* TODO: provide encoding / binary detection callbacks that can * be UTF-8 aware, etc. For now, instead of trying to be smart, * let's just use the simple NUL-byte detection that core git uses. */ + /* previously was: if (git_buf_text_is_binary(&search)) */ if (git_buf_text_contains_nul(&search)) file->flags |= GIT_DIFF_FILE_BINARY; From ed55fd8bf85692040ddf6187a3528b99970c0127 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Mon, 11 Feb 2013 13:29:07 -0800 Subject: [PATCH 2/3] Reorganize FORCE_TEXT diff flag checks --- src/diff_output.c | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/src/diff_output.c b/src/diff_output.c index 2e0be63c2..26b073aad 100644 --- a/src/diff_output.c +++ b/src/diff_output.c @@ -89,13 +89,14 @@ static void update_delta_is_binary(git_diff_delta *delta) /* otherwise leave delta->binary value untouched */ } -static int diff_delta_is_binary_by_attr( - diff_context *ctxt, git_diff_patch *patch) +/* returns if we forced binary setting (and no further checks needed) */ +static bool diff_delta_is_binary_forced( + diff_context *ctxt, + git_diff_delta *delta) { - int error = 0, mirror_new; - git_diff_delta *delta = patch->delta; - - delta->binary = -1; + /* return true if binary-ness has already been settled */ + if (delta->binary != -1) + return true; /* make sure files are conceivably mmap-able */ if ((git_off_t)((size_t)delta->old_file.size) != delta->old_file.size || @@ -104,7 +105,7 @@ static int diff_delta_is_binary_by_attr( delta->old_file.flags |= GIT_DIFF_FILE_BINARY; delta->new_file.flags |= GIT_DIFF_FILE_BINARY; delta->binary = 1; - return 0; + return true; } /* check if user is forcing us to text diff these files */ @@ -112,9 +113,23 @@ static int diff_delta_is_binary_by_attr( delta->old_file.flags |= GIT_DIFF_FILE_NOT_BINARY; delta->new_file.flags |= GIT_DIFF_FILE_NOT_BINARY; delta->binary = 0; - return 0; + return true; } + return false; +} + +static int diff_delta_is_binary_by_attr( + diff_context *ctxt, git_diff_patch *patch) +{ + int error = 0, mirror_new; + git_diff_delta *delta = patch->delta; + + delta->binary = -1; + + if (diff_delta_is_binary_forced(ctxt, delta)) + return 0; + /* check diff attribute +, -, or 0 */ if (update_file_is_binary_by_attr(ctxt->repo, &delta->old_file) < 0) return -1; @@ -137,13 +152,8 @@ static int diff_delta_is_binary_by_content( git_diff_file *file, const git_map *map) { - /* check if user is forcing us to text diff these files */ - if (ctxt->opts && (ctxt->opts->flags & GIT_DIFF_FORCE_TEXT) != 0) { - delta->old_file.flags |= GIT_DIFF_FILE_NOT_BINARY; - delta->new_file.flags |= GIT_DIFF_FILE_NOT_BINARY; - delta->binary = 0; + if (diff_delta_is_binary_forced(ctxt, delta)) return 0; - } if ((file->flags & KNOWN_BINARY_FLAGS) == 0) { const git_buf search = { map->data, 0, min(map->len, 4000) }; From c2c0874de2a49a153f92e7a74c3faa1778a164e9 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Mon, 11 Feb 2013 14:44:56 -0800 Subject: [PATCH 3/3] More diff tests with binary data --- tests-clar/diff/blob.c | 134 ++++++++++++++++-- .../b4/35cd5689a0fb54afbeda4ac20368aa480e8f04 | Bin 0 -> 40 bytes 2 files changed, 119 insertions(+), 15 deletions(-) create mode 100644 tests-clar/resources/attr/.gitted/objects/b4/35cd5689a0fb54afbeda4ac20368aa480e8f04 diff --git a/tests-clar/diff/blob.c b/tests-clar/diff/blob.c index 4b29c9c94..a6950b871 100644 --- a/tests-clar/diff/blob.c +++ b/tests-clar/diff/blob.c @@ -352,6 +352,20 @@ void test_diff_blob__can_correctly_detect_a_textual_blob_as_non_binary(void) * git_diff_blob_to_buffer tests */ +static void assert_changed_single_one_line_file( + diff_expects *expected, git_delta_t mod) +{ + cl_assert_equal_i(1, expected->files); + cl_assert_equal_i(1, expected->file_status[mod]); + cl_assert_equal_i(1, expected->hunks); + cl_assert_equal_i(1, expected->lines); + + if (mod == GIT_DELTA_ADDED) + cl_assert_equal_i(1, expected->line_adds); + else if (mod == GIT_DELTA_DELETED) + cl_assert_equal_i(1, expected->line_dels); +} + void test_diff_blob__can_compare_blob_to_buffer(void) { git_blob *a; @@ -391,11 +405,7 @@ void test_diff_blob__can_compare_blob_to_buffer(void) NULL, a_content, strlen(a_content), &opts, diff_file_cb, diff_hunk_cb, diff_line_cb, &expected)); - cl_assert_equal_i(1, expected.files); - cl_assert_equal_i(1, expected.file_status[GIT_DELTA_ADDED]); - cl_assert_equal_i(1, expected.hunks); - cl_assert_equal_i(1, expected.lines); - cl_assert_equal_i(1, expected.line_adds); + assert_changed_single_one_line_file(&expected, GIT_DELTA_ADDED); /* diff from blob a to NULL buffer */ memset(&expected, 0, sizeof(expected)); @@ -403,11 +413,7 @@ void test_diff_blob__can_compare_blob_to_buffer(void) a, NULL, 0, &opts, diff_file_cb, diff_hunk_cb, diff_line_cb, &expected)); - cl_assert_equal_i(1, expected.files); - cl_assert_equal_i(1, expected.file_status[GIT_DELTA_DELETED]); - cl_assert_equal_i(1, expected.hunks); - cl_assert_equal_i(1, expected.lines); - cl_assert_equal_i(1, expected.line_dels); + assert_changed_single_one_line_file(&expected, GIT_DELTA_DELETED); /* diff with reverse */ opts.flags ^= GIT_DIFF_REVERSE; @@ -417,11 +423,109 @@ void test_diff_blob__can_compare_blob_to_buffer(void) a, NULL, 0, &opts, diff_file_cb, diff_hunk_cb, diff_line_cb, &expected)); - cl_assert_equal_i(1, expected.files); - cl_assert_equal_i(1, expected.file_status[GIT_DELTA_ADDED]); - cl_assert_equal_i(1, expected.hunks); - cl_assert_equal_i(1, expected.lines); - cl_assert_equal_i(1, expected.line_adds); + assert_changed_single_one_line_file(&expected, GIT_DELTA_ADDED); git_blob_free(a); } + + +static void assert_one_modified_with_lines(diff_expects *expected, int lines) +{ + cl_assert_equal_i(1, expected->files); + cl_assert_equal_i(1, expected->file_status[GIT_DELTA_MODIFIED]); + cl_assert_equal_i(0, expected->files_binary); + cl_assert_equal_i(lines, expected->lines); +} + +void test_diff_blob__binary_data_comparisons(void) +{ + git_blob *bin, *nonbin; + git_oid oid; + const char *nonbin_content = "Hello from the root\n"; + size_t nonbin_len = 20; + const char *bin_content = "0123456789\n\x01\x02\x03\x04\x05\x06\x07\x08\x09\x00\n0123456789\n"; + size_t bin_len = 33; + + cl_git_pass(git_oid_fromstrn(&oid, "45141a79", 8)); + cl_git_pass(git_blob_lookup_prefix(&nonbin, g_repo, &oid, 4)); + + cl_git_pass(git_oid_fromstrn(&oid, "b435cd56", 8)); + cl_git_pass(git_blob_lookup_prefix(&bin, g_repo, &oid, 4)); + + /* non-binary to reference content */ + + memset(&expected, 0, sizeof(expected)); + cl_git_pass(git_diff_blob_to_buffer( + nonbin, nonbin_content, nonbin_len, + &opts, diff_file_cb, diff_hunk_cb, diff_line_cb, &expected)); + assert_identical_blobs_comparison(&expected); + cl_assert_equal_i(0, expected.files_binary); + + /* binary to reference content */ + + memset(&expected, 0, sizeof(expected)); + cl_git_pass(git_diff_blob_to_buffer( + bin, bin_content, bin_len, + &opts, diff_file_cb, diff_hunk_cb, diff_line_cb, &expected)); + assert_identical_blobs_comparison(&expected); + + cl_assert_equal_i(1, expected.files_binary); + + /* non-binary to binary content */ + + memset(&expected, 0, sizeof(expected)); + cl_git_pass(git_diff_blob_to_buffer( + nonbin, bin_content, bin_len, + &opts, diff_file_cb, diff_hunk_cb, diff_line_cb, &expected)); + assert_binary_blobs_comparison(&expected); + + /* binary to non-binary content */ + + memset(&expected, 0, sizeof(expected)); + cl_git_pass(git_diff_blob_to_buffer( + bin, nonbin_content, nonbin_len, + &opts, diff_file_cb, diff_hunk_cb, diff_line_cb, &expected)); + assert_binary_blobs_comparison(&expected); + + /* non-binary to binary blob */ + + memset(&expected, 0, sizeof(expected)); + cl_git_pass(git_diff_blobs( + bin, nonbin, &opts, + diff_file_cb, diff_hunk_cb, diff_line_cb, &expected)); + assert_binary_blobs_comparison(&expected); + + /* + * repeat with FORCE_TEXT + */ + + opts.flags |= GIT_DIFF_FORCE_TEXT; + + memset(&expected, 0, sizeof(expected)); + cl_git_pass(git_diff_blob_to_buffer( + bin, bin_content, bin_len, + &opts, diff_file_cb, diff_hunk_cb, diff_line_cb, &expected)); + assert_identical_blobs_comparison(&expected); + + memset(&expected, 0, sizeof(expected)); + cl_git_pass(git_diff_blob_to_buffer( + nonbin, bin_content, bin_len, + &opts, diff_file_cb, diff_hunk_cb, diff_line_cb, &expected)); + assert_one_modified_with_lines(&expected, 4); + + memset(&expected, 0, sizeof(expected)); + cl_git_pass(git_diff_blob_to_buffer( + bin, nonbin_content, nonbin_len, + &opts, diff_file_cb, diff_hunk_cb, diff_line_cb, &expected)); + assert_one_modified_with_lines(&expected, 4); + + memset(&expected, 0, sizeof(expected)); + cl_git_pass(git_diff_blobs( + bin, nonbin, &opts, + diff_file_cb, diff_hunk_cb, diff_line_cb, &expected)); + assert_one_modified_with_lines(&expected, 4); + + /* cleanup */ + git_blob_free(bin); + git_blob_free(nonbin); +} diff --git a/tests-clar/resources/attr/.gitted/objects/b4/35cd5689a0fb54afbeda4ac20368aa480e8f04 b/tests-clar/resources/attr/.gitted/objects/b4/35cd5689a0fb54afbeda4ac20368aa480e8f04 new file mode 100644 index 0000000000000000000000000000000000000000..ffe3473f4bdc5fa059754fe5b4a2ff796f2f2e17 GIT binary patch literal 40 wcmb9JYNhwKbiK)rYAEZ2R5@KbTm&?`-05dHQTmS$7 literal 0 HcmV?d00001