diff --git a/include/git2/diff.h b/include/git2/diff.h index 6afa608bb..bafe6268c 100644 --- a/include/git2/diff.h +++ b/include/git2/diff.h @@ -337,12 +337,21 @@ GIT_EXTERN(int) git_diff_print_patch( /** * Directly run a text diff on two blobs. + * + * Compared to a file, a blob lacks some contextual information. As such, the + * `git_diff_file` parameters of the callbacks will be filled accordingly to the following: + * `mode` will be set to 0, `path` will be set to NULL. When dealing with a NULL blob, `oid` + * will be set to 0. + * + * When at least one of the blobs being dealt with is binary, the `git_diff_delta` binary + * attribute will be set to 1 and no call to the hunk_cb nor line_cb will be made. */ GIT_EXTERN(int) git_diff_blobs( git_blob *old_blob, git_blob *new_blob, git_diff_options *options, void *cb_data, + git_diff_file_fn file_cb, git_diff_hunk_fn hunk_cb, git_diff_data_fn line_cb); diff --git a/src/diff_output.c b/src/diff_output.c index c380db996..9c8e07972 100644 --- a/src/diff_output.c +++ b/src/diff_output.c @@ -169,7 +169,6 @@ static int file_is_binary_by_attr( } static int file_is_binary_by_content( - git_diff_list *diff, git_diff_delta *delta, git_map *old_data, git_map *new_data) @@ -177,8 +176,6 @@ static int file_is_binary_by_content( git_buf search; git_text_stats stats; - GIT_UNUSED(diff); - if ((delta->old_file.flags & BINARY_DIFF_FLAGS) == 0) { search.ptr = old_data->data; search.size = min(old_data->len, 4000); @@ -301,6 +298,16 @@ static void release_content(git_diff_file *file, git_map *map, git_blob *blob) } } +static void fill_map_from_mmfile(git_map *dst, mmfile_t *src) { + assert(dst && src); + + dst->data = src->ptr; + dst->len = src->size; +#ifdef GIT_WIN32 + dst->fmh = NULL; +#endif +} + int git_diff_foreach( git_diff_list *diff, void *data, @@ -408,7 +415,7 @@ int git_diff_foreach( */ if (delta->binary == -1) { error = file_is_binary_by_content( - diff, delta, &old_data, &new_data); + delta, &old_data, &new_data); if (error < 0) goto cleanup; } @@ -689,55 +696,83 @@ int git_diff_print_patch( return error; } - int git_diff_blobs( git_blob *old_blob, git_blob *new_blob, git_diff_options *options, void *cb_data, + git_diff_file_fn file_cb, git_diff_hunk_fn hunk_cb, git_diff_data_fn line_cb) { diff_output_info info; git_diff_delta delta; mmfile_t old_data, new_data; + git_map old_map, new_map; xpparam_t xdiff_params; xdemitconf_t xdiff_config; xdemitcb_t xdiff_callback; + git_blob *new, *old; + + memset(&delta, 0, sizeof(delta)); + + new = new_blob; + old = old_blob; if (options && (options->flags & GIT_DIFF_REVERSE)) { - git_blob *swap = old_blob; - old_blob = new_blob; - new_blob = swap; + git_blob *swap = old; + old = new; + new = swap; } - if (old_blob) { - old_data.ptr = (char *)git_blob_rawcontent(old_blob); - old_data.size = git_blob_rawsize(old_blob); + if (old) { + old_data.ptr = (char *)git_blob_rawcontent(old); + old_data.size = git_blob_rawsize(old); + git_oid_cpy(&delta.old_file.oid, git_object_id((const git_object *)old)); } else { old_data.ptr = ""; old_data.size = 0; } - if (new_blob) { - new_data.ptr = (char *)git_blob_rawcontent(new_blob); - new_data.size = git_blob_rawsize(new_blob); + if (new) { + new_data.ptr = (char *)git_blob_rawcontent(new); + new_data.size = git_blob_rawsize(new); + git_oid_cpy(&delta.new_file.oid, git_object_id((const git_object *)new)); } else { new_data.ptr = ""; new_data.size = 0; } /* populate a "fake" delta record */ - delta.status = old_data.ptr ? - (new_data.ptr ? GIT_DELTA_MODIFIED : GIT_DELTA_DELETED) : - (new_data.ptr ? GIT_DELTA_ADDED : GIT_DELTA_UNTRACKED); - delta.old_file.mode = 0100644; /* can't know the truth from a blob alone */ - delta.new_file.mode = 0100644; - git_oid_cpy(&delta.old_file.oid, git_object_id((const git_object *)old_blob)); - git_oid_cpy(&delta.new_file.oid, git_object_id((const git_object *)new_blob)); - delta.old_file.path = NULL; - delta.new_file.path = NULL; - delta.similarity = 0; + delta.status = new ? + (old ? GIT_DELTA_MODIFIED : GIT_DELTA_ADDED) : + (old ? GIT_DELTA_DELETED : GIT_DELTA_UNTRACKED); + + if (git_oid_cmp(&delta.new_file.oid, &delta.old_file.oid) == 0) + delta.status = GIT_DELTA_UNMODIFIED; + + delta.old_file.size = old_data.size; + delta.new_file.size = new_data.size; + + fill_map_from_mmfile(&old_map, &old_data); + fill_map_from_mmfile(&new_map, &new_data); + + if (file_is_binary_by_content(&delta, &old_map, &new_map) < 0) + return -1; + + if (file_cb != NULL) { + int error = file_cb(cb_data, &delta, 1); + if (error < 0) + return error; + } + + /* don't do hunk and line diffs if the two blobs are identical */ + if (delta.status == GIT_DELTA_UNMODIFIED) + return 0; + + /* don't do hunk and line diffs if file is binary */ + if (delta.binary == 1) + return 0; info.diff = NULL; info.delta = δ diff --git a/tests-clar/diff/blob.c b/tests-clar/diff/blob.c index ed1f14a07..cceb00d25 100644 --- a/tests-clar/diff/blob.c +++ b/tests-clar/diff/blob.c @@ -2,23 +2,43 @@ #include "diff_helpers.h" static git_repository *g_repo = NULL; +static diff_expects exp; +static git_diff_options opts; +static git_blob *d, *alien; void test_diff_blob__initialize(void) { + git_oid oid; + g_repo = cl_git_sandbox_init("attr"); + + memset(&opts, 0, sizeof(opts)); + opts.context_lines = 1; + opts.interhunk_lines = 1; + + memset(&exp, 0, sizeof(exp)); + + /* tests/resources/attr/root_test4.txt */ + cl_git_pass(git_oid_fromstrn(&oid, "fe773770c5a6", 12)); + cl_git_pass(git_blob_lookup_prefix(&d, g_repo, &oid, 6)); + + /* alien.png */ + cl_git_pass(git_oid_fromstrn(&oid, "edf3dcee", 8)); + cl_git_pass(git_blob_lookup_prefix(&alien, g_repo, &oid, 4)); } void test_diff_blob__cleanup(void) { + git_blob_free(d); + git_blob_free(alien); + cl_git_sandbox_cleanup(); } -void test_diff_blob__0(void) +void test_diff_blob__can_compare_text_blobs(void) { - git_blob *a, *b, *c, *d; - git_oid a_oid, b_oid, c_oid, d_oid; - git_diff_options opts = {0}; - diff_expects exp; + git_blob *a, *b, *c; + git_oid a_oid, b_oid, c_oid; /* tests/resources/attr/root_test1 */ cl_git_pass(git_oid_fromstrn(&a_oid, "45141a79", 8)); @@ -32,18 +52,14 @@ void test_diff_blob__0(void) cl_git_pass(git_oid_fromstrn(&c_oid, "c96bbb2c2557a832", 16)); cl_git_pass(git_blob_lookup_prefix(&c, g_repo, &c_oid, 8)); - /* tests/resources/attr/root_test4.txt */ - cl_git_pass(git_oid_fromstrn(&d_oid, "fe773770c5a6", 12)); - cl_git_pass(git_blob_lookup_prefix(&d, g_repo, &d_oid, 6)); - /* Doing the equivalent of a `git diff -U1` on these files */ - opts.context_lines = 1; - opts.interhunk_lines = 1; - - memset(&exp, 0, sizeof(exp)); cl_git_pass(git_diff_blobs( - a, b, &opts, &exp, diff_hunk_fn, diff_line_fn)); + a, b, &opts, &exp, diff_file_fn, diff_hunk_fn, diff_line_fn)); + + cl_assert(exp.files == 1); + cl_assert(exp.file_mods == 1); + cl_assert(exp.at_least_one_of_them_is_binary == false); cl_assert(exp.hunks == 1); cl_assert(exp.lines == 6); @@ -53,7 +69,11 @@ void test_diff_blob__0(void) memset(&exp, 0, sizeof(exp)); cl_git_pass(git_diff_blobs( - b, c, &opts, &exp, diff_hunk_fn, diff_line_fn)); + b, c, &opts, &exp, diff_file_fn, diff_hunk_fn, diff_line_fn)); + + cl_assert(exp.files == 1); + cl_assert(exp.file_mods == 1); + cl_assert(exp.at_least_one_of_them_is_binary == false); cl_assert(exp.hunks == 1); cl_assert(exp.lines == 15); @@ -63,7 +83,11 @@ void test_diff_blob__0(void) memset(&exp, 0, sizeof(exp)); cl_git_pass(git_diff_blobs( - a, c, &opts, &exp, diff_hunk_fn, diff_line_fn)); + a, c, &opts, &exp, diff_file_fn, diff_hunk_fn, diff_line_fn)); + + cl_assert(exp.files == 1); + cl_assert(exp.file_mods == 1); + cl_assert(exp.at_least_one_of_them_is_binary == false); cl_assert(exp.hunks == 1); cl_assert(exp.lines == 13); @@ -75,7 +99,11 @@ void test_diff_blob__0(void) memset(&exp, 0, sizeof(exp)); cl_git_pass(git_diff_blobs( - c, d, &opts, &exp, diff_hunk_fn, diff_line_fn)); + c, d, &opts, &exp, diff_file_fn, diff_hunk_fn, diff_line_fn)); + + cl_assert(exp.files == 1); + cl_assert(exp.file_mods == 1); + cl_assert(exp.at_least_one_of_them_is_binary == false); cl_assert(exp.hunks == 2); cl_assert(exp.lines == 14); @@ -86,6 +114,141 @@ void test_diff_blob__0(void) git_blob_free(a); git_blob_free(b); git_blob_free(c); - git_blob_free(d); } +void test_diff_blob__can_compare_against_null_blobs(void) +{ + git_blob *e = NULL; + + cl_git_pass(git_diff_blobs( + d, e, &opts, &exp, diff_file_fn, diff_hunk_fn, diff_line_fn)); + + cl_assert(exp.files == 1); + cl_assert(exp.file_dels == 1); + cl_assert(exp.at_least_one_of_them_is_binary == false); + + cl_assert(exp.hunks == 1); + cl_assert(exp.hunk_old_lines == 14); + cl_assert(exp.lines == 14); + cl_assert(exp.line_dels == 14); + + opts.flags |= GIT_DIFF_REVERSE; + memset(&exp, 0, sizeof(exp)); + + cl_git_pass(git_diff_blobs( + d, e, &opts, &exp, diff_file_fn, diff_hunk_fn, diff_line_fn)); + + cl_assert(exp.files == 1); + cl_assert(exp.file_adds == 1); + cl_assert(exp.at_least_one_of_them_is_binary == false); + + cl_assert(exp.hunks == 1); + cl_assert(exp.hunk_new_lines == 14); + cl_assert(exp.lines == 14); + cl_assert(exp.line_adds == 14); + + opts.flags ^= GIT_DIFF_REVERSE; + memset(&exp, 0, sizeof(exp)); + + cl_git_pass(git_diff_blobs( + alien, NULL, &opts, &exp, diff_file_fn, diff_hunk_fn, diff_line_fn)); + + cl_assert(exp.at_least_one_of_them_is_binary == true); + + cl_assert(exp.files == 1); + cl_assert(exp.file_dels == 1); + cl_assert(exp.hunks == 0); + cl_assert(exp.lines == 0); + + memset(&exp, 0, sizeof(exp)); + + cl_git_pass(git_diff_blobs( + NULL, alien, &opts, &exp, diff_file_fn, diff_hunk_fn, diff_line_fn)); + + cl_assert(exp.at_least_one_of_them_is_binary == true); + + cl_assert(exp.files == 1); + cl_assert(exp.file_adds == 1); + cl_assert(exp.hunks == 0); + cl_assert(exp.lines == 0); +} + +void assert_identical_blobs_comparison(diff_expects exp) +{ + cl_assert(exp.files == 1); + cl_assert(exp.file_unmodified == 1); + cl_assert(exp.hunks == 0); + cl_assert(exp.lines == 0); +} + +void test_diff_blob__can_compare_identical_blobs(void) +{ + cl_git_pass(git_diff_blobs( + d, d, &opts, &exp, diff_file_fn, diff_hunk_fn, diff_line_fn)); + + cl_assert(exp.at_least_one_of_them_is_binary == false); + assert_identical_blobs_comparison(exp); + + memset(&exp, 0, sizeof(exp)); + cl_git_pass(git_diff_blobs( + NULL, NULL, &opts, &exp, diff_file_fn, diff_hunk_fn, diff_line_fn)); + + cl_assert(exp.at_least_one_of_them_is_binary == false); + assert_identical_blobs_comparison(exp); + + memset(&exp, 0, sizeof(exp)); + cl_git_pass(git_diff_blobs( + alien, alien, &opts, &exp, diff_file_fn, diff_hunk_fn, diff_line_fn)); + + cl_assert(exp.at_least_one_of_them_is_binary == true); + assert_identical_blobs_comparison(exp); +} + +void assert_binary_blobs_comparison(diff_expects exp) +{ + cl_assert(exp.at_least_one_of_them_is_binary == true); + + cl_assert(exp.files == 1); + cl_assert(exp.file_mods == 1); + cl_assert(exp.hunks == 0); + cl_assert(exp.lines == 0); +} + +void test_diff_blob__can_compare_two_binary_blobs(void) +{ + git_blob *heart; + git_oid h_oid; + + /* heart.png */ + cl_git_pass(git_oid_fromstrn(&h_oid, "de863bff", 8)); + cl_git_pass(git_blob_lookup_prefix(&heart, g_repo, &h_oid, 4)); + + cl_git_pass(git_diff_blobs( + alien, heart, &opts, &exp, diff_file_fn, diff_hunk_fn, diff_line_fn)); + + assert_binary_blobs_comparison(exp); + + memset(&exp, 0, sizeof(exp)); + + cl_git_pass(git_diff_blobs( + heart, alien, &opts, &exp, diff_file_fn, diff_hunk_fn, diff_line_fn)); + + assert_binary_blobs_comparison(exp); + + git_blob_free(heart); +} + +void test_diff_blob__can_compare_a_binary_blob_and_a_text_blob(void) +{ + cl_git_pass(git_diff_blobs( + alien, d, &opts, &exp, diff_file_fn, diff_hunk_fn, diff_line_fn)); + + assert_binary_blobs_comparison(exp); + + memset(&exp, 0, sizeof(exp)); + + cl_git_pass(git_diff_blobs( + d, alien, &opts, &exp, diff_file_fn, diff_hunk_fn, diff_line_fn)); + + assert_binary_blobs_comparison(exp); +} diff --git a/tests-clar/diff/diff_helpers.c b/tests-clar/diff/diff_helpers.c index 74a44ab99..8587be9b1 100644 --- a/tests-clar/diff/diff_helpers.c +++ b/tests-clar/diff/diff_helpers.c @@ -30,6 +30,8 @@ int diff_file_fn( GIT_UNUSED(progress); + e-> at_least_one_of_them_is_binary = delta->binary; + e->files++; switch (delta->status) { case GIT_DELTA_ADDED: e->file_adds++; break; @@ -37,6 +39,7 @@ int diff_file_fn( case GIT_DELTA_MODIFIED: e->file_mods++; break; case GIT_DELTA_IGNORED: e->file_ignored++; break; case GIT_DELTA_UNTRACKED: e->file_untracked++; break; + case GIT_DELTA_UNMODIFIED: e->file_unmodified++; break; default: break; } return 0; diff --git a/tests-clar/diff/diff_helpers.h b/tests-clar/diff/diff_helpers.h index ca8c40177..0aaa6c111 100644 --- a/tests-clar/diff/diff_helpers.h +++ b/tests-clar/diff/diff_helpers.h @@ -11,6 +11,7 @@ typedef struct { int file_mods; int file_ignored; int file_untracked; + int file_unmodified; int hunks; int hunk_new_lines; @@ -20,6 +21,8 @@ typedef struct { int line_ctxt; int line_adds; int line_dels; + + bool at_least_one_of_them_is_binary; } diff_expects; extern int diff_file_fn( diff --git a/tests-clar/diff/tree.c b/tests-clar/diff/tree.c index 06f51a16b..b932fa10e 100644 --- a/tests-clar/diff/tree.c +++ b/tests-clar/diff/tree.c @@ -113,16 +113,16 @@ void test_diff_tree__options(void) */ diff_expects test_expects[] = { /* a vs b tests */ - { 5, 3, 0, 2, 0, 0, 4, 0, 0, 51, 2, 46, 3 }, - { 5, 3, 0, 2, 0, 0, 4, 0, 0, 53, 4, 46, 3 }, - { 5, 0, 3, 2, 0, 0, 4, 0, 0, 52, 3, 3, 46 }, - { 5, 3, 0, 2, 0, 0, 5, 0, 0, 54, 3, 48, 3 }, + { 5, 3, 0, 2, 0, 0, 0, 4, 0, 0, 51, 2, 46, 3 }, + { 5, 3, 0, 2, 0, 0, 0, 4, 0, 0, 53, 4, 46, 3 }, + { 5, 0, 3, 2, 0, 0, 0, 4, 0, 0, 52, 3, 3, 46 }, + { 5, 3, 0, 2, 0, 0, 0, 5, 0, 0, 54, 3, 48, 3 }, /* c vs d tests */ - { 1, 0, 0, 1, 0, 0, 1, 0, 0, 22, 9, 10, 3 }, - { 1, 0, 0, 1, 0, 0, 1, 0, 0, 19, 12, 7, 0 }, - { 1, 0, 0, 1, 0, 0, 1, 0, 0, 20, 11, 8, 1 }, - { 1, 0, 0, 1, 0, 0, 1, 0, 0, 20, 11, 8, 1 }, - { 1, 0, 0, 1, 0, 0, 1, 0, 0, 18, 11, 0, 7 }, + { 1, 0, 0, 1, 0, 0, 0, 1, 0, 0, 22, 9, 10, 3 }, + { 1, 0, 0, 1, 0, 0, 0, 1, 0, 0, 19, 12, 7, 0 }, + { 1, 0, 0, 1, 0, 0, 0, 1, 0, 0, 20, 11, 8, 1 }, + { 1, 0, 0, 1, 0, 0, 0, 1, 0, 0, 20, 11, 8, 1 }, + { 1, 0, 0, 1, 0, 0, 0, 1, 0, 0, 18, 11, 0, 7 }, { 0 }, }; diff_expects *expected; diff --git a/tests-clar/resources/attr/.gitted/objects/de/863bff4976c9ed7e17a4da0fd524908dc84049 b/tests-clar/resources/attr/.gitted/objects/de/863bff4976c9ed7e17a4da0fd524908dc84049 new file mode 100644 index 000000000..7d9b8551f Binary files /dev/null and b/tests-clar/resources/attr/.gitted/objects/de/863bff4976c9ed7e17a4da0fd524908dc84049 differ diff --git a/tests-clar/resources/attr/.gitted/objects/ed/f3dcee4003d71f139777898882ccd097e34c53 b/tests-clar/resources/attr/.gitted/objects/ed/f3dcee4003d71f139777898882ccd097e34c53 new file mode 100644 index 000000000..d28184670 Binary files /dev/null and b/tests-clar/resources/attr/.gitted/objects/ed/f3dcee4003d71f139777898882ccd097e34c53 differ