From 76633215d155dff2d5cda302aa868043b2c7090c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Wed, 24 Jun 2015 14:25:36 +0200 Subject: [PATCH 1/3] binary diff: test that the diff and patch otputs are the same We test the generation of the textual patch via the patch function, which are just one of two possibilities to get the output. Add a second patch generation via the diff function to make sure both outputs are in sync. --- tests/diff/binary.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/diff/binary.c b/tests/diff/binary.c index 424a53e5f..90f2d5a06 100644 --- a/tests/diff/binary.c +++ b/tests/diff/binary.c @@ -1,5 +1,7 @@ #include "clar_libgit2.h" +#include "git2/sys/diff.h" + #include "buffer.h" #include "filebuf.h" @@ -49,6 +51,11 @@ void test_patch( cl_assert_equal_s(expected, actual.ptr); + git_buf_clear(&actual); + cl_git_pass(git_diff_print(diff, GIT_DIFF_FORMAT_PATCH, git_diff_print_callback__to_buf, &actual)); + + cl_assert_equal_s(expected, actual.ptr); + git_buf_free(&actual); git_patch_free(patch); git_diff_free(diff); From ba8fb7c46ac66e941f9452c9743c13fc2daaa85b Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Wed, 24 Jun 2015 11:39:59 -0400 Subject: [PATCH 2/3] diff::binary tests: empty diff when forced binary Ensure that even when we're forcing a binary diff that we do not assume that there *is* a diff. There should be an empty diff for no change. --- tests/diff/binary.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/diff/binary.c b/tests/diff/binary.c index 90f2d5a06..6a5f3907b 100644 --- a/tests/diff/binary.c +++ b/tests/diff/binary.c @@ -269,6 +269,36 @@ void test_diff_binary__delta_append(void) git_index_free(index); } +void test_diff_binary__empty_for_no_diff(void) +{ + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; + git_oid id; + git_commit *commit; + git_tree *tree; + git_diff *diff; + git_buf actual = GIT_BUF_INIT; + const char *expected = ""; + + opts.flags = GIT_DIFF_SHOW_BINARY | GIT_DIFF_FORCE_BINARY; + opts.id_abbrev = GIT_OID_HEXSZ; + + repo = cl_git_sandbox_init("renames"); + + cl_git_pass(git_oid_fromstr(&id, "19dd32dfb1520a64e5bbaae8dce6ef423dfa2f13")); + cl_git_pass(git_commit_lookup(&commit, repo, &id)); + cl_git_pass(git_commit_tree(&tree, commit)); + + cl_git_pass(git_diff_tree_to_tree(&diff, repo, tree, tree, &opts)); + cl_git_pass(git_diff_print(diff, GIT_DIFF_FORMAT_PATCH, git_diff_print_callback__to_buf, &actual)); + + cl_assert_equal_s("", actual.ptr); + + git_buf_free(&actual); + git_diff_free(diff); + git_commit_free(commit); + git_tree_free(tree); +} + void test_diff_binary__index_to_workdir(void) { git_index *index; From 54077091c8cc12d83ad10b3252462c591c960de8 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Wed, 24 Jun 2015 12:06:41 -0400 Subject: [PATCH 3/3] diff: determine DIFFABLE-ness for binaries Always set `GIT_DIFF_PATCH_DIFFABLE` for all files, regardless of binary-ness, so that the binary callback is invoked to either show the binary contents, or just print the standard "Binary files differ" message. We may need to do deeper inspection for binary files where we have avoided loading the contents into a file map. --- src/diff_patch.c | 42 ++++++++++++++++++++++++++++++------------ src/diff_patch.h | 2 ++ 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/src/diff_patch.c b/src/diff_patch.c index 60648f8d7..ec4979a52 100644 --- a/src/diff_patch.c +++ b/src/diff_patch.c @@ -121,6 +121,35 @@ GIT_INLINE(bool) should_skip_binary(git_patch *patch, git_diff_file *file) return (file->flags & GIT_DIFF_FLAG_BINARY) != 0; } +static bool diff_patch_diffable(git_patch *patch) +{ + size_t olen, nlen; + + if (patch->delta->status == GIT_DELTA_UNMODIFIED) + return false; + + /* if we've determined this to be binary (and we are not showing binary + * data) then we have skipped loading the map data. instead, query the + * file data itself. + */ + if ((patch->delta->flags & GIT_DIFF_FLAG_BINARY) != 0 && + (patch->diff_opts.flags & GIT_DIFF_SHOW_BINARY) == 0) { + olen = (size_t)patch->ofile.file->size; + nlen = (size_t)patch->nfile.file->size; + } else { + olen = patch->ofile.map.len; + nlen = patch->nfile.map.len; + } + + /* if both sides are empty, files are identical */ + if (!olen && !nlen) + return false; + + /* otherwise, check the file sizes and the oid */ + return (olen != nlen || + !git_oid_equal(&patch->ofile.file->id, &patch->nfile.file->id)); +} + static int diff_patch_load(git_patch *patch, git_diff_output *output) { int error = 0; @@ -186,18 +215,7 @@ cleanup: diff_patch_update_binary(patch); if (!error) { - bool skip_binary = - (patch->delta->flags & GIT_DIFF_FLAG_BINARY) != 0 && - (patch->diff_opts.flags & GIT_DIFF_SHOW_BINARY) == 0; - - /* patch is diffable only for non-binary, modified files where - * at least one side has data and the data actually changed - */ - if (!skip_binary && - patch->delta->status != GIT_DELTA_UNMODIFIED && - (patch->ofile.map.len || patch->nfile.map.len) && - (patch->ofile.map.len != patch->nfile.map.len || - !git_oid_equal(&patch->ofile.file->id, &patch->nfile.file->id))) + if (diff_patch_diffable(patch)) patch->flags |= GIT_DIFF_PATCH_DIFFABLE; patch->flags |= GIT_DIFF_PATCH_LOADED; diff --git a/src/diff_patch.h b/src/diff_patch.h index f6ce57ddd..7b4dacdde 100644 --- a/src/diff_patch.h +++ b/src/diff_patch.h @@ -24,7 +24,9 @@ enum { GIT_DIFF_PATCH_ALLOCATED = (1 << 0), GIT_DIFF_PATCH_INITIALIZED = (1 << 1), GIT_DIFF_PATCH_LOADED = (1 << 2), + /* the two sides are different */ GIT_DIFF_PATCH_DIFFABLE = (1 << 3), + /* the difference between the two sides has been computed */ GIT_DIFF_PATCH_DIFFED = (1 << 4), GIT_DIFF_PATCH_FLATTENED = (1 << 5), };