diff --git a/src/merge.c b/src/merge.c index fc5088c82..89b8e8505 100644 --- a/src/merge.c +++ b/src/merge.c @@ -61,12 +61,6 @@ struct merge_diff_df_data { git_merge_diff *prev_conflict; }; -GIT_INLINE(int) merge_diff_detect_binary( - bool *binary_out, - git_repository *repo, - const git_merge_diff *conflict); - - /* Merge base computation */ int merge_bases_many(git_commit_list **out, git_revwalk **walk_out, git_repository *repo, size_t length, const git_oid input_array[]) @@ -668,7 +662,6 @@ static int merge_conflict_resolve_automerge( git_odb *odb = NULL; git_oid automerge_oid; int error = 0; - bool binary = false; assert(resolved && diff_list && conflict); @@ -703,12 +696,6 @@ static int merge_conflict_resolve_automerge( strcmp(conflict->ancestor_entry.path, conflict->their_entry.path) != 0) return 0; - /* Reject binary conflicts */ - if ((error = merge_diff_detect_binary(&binary, diff_list->repo, conflict)) < 0) - return error; - if (binary) - return 0; - ancestor = GIT_MERGE_INDEX_ENTRY_EXISTS(conflict->ancestor_entry) ? &conflict->ancestor_entry : NULL; ours = GIT_MERGE_INDEX_ENTRY_EXISTS(conflict->our_entry) ? @@ -1314,48 +1301,6 @@ GIT_INLINE(int) merge_diff_detect_type( return 0; } -GIT_INLINE(int) merge_diff_detect_binary( - bool *binary_out, - git_repository *repo, - const git_merge_diff *conflict) -{ - git_blob *ancestor_blob = NULL, *our_blob = NULL, *their_blob = NULL; - int error = 0; - bool binary = false; - - if (GIT_MERGE_INDEX_ENTRY_ISFILE(conflict->ancestor_entry)) { - if ((error = git_blob_lookup(&ancestor_blob, repo, &conflict->ancestor_entry.id)) < 0) - goto done; - - binary = git_blob_is_binary(ancestor_blob); - } - - if (!binary && - GIT_MERGE_INDEX_ENTRY_ISFILE(conflict->our_entry)) { - if ((error = git_blob_lookup(&our_blob, repo, &conflict->our_entry.id)) < 0) - goto done; - - binary = git_blob_is_binary(our_blob); - } - - if (!binary && - GIT_MERGE_INDEX_ENTRY_ISFILE(conflict->their_entry)) { - if ((error = git_blob_lookup(&their_blob, repo, &conflict->their_entry.id)) < 0) - goto done; - - binary = git_blob_is_binary(their_blob); - } - - *binary_out = binary; - -done: - git_blob_free(ancestor_blob); - git_blob_free(our_blob); - git_blob_free(their_blob); - - return error; -} - GIT_INLINE(int) index_entry_dup_pool( git_index_entry *out, git_pool *pool, diff --git a/src/merge_file.c b/src/merge_file.c index 6d89b089d..b174231cc 100644 --- a/src/merge_file.c +++ b/src/merge_file.c @@ -15,9 +15,15 @@ #include "git2/repository.h" #include "git2/object.h" #include "git2/index.h" +#include "git2/merge.h" #include "xdiff/xdiff.h" +/* only examine the first 8000 bytes for binaryness. + * https://github.com/git/git/blob/77bd3ea9f54f1584147b594abc04c26ca516d987/xdiff-interface.c#L197 + */ +#define GIT_MERGE_FILE_BINARY_SIZE 8000 + #define GIT_MERGE_FILE_SIDE_EXISTS(X) ((X)->mode != 0) GIT_INLINE(const char *) merge_file_best_path( @@ -100,7 +106,7 @@ static void merge_file_normalize_opts( } } -static int git_merge_file__from_inputs( +static int merge_file__xdiff( git_merge_file_result *out, const git_merge_file_input *ancestor, const git_merge_file_input *ours, @@ -189,6 +195,63 @@ done: return error; } +static bool merge_file__is_binary(const git_merge_file_input *file) +{ + size_t len = file ? file->size : 0; + + if (len > GIT_MERGE_FILE_XDIFF_MAX) + return true; + if (len > GIT_MERGE_FILE_BINARY_SIZE) + len = GIT_MERGE_FILE_BINARY_SIZE; + + return len ? (memchr(file->ptr, 0, len) != NULL) : false; +} + +static int merge_file__binary( + git_merge_file_result *out, + const git_merge_file_input *ours, + const git_merge_file_input *theirs, + const git_merge_file_options *given_opts) +{ + const git_merge_file_input *favored = NULL; + + memset(out, 0x0, sizeof(git_merge_file_result)); + + if (given_opts && given_opts->favor == GIT_MERGE_FILE_FAVOR_OURS) + favored = ours; + else if (given_opts && given_opts->favor == GIT_MERGE_FILE_FAVOR_THEIRS) + favored = theirs; + else + goto done; + + if ((out->path = git__strdup(favored->path)) == NULL || + (out->ptr = git__malloc(favored->size)) == NULL) + goto done; + + memcpy((char *)out->ptr, favored->ptr, favored->size); + out->len = favored->size; + out->mode = favored->mode; + out->automergeable = 1; + +done: + return 0; +} + +static int merge_file__from_inputs( + git_merge_file_result *out, + const git_merge_file_input *ancestor, + const git_merge_file_input *ours, + const git_merge_file_input *theirs, + const git_merge_file_options *given_opts) +{ + if (merge_file__is_binary(ancestor) || + merge_file__is_binary(ours) || + merge_file__is_binary(theirs)) + return merge_file__binary(out, ours, theirs, given_opts); + + return merge_file__xdiff(out, ancestor, ours, theirs, given_opts); +} + static git_merge_file_input *git_merge_file__normalize_inputs( git_merge_file_input *out, const git_merge_file_input *given) @@ -223,7 +286,7 @@ int git_merge_file( ours = git_merge_file__normalize_inputs(&inputs[1], ours); theirs = git_merge_file__normalize_inputs(&inputs[2], theirs); - return git_merge_file__from_inputs(out, ancestor, ours, theirs, options); + return merge_file__from_inputs(out, ancestor, ours, theirs, options); } int git_merge_file_from_index( @@ -234,8 +297,8 @@ int git_merge_file_from_index( const git_index_entry *theirs, const git_merge_file_options *options) { - git_merge_file_input inputs[3] = { {0} }, - *ancestor_input = NULL, *our_input = NULL, *their_input = NULL; + git_merge_file_input *ancestor_ptr = NULL, + ancestor_input = {0}, our_input = {0}, their_input = {0}; git_odb *odb = NULL; git_odb_object *odb_object[3] = { 0 }; int error = 0; @@ -249,27 +312,20 @@ int git_merge_file_from_index( if (ancestor) { if ((error = git_merge_file__input_from_index( - &inputs[0], &odb_object[0], odb, ancestor)) < 0) + &ancestor_input, &odb_object[0], odb, ancestor)) < 0) goto done; - ancestor_input = &inputs[0]; + ancestor_ptr = &ancestor_input; } if ((error = git_merge_file__input_from_index( - &inputs[1], &odb_object[1], odb, ours)) < 0) + &our_input, &odb_object[1], odb, ours)) < 0 || + (error = git_merge_file__input_from_index( + &their_input, &odb_object[2], odb, theirs)) < 0) goto done; - our_input = &inputs[1]; - - if ((error = git_merge_file__input_from_index( - &inputs[2], &odb_object[2], odb, theirs)) < 0) - goto done; - - their_input = &inputs[2]; - - if ((error = git_merge_file__from_inputs(out, - ancestor_input, our_input, their_input, options)) < 0) - goto done; + error = merge_file__from_inputs(out, + ancestor_ptr, &our_input, &their_input, options); done: git_odb_object_free(odb_object[0]); @@ -286,7 +342,5 @@ void git_merge_file_result_free(git_merge_file_result *result) return; git__free((char *)result->path); - - /* xdiff uses malloc() not git_malloc, so we use free(), not git_free() */ - free((char *)result->ptr); + git__free((char *)result->ptr); } diff --git a/src/merge_file.h b/src/merge_file.h index 263391ee3..08ecfc095 100644 --- a/src/merge_file.h +++ b/src/merge_file.h @@ -7,8 +7,7 @@ #ifndef INCLUDE_filediff_h__ #define INCLUDE_filediff_h__ -#include "xdiff/xdiff.h" - -#include "git2/merge.h" +/* xdiff cannot cope with large files, just treat them as binary */ +#define GIT_MERGE_FILE_XDIFF_MAX (1024UL * 1024 * 1023) #endif diff --git a/src/xdiff/xdiff.h b/src/xdiff/xdiff.h index e2f1e892b..db5d59884 100644 --- a/src/xdiff/xdiff.h +++ b/src/xdiff/xdiff.h @@ -20,6 +20,8 @@ * */ +#include "util.h" + #if !defined(XDIFF_H) #define XDIFF_H @@ -106,9 +108,9 @@ typedef struct s_bdiffparam { } bdiffparam_t; -#define xdl_malloc(x) malloc(x) -#define xdl_free(ptr) free(ptr) -#define xdl_realloc(ptr,x) realloc(ptr,x) +#define xdl_malloc(x) git__malloc(x) +#define xdl_free(ptr) git__free(ptr) +#define xdl_realloc(ptr,x) git__realloc(ptr,x) void *xdl_mmfile_first(mmfile_t *mmf, long *size); long xdl_mmfile_size(mmfile_t *mmf); diff --git a/tests/merge/files.c b/tests/merge/files.c index 2fd90d066..b365d5a42 100644 --- a/tests/merge/files.c +++ b/tests/merge/files.c @@ -3,6 +3,7 @@ #include "git2/merge.h" #include "buffer.h" #include "merge.h" +#include "merge_file.h" #include "merge_helpers.h" #include "refs.h" #include "fileops.h" @@ -288,3 +289,90 @@ void test_merge_files__doesnt_add_newline(void) git_merge_file_result_free(&result); } +void test_merge_files__skips_large_files(void) +{ + git_merge_file_input ours = GIT_MERGE_FILE_INPUT_INIT, + theirs = GIT_MERGE_FILE_INPUT_INIT; + git_merge_file_options opts = GIT_MERGE_FILE_OPTIONS_INIT; + git_merge_file_result result = {0}; + + ours.size = GIT_MERGE_FILE_XDIFF_MAX + 1; + ours.path = "testfile.txt"; + ours.mode = 0100755; + + theirs.size = GIT_MERGE_FILE_XDIFF_MAX + 1; + theirs.path = "testfile.txt"; + theirs.mode = 0100755; + + cl_git_pass(git_merge_file(&result, NULL, &ours, &theirs, &opts)); + + cl_assert_equal_i(0, result.automergeable); + + git_merge_file_result_free(&result); +} + +void test_merge_files__skips_binaries(void) +{ + git_merge_file_input ancestor = GIT_MERGE_FILE_INPUT_INIT, + ours = GIT_MERGE_FILE_INPUT_INIT, + theirs = GIT_MERGE_FILE_INPUT_INIT; + git_merge_file_result result = {0}; + + ancestor.ptr = "ance\0stor\0"; + ancestor.size = 10; + ancestor.path = "ancestor.txt"; + ancestor.mode = 0100755; + + ours.ptr = "foo\0bar\0"; + ours.size = 8; + ours.path = "ours.txt"; + ours.mode = 0100755; + + theirs.ptr = "bar\0foo\0"; + theirs.size = 8; + theirs.path = "theirs.txt"; + theirs.mode = 0100644; + + cl_git_pass(git_merge_file(&result, &ancestor, &ours, &theirs, NULL)); + + cl_assert_equal_i(0, result.automergeable); + + git_merge_file_result_free(&result); +} + +void test_merge_files__handles_binaries_when_favored(void) +{ + git_merge_file_input ancestor = GIT_MERGE_FILE_INPUT_INIT, + ours = GIT_MERGE_FILE_INPUT_INIT, + theirs = GIT_MERGE_FILE_INPUT_INIT; + git_merge_file_options opts = GIT_MERGE_FILE_OPTIONS_INIT; + git_merge_file_result result = {0}; + + ancestor.ptr = "ance\0stor\0"; + ancestor.size = 10; + ancestor.path = "ancestor.txt"; + ancestor.mode = 0100755; + + ours.ptr = "foo\0bar\0"; + ours.size = 8; + ours.path = "ours.txt"; + ours.mode = 0100755; + + theirs.ptr = "bar\0foo\0"; + theirs.size = 8; + theirs.path = "theirs.txt"; + theirs.mode = 0100644; + + opts.favor = GIT_MERGE_FILE_FAVOR_OURS; + cl_git_pass(git_merge_file(&result, &ancestor, &ours, &theirs, &opts)); + + cl_assert_equal_i(1, result.automergeable); + + cl_assert_equal_s("ours.txt", result.path); + cl_assert_equal_i(0100755, result.mode); + + cl_assert_equal_i(ours.size, result.len); + cl_assert(memcmp(result.ptr, ours.ptr, ours.size) == 0); + + git_merge_file_result_free(&result); +}