Commit ece5bb5e7d6e35e50096bac3d7bf17342daaec77
2019-11-07T14:10:00
diff: make patchid computation work with all types of commits. Current implementation of patchid is not computing a correct patchid when given a patch where, for example, a new file is added or removed. Some more corner cases need to be handled to have same behavior as git patch-id command. Add some more tests to cover those corner cases. Signed-off-by: Gregory Herrero <gregory.herrero@oracle.com>
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 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202
diff --git a/src/diff.c b/src/diff.c
index 15a32ed..47f49d9 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -426,81 +426,38 @@ static void strip_spaces(git_buf *buf)
git_buf_truncate(buf, len);
}
-static int file_cb(
+int git_diff_patchid_print_callback__to_buf(
const git_diff_delta *delta,
- float progress,
+ const git_diff_hunk *hunk,
+ const git_diff_line *line,
void *payload)
{
struct patch_id_args *args = (struct patch_id_args *) payload;
git_buf buf = GIT_BUF_INIT;
- int error;
-
- GIT_UNUSED(progress);
+ int error = 0;
- if (!args->first_file &&
- (error = flush_hunk(&args->result, &args->ctx)) < 0)
- goto out;
- args->first_file = 0;
-
- if ((error = git_buf_printf(&buf,
- "diff--gita/%sb/%s---a/%s+++b/%s",
- delta->old_file.path,
- delta->new_file.path,
- delta->old_file.path,
- delta->new_file.path)) < 0)
+ if (line->origin == GIT_DIFF_LINE_CONTEXT_EOFNL ||
+ line->origin == GIT_DIFF_LINE_ADD_EOFNL ||
+ line->origin == GIT_DIFF_LINE_DEL_EOFNL)
goto out;
- strip_spaces(&buf);
-
- if ((error = git_hash_update(&args->ctx, buf.ptr, buf.size)) < 0)
+ if ((error = git_diff_print_callback__to_buf(delta, hunk,
+ line, &buf)) < 0)
goto out;
-out:
- git_buf_dispose(&buf);
- return error;
-}
-
-static int patchid_line_cb(
- const git_diff_delta *delta,
- const git_diff_hunk *hunk,
- const git_diff_line *line,
- void *payload)
-{
- struct patch_id_args *args = (struct patch_id_args *) payload;
- git_buf buf = GIT_BUF_INIT;
- int error;
-
- GIT_UNUSED(delta);
- GIT_UNUSED(hunk);
-
- switch (line->origin) {
- case GIT_DIFF_LINE_ADDITION:
- git_buf_putc(&buf, '+');
- break;
- case GIT_DIFF_LINE_DELETION:
- git_buf_putc(&buf, '-');
- break;
- case GIT_DIFF_LINE_CONTEXT:
- break;
- case GIT_DIFF_LINE_CONTEXT_EOFNL:
- case GIT_DIFF_LINE_ADD_EOFNL:
- case GIT_DIFF_LINE_DEL_EOFNL:
- /*
- * Ignore EOF without newlines for patch IDs as whitespace is
- * not supposed to be significant.
- */
- return 0;
- default:
- git_error_set(GIT_ERROR_PATCH, "invalid line origin for patch");
- return -1;
- }
-
- git_buf_put(&buf, line->content, line->content_len);
strip_spaces(&buf);
+ if (line->origin == GIT_DIFF_LINE_FILE_HDR &&
+ !args->first_file &&
+ (error = flush_hunk(&args->result, &args->ctx) < 0))
+ goto out;
+
if ((error = git_hash_update(&args->ctx, buf.ptr, buf.size)) < 0)
goto out;
+ if (line->origin == GIT_DIFF_LINE_FILE_HDR && args->first_file)
+ args->first_file = 0;
+
out:
git_buf_dispose(&buf);
return error;
@@ -526,7 +483,10 @@ int git_diff_patchid(git_oid *out, git_diff *diff, git_diff_patchid_options *opt
if ((error = git_hash_ctx_init(&args.ctx)) < 0)
goto out;
- if ((error = git_diff_foreach(diff, file_cb, NULL, NULL, patchid_line_cb, &args)) < 0)
+ if ((error = git_diff_print(diff,
+ GIT_DIFF_FORMAT_PATCH_ID,
+ git_diff_patchid_print_callback__to_buf,
+ &args)) < 0)
goto out;
if ((error = (flush_hunk(&args.result, &args.ctx))) < 0)
diff --git a/tests/diff/patchid.c b/tests/diff/patchid.c
index 75a2aa8..621a720 100644
--- a/tests/diff/patchid.c
+++ b/tests/diff/patchid.c
@@ -20,6 +20,39 @@ void test_diff_patchid__simple_commit(void)
verify_patch_id(PATCH_SIMPLE_COMMIT, "06094b1948b878b7d9ff7560b4eae672a014b0ec");
}
+void test_diff_patchid__deleted_file(void)
+{
+ verify_patch_id(PATCH_DELETE_ORIGINAL, "d18507fe189f49c028b32c8c34e1ad98dd6a1aad");
+ verify_patch_id(PATCH_DELETED_FILE_2_HUNKS, "f31412498a17e6c3fbc635f2c5f9aa3ef4c1a9b7");
+}
+
+void test_diff_patchid__created_file(void)
+{
+ verify_patch_id(PATCH_ADD_ORIGINAL, "a7d39379308021465ae2ce65e338c048a3110db6");
+}
+
+void test_diff_patchid__binary_file(void)
+{
+ verify_patch_id(PATCH_ADD_BINARY_NOT_PRINTED, "2b31236b485faa30cf4dd33e4d6539829996739f");
+}
+
+void test_diff_patchid__renamed_file(void)
+{
+ verify_patch_id(PATCH_RENAME_EXACT, "4666d50cea4976f6f727448046d43461912058fd");
+ verify_patch_id(PATCH_RENAME_SIMILAR, "a795087575fcb940227be524488bedd6b3d3f438");
+}
+
+void test_diff_patchid__modechange(void)
+{
+ verify_patch_id(PATCH_MODECHANGE_UNCHANGED, "dbf3423ee98375ef1c72a79fbd29a049a2bae771");
+ verify_patch_id(PATCH_MODECHANGE_MODIFIED, "93aba696e1bbd2bbb73e3e3e62ed71f232137657");
+}
+
+void test_diff_patchid__shuffle_hunks(void)
+{
+ verify_patch_id(PATCH_DELETED_FILE_2_HUNKS_SHUFFLED, "f31412498a17e6c3fbc635f2c5f9aa3ef4c1a9b7");
+}
+
void test_diff_patchid__filename_with_spaces(void)
{
verify_patch_id(PATCH_APPEND_NO_NL, "f0ba05413beaef743b630e796153839462ee477a");
diff --git a/tests/patch/patch_common.h b/tests/patch/patch_common.h
index 1c6ad7e..6601685 100644
--- a/tests/patch/patch_common.h
+++ b/tests/patch/patch_common.h
@@ -385,6 +385,38 @@
"@@ -9,0 +10 @@ below it!\n" \
"+insert at end\n"
+#define PATCH_DELETED_FILE_2_HUNKS \
+ "diff --git a/a b/a\n" \
+ "index 7f129fd..af431f2 100644\n" \
+ "--- a/a\n" \
+ "+++ b/a\n" \
+ "@@ -1 +1 @@\n" \
+ "-a contents 2\n" \
+ "+a contents\n" \
+ "diff --git a/c/d b/c/d\n" \
+ "deleted file mode 100644\n" \
+ "index 297efb8..0000000\n" \
+ "--- a/c/d\n" \
+ "+++ /dev/null\n" \
+ "@@ -1 +0,0 @@\n" \
+ "-c/d contents\n"
+
+#define PATCH_DELETED_FILE_2_HUNKS_SHUFFLED \
+ "diff --git a/c/d b/c/d\n" \
+ "deleted file mode 100644\n" \
+ "index 297efb8..0000000\n" \
+ "--- a/c/d\n" \
+ "+++ /dev/null\n" \
+ "@@ -1 +0,0 @@\n" \
+ "-c/d contents\n" \
+ "diff --git a/a b/a\n" \
+ "index 7f129fd..af431f2 100644\n" \
+ "--- a/a\n" \
+ "+++ b/a\n" \
+ "@@ -1 +1 @@\n" \
+ "-a contents 2\n" \
+ "+a contents\n"
+
#define PATCH_SIMPLE_COMMIT \
"commit 15e119375018fba121cf58e02a9f17fe22df0df8\n" \
"Author: Edward Thomson <ethomson@edwardthomson.com>\n" \