From 1cca150c71cfb0ac9dff0014b9d9b98aa9c3b691 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Fri, 2 Sep 2016 02:03:45 -0500 Subject: diff: treat binary patches with no data special When creating and printing diffs, deal with binary deltas that have binary data specially, versus diffs that have a binary file but lack the actual binary data. --- include/git2/diff.h | 7 +++++++ src/apply.c | 5 +++++ src/diff_print.c | 9 +++------ src/patch_generate.c | 4 +++- src/patch_parse.c | 45 ++++++++++++++++++++++++++++++++++----------- tests/apply/fromdiff.c | 2 +- tests/patch/patch_common.h | 5 +++++ tests/patch/print.c | 6 ++++++ 8 files changed, 64 insertions(+), 19 deletions(-) diff --git a/include/git2/diff.h b/include/git2/diff.h index c5e463fe3..70664637b 100644 --- a/include/git2/diff.h +++ b/include/git2/diff.h @@ -490,6 +490,13 @@ typedef struct { /** Structure describing the binary contents of a diff. */ typedef struct { + /** + * Whether there is data in this binary structure or not. If this + * is non-zero, then this was generated knowing only that a binary + * file changed but without providing the data. For example, seeing + * only `BBinary files a/file.txt and b/file.txt differ`. + */ + unsigned int empty_data; git_diff_binary_file old_file; /**< The contents of the old file. */ git_diff_binary_file new_file; /**< The contents of the new file. */ } git_diff_binary; diff --git a/src/apply.c b/src/apply.c index 10bf1f492..f05fc7c2f 100644 --- a/src/apply.c +++ b/src/apply.c @@ -293,6 +293,11 @@ static int apply_binary( git_buf reverse = GIT_BUF_INIT; int error; + if (patch->binary.empty_data) { + error = apply_err("patch does not contain binary data"); + goto done; + } + /* first, apply the new_file delta to the given source */ if ((error = apply_binary_delta(out, source, source_len, &patch->binary.new_file)) < 0) diff --git a/src/diff_print.c b/src/diff_print.c index f72ca8935..c19693251 100644 --- a/src/diff_print.c +++ b/src/diff_print.c @@ -500,7 +500,6 @@ static int diff_print_patch_file_binary_noshow( &new_path, new_pfx, delta->new_file.path)) < 0) goto done; - pi->line.num_lines = 1; error = diff_delta_format_with_paths( pi->buf, delta, "Binary files %s and %s differ\n", @@ -521,13 +520,10 @@ static int diff_print_patch_file_binary( size_t pre_binary_size; int error; - if ((pi->flags & GIT_DIFF_SHOW_BINARY) == 0) + if ((pi->flags & GIT_DIFF_SHOW_BINARY) == 0 || binary->empty_data) return diff_print_patch_file_binary_noshow( pi, delta, old_pfx, new_pfx); - if (binary->new_file.datalen == 0 && binary->old_file.datalen == 0) - return 0; - pre_binary_size = pi->buf->size; git_buf_printf(pi->buf, "GIT binary patch\n"); pi->line.num_lines++; @@ -564,7 +560,8 @@ static int diff_print_patch_file( (pi->flags & GIT_DIFF_FORCE_BINARY); bool show_binary = !!(pi->flags & GIT_DIFF_SHOW_BINARY); int id_strlen = binary && show_binary ? - GIT_OID_HEXSZ : pi->id_strlen; + MAX(delta->old_file.id_abbrev, delta->new_file.id_abbrev) : + pi->id_strlen; GIT_UNUSED(progress); diff --git a/src/patch_generate.c b/src/patch_generate.c index 67a13b706..d0103a85c 100644 --- a/src/patch_generate.c +++ b/src/patch_generate.c @@ -342,7 +342,7 @@ done: static int diff_binary(git_patch_generated_output *output, git_patch_generated *patch) { - git_diff_binary binary = {{0}}; + git_diff_binary binary = {0}; const char *old_data = patch->ofile.map.data; const char *new_data = patch->nfile.map.data; size_t old_len = patch->ofile.map.len, @@ -366,6 +366,8 @@ static int diff_binary(git_patch_generated_output *output, git_patch_generated * &binary.new_file.inflatedlen, old_data, old_len, new_data, new_len)) < 0) return error; + } else { + binary.empty_data = 1; } error = giterr_set_after_callback_function( diff --git a/src/patch_parse.c b/src/patch_parse.c index 44bcf8f75..95c381b2e 100644 --- a/src/patch_parse.c +++ b/src/patch_parse.c @@ -74,7 +74,10 @@ static int parse_advance_expected( return 0; } -#define parse_advance_expected_s(ctx, str) \ +#define parse_advance_expected_str(ctx, str) \ + parse_advance_expected(ctx, str, strlen(str)) + +#define parse_advance_expected_const_str(ctx, str) \ parse_advance_expected(ctx, str, sizeof(str) - 1) static int parse_advance_ws(git_patch_parse_ctx *ctx) @@ -220,7 +223,7 @@ static int parse_header_git_index( { if (parse_header_oid(&patch->base.delta->old_file.id, &patch->base.delta->old_file.id_abbrev, ctx) < 0 || - parse_advance_expected_s(ctx, "..") < 0 || + parse_advance_expected_const_str(ctx, "..") < 0 || parse_header_oid(&patch->base.delta->new_file.id, &patch->base.delta->new_file.id_abbrev, ctx) < 0) return -1; @@ -336,7 +339,7 @@ static int parse_header_percent(uint16_t *out, git_patch_parse_ctx *ctx) parse_advance_chars(ctx, (end - ctx->line)); - if (parse_advance_expected_s(ctx, "%") < 0) + if (parse_advance_expected_const_str(ctx, "%") < 0) return -1; if (val > 100) @@ -379,6 +382,7 @@ static const header_git_op header_git_ops[] = { { "diff --git ", NULL }, { "@@ -", NULL }, { "GIT binary patch", NULL }, + { "Binary files ", NULL }, { "--- ", parse_header_git_oldpath }, { "+++ ", parse_header_git_newpath }, { "index ", parse_header_git_index }, @@ -404,7 +408,7 @@ static int parse_header_git( int error = 0; /* Parse the diff --git line */ - if (parse_advance_expected_s(ctx, "diff --git ") < 0) + if (parse_advance_expected_const_str(ctx, "diff --git ") < 0) return parse_err("corrupt git diff header at line %d", ctx->line_num); if (parse_header_path(&patch->header_old_path, ctx) < 0) @@ -443,7 +447,7 @@ static int parse_header_git( goto done; parse_advance_ws(ctx); - parse_advance_expected_s(ctx, "\n"); + parse_advance_expected_const_str(ctx, "\n"); if (ctx->line_len > 0) { error = parse_err("trailing data at line %d", ctx->line_num); @@ -505,27 +509,27 @@ static int parse_hunk_header( hunk->hunk.old_lines = 1; hunk->hunk.new_lines = 1; - if (parse_advance_expected_s(ctx, "@@ -") < 0 || + if (parse_advance_expected_const_str(ctx, "@@ -") < 0 || parse_int(&hunk->hunk.old_start, ctx) < 0) goto fail; if (ctx->line_len > 0 && ctx->line[0] == ',') { - if (parse_advance_expected_s(ctx, ",") < 0 || + if (parse_advance_expected_const_str(ctx, ",") < 0 || parse_int(&hunk->hunk.old_lines, ctx) < 0) goto fail; } - if (parse_advance_expected_s(ctx, " +") < 0 || + if (parse_advance_expected_const_str(ctx, " +") < 0 || parse_int(&hunk->hunk.new_start, ctx) < 0) goto fail; if (ctx->line_len > 0 && ctx->line[0] == ',') { - if (parse_advance_expected_s(ctx, ",") < 0 || + if (parse_advance_expected_const_str(ctx, ",") < 0 || parse_int(&hunk->hunk.new_lines, ctx) < 0) goto fail; } - if (parse_advance_expected_s(ctx, " @@") < 0) + if (parse_advance_expected_const_str(ctx, " @@") < 0) goto fail; parse_advance_line(ctx); @@ -782,7 +786,7 @@ static int parse_patch_binary( { int error; - if (parse_advance_expected_s(ctx, "GIT binary patch") < 0 || + if (parse_advance_expected_const_str(ctx, "GIT binary patch") < 0 || parse_advance_nl(ctx) < 0) return parse_err("corrupt git binary header at line %d", ctx->line_num); @@ -808,6 +812,23 @@ static int parse_patch_binary( return 0; } +static int parse_patch_binary_nodata( + git_patch_parsed *patch, + git_patch_parse_ctx *ctx) +{ + if (parse_advance_expected_const_str(ctx, "Binary files ") < 0 || + parse_advance_expected_str(ctx, patch->header_old_path) < 0 || + parse_advance_expected_const_str(ctx, " and ") < 0 || + parse_advance_expected_str(ctx, patch->header_new_path) < 0 || + parse_advance_expected_const_str(ctx, " differ") < 0 || + parse_advance_nl(ctx) < 0) + return parse_err("corrupt git binary header at line %d", ctx->line_num); + + patch->base.delta->flags |= GIT_DIFF_FLAG_BINARY; + patch->base.binary.empty_data = 1; + return 0; +} + static int parse_patch_hunks( git_patch_parsed *patch, git_patch_parse_ctx *ctx) @@ -840,6 +861,8 @@ static int parse_patch_body( { if (parse_ctx_contains_s(ctx, "GIT binary patch")) return parse_patch_binary(patch, ctx); + else if (parse_ctx_contains_s(ctx, "Binary files ")) + return parse_patch_binary_nodata(patch, ctx); else return parse_patch_hunks(patch, ctx); } diff --git a/tests/apply/fromdiff.c b/tests/apply/fromdiff.c index 349773964..acaa63356 100644 --- a/tests/apply/fromdiff.c +++ b/tests/apply/fromdiff.c @@ -240,7 +240,7 @@ void test_apply_fromdiff__binary_no_change(void) cl_git_pass(apply_gitbuf( &original, "binary.bin", &original, "binary.bin", - "", &binary_opts)); + "GIT binary patch\nliteral 0\n\nliteral 0\n\n", &binary_opts)); } void test_apply_fromdiff__binary_change_delta(void) diff --git a/tests/patch/patch_common.h b/tests/patch/patch_common.h index e097062d2..6ec554690 100644 --- a/tests/patch/patch_common.h +++ b/tests/patch/patch_common.h @@ -661,3 +661,8 @@ "\n" \ "delta 48\n" \ "mc$~Y)c#%<%fq{_;hPgsAGK(h)CJASj=y9P)1m{m|^9BI99|yz$\n\n" + +#define PATCH_BINARY_NOT_PRINTED \ + "diff --git a/binary.bin b/binary.bin\n" \ + "index 27184d9..7c94f9e 100644\n" \ + "Binary files a/binary.bin and b/binary.bin differ\n" diff --git a/tests/patch/print.c b/tests/patch/print.c index 5a86573b3..62e50b93e 100644 --- a/tests/patch/print.c +++ b/tests/patch/print.c @@ -166,3 +166,9 @@ void test_patch_print__not_reversible(void) patch_print_from_patchfile(PATCH_BINARY_NOT_REVERSIBLE, strlen(PATCH_BINARY_NOT_REVERSIBLE)); } + +void test_patch_print__binary_not_shown(void) +{ + patch_print_from_patchfile(PATCH_BINARY_NOT_PRINTED, + strlen(PATCH_BINARY_NOT_PRINTED)); +} -- cgit v1.2.1