Merge pull request #3254 from ethomson/diff-binary-patch Handle binary DIFFABLEness properly
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135
diff --git a/src/diff_patch.c b/src/diff_patch.c
index 60648f8..ec4979a 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 f6ce57d..7b4dacd 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),
};
diff --git a/tests/diff/binary.c b/tests/diff/binary.c
index 424a53e..6a5f390 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);
@@ -262,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;