{"thread":{"id":"36300","subject":"[PATCH 2/3] patch-id: document new behaviour","startedAt":"2014-03-27T09:25:06Z","lastAt":"2014-03-28T00:25:04Z","messageCount":10,"participants":["Michael S. Tsirkin","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"237889","messageId":"1395912239-29663-1-git-send-email-mst@redhat.com","threadId":"36300","inReplyTo":null,"subject":"[PATCH 1/3] patch-id: make it stable against hunk reordering","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-03-27T09:25:06Z","receivedAt":"2014-03-27T09:25:06Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"Patch id changes if you reorder hunks in a diff.\nAs the result is functionally equivalent, this is surprising to many\npeople.\nIn particular, reordering hunks is helpful to make patches\nmore readable (e.g. API header diff before implementation diff).\n\nChange patch-id behaviour making it stable against\nhunk reodering:\n\t- prepend header to each hunk (if not there)\n\t- calculate SHA1 hash for each hunk separately\n\t- sum all hashes to get patch id\n\nAdd a new flag --unstable to get the historical behaviour.\n\nAdd --stable which is a nop, for symmetry.\n\nSigned-off-by: Michael S. Tsirkin \n---\n builtin/patch-id.c | 71 ++++++++++++++++++++++++++++++++++++++++++------------\n 1 file changed, 55 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/patch-id.c b/builtin/patch-id.c\nindex 3cfe02d..253ad87 100644\n--- a/builtin/patch-id.c\n+++ b/builtin/patch-id.c\n@@ -1,17 +1,14 @@\n #include \"builtin.h\"\n \n-static void flush_current_id(int patchlen, unsigned char *id, git_SHA_CTX *c)\n+static void flush_current_id(int patchlen, unsigned char *id, unsigned char *result)\n {\n-\tunsigned char result[20];\n \tchar name[50];\n \n \tif (!patchlen)\n \t\treturn;\n \n-\tgit_SHA1_Final(result, c);\n \tmemcpy(name, sha1_to_hex(id), 41);\n \tprintf(\"%s %s\\n\", sha1_to_hex(result), name);\n-\tgit_SHA1_Init(c);\n }\n \n static int remove_space(char *line)\n@@ -56,10 +53,30 @@ static int scan_hunk_header(const char *p, int *p_before, int *p_after)\n \treturn 1;\n }\n \n-static int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx, struct strbuf *line_buf)\n+static void flush_one_hunk(unsigned char *result, git_SHA_CTX *ctx)\n {\n-\tint patchlen = 0, found_next = 0;\n+\tunsigned char hash[20];\n+\tunsigned short carry = 0;\n+\tint i;\n+\n+\tgit_SHA1_Final(hash, ctx);\n+\tgit_SHA1_Init(ctx);\n+\t/* 20-byte sum, with carry */\n+\tfor (i = 0; i < 20; ++i) {\n+\t\tcarry += result[i] + hash[i];\n+\t\tresult[i] = carry;\n+\t\tcarry >>= 8;\n+\t}\n+}\n+static int get_one_patchid(unsigned char *next_sha1, unsigned char *result,\n+\t\t\t struct strbuf *line_buf, int stable)\n+{\n+\tint patchlen = 0, found_next = 0, hunks = 0;\n \tint before = -1, after = -1;\n+\tgit_SHA_CTX ctx, header_ctx;\n+\n+\tgit_SHA1_Init(&ctx);\n+\thashclr(result);\n \n \twhile (strbuf_getwholeline(line_buf, stdin, '\\n') != EOF) {\n \t\tchar *line = line_buf->buf;\n@@ -99,6 +116,18 @@ static int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx, struct st\n \t\t\tif (!memcmp(line, \"@@ -\", 4)) {\n \t\t\t\t/* Parse next hunk, but ignore line numbers. */\n \t\t\t\tscan_hunk_header(line, &before, &after);\n+\t\t\t\tif (stable) {\n+\t\t\t\t\tif (hunks) {\n+\t\t\t\t\t\tflush_one_hunk(result, &ctx);\n+\t\t\t\t\t\tmemcpy(&ctx, &header_ctx,\n+\t\t\t\t\t\t sizeof ctx);\n+\t\t\t\t\t} else {\n+\t\t\t\t\t\t/* Save ctx for next hunk. */\n+\t\t\t\t\t\tmemcpy(&header_ctx, &ctx,\n+\t\t\t\t\t\t sizeof ctx);\n+\t\t\t\t\t}\n+\t\t\t\t}\n+\t\t\t\thunks++;\n \t\t\t\tcontinue;\n \t\t\t}\n \n@@ -107,7 +136,10 @@ static int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx, struct st\n \t\t\t\tbreak;\n \n \t\t\t/* Else we're parsing another header. */\n+\t\t\tif (stable && hunks)\n+\t\t\t\tflush_one_hunk(result, &ctx);\n \t\t\tbefore = after = -1;\n+\t\t\thunks = 0;\n \t\t}\n \n \t\t/* If we get here, we're inside a hunk. */\n@@ -119,39 +151,46 @@ static int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx, struct st\n \t\t/* Compute the sha without whitespace */\n \t\tlen = remove_space(line);\n \t\tpatchlen += len;\n-\t\tgit_SHA1_Update(ctx, line, len);\n+\t\tgit_SHA1_Update(&ctx, line, len);\n \t}\n \n \tif (!found_next)\n \t\thashclr(next_sha1);\n \n+\tflush_one_hunk(result, &ctx);\n+\n \treturn patchlen;\n }\n \n-static void generate_id_list(void)\n+static void generate_id_list(int stable)\n {\n-\tunsigned char sha1[20], n[20];\n-\tgit_SHA_CTX ctx;\n+\tunsigned char sha1[20], n[20], result[20];\n \tint patchlen;\n \tstruct strbuf line_buf = STRBUF_INIT;\n \n-\tgit_SHA1_Init(&ctx);\n \thashclr(sha1);\n \twhile (!feof(stdin)) {\n-\t\tpatchlen = get_one_patchid(n, &ctx, &line_buf);\n-\t\tflush_current_id(patchlen, sha1, &ctx);\n+\t\tpatchlen = get_one_patchid(n, result, &line_buf, stable);\n+\t\tflush_current_id(patchlen, sha1, result);\n \t\thashcpy(sha1, n);\n \t}\n \tstrbuf_release(&line_buf);\n }\n \n-static const char patch_id_usage[] = \"git patch-id < patch\";\n+static const char patch_id_usage[] = \"git patch-id [--stable | --unstable] < patch\";\n \n int cmd_patch_id(int argc, const char **argv, const char *prefix)\n {\n-\tif (argc != 1)\n+\tint stable;\n+\tif (argc == 2 && !strcmp(argv[1], \"--stable\"))\n+\t\tstable = 1;\n+\telse if (argc == 2 && !strcmp(argv[1], \"--unstable\"))\n+\t\tstable = 0;\n+\telse if (argc == 1)\n+\t\tstable = 1;\n+\telse\n \t\tusage(patch_id_usage);\n \n-\tgenerate_id_list();\n+\tgenerate_id_list(stable);\n \treturn 0;\n }\n-- \nMST\n"},{"id":"237887","messageId":"1395912239-29663-2-git-send-email-mst@redhat.com","threadId":"36300","inReplyTo":"1395912239-29663-1-git-send-email-mst@redhat.com","subject":"[PATCH 2/3] patch-id: document new behaviour","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-03-27T09:25:14Z","receivedAt":"2014-03-27T09:25:14Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"Clarify that patch ID is now a sum of hashes, not a hash.\nDocument --stable and --unstable flags.\n\nSigned-off-by: Michael S. Tsirkin \n---\n Documentation/git-patch-id.txt | 21 ++++++++++++++++-----\n 1 file changed, 16 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-patch-id.txt b/Documentation/git-patch-id.txt\nindex 312c3b1..1bc6d52 100644\n--- a/Documentation/git-patch-id.txt\n+++ b/Documentation/git-patch-id.txt\n@@ -8,14 +8,14 @@ git-patch-id - Compute unique ID for a patch\n SYNOPSIS\n --------\n [verse]\n-'git patch-id' < \n+'git patch-id' [--stable | --unstable] < \n \n DESCRIPTION\n -----------\n-A \"patch ID\" is nothing but a SHA-1 of the diff associated with a patch, with\n-whitespace and line numbers ignored. As such, it's \"reasonably stable\", but at\n-the same time also reasonably unique, i.e., two patches that have the same \"patch\n-ID\" are almost guaranteed to be the same thing.\n+A \"patch ID\" is nothing but a sum of SHA-1 of the diff hunks associated with a\n+patch, with whitespace and line numbers ignored. As such, it's \"reasonably\n+stable\", but at the same time also reasonably unique, i.e., two patches that\n+have the same \"patch ID\" are almost guaranteed to be the same thing.\n \n IOW, you can use this thing to look for likely duplicate commits.\n \n@@ -27,6 +27,17 @@ This can be used to make a mapping from patch ID to commit ID.\n \n OPTIONS\n -------\n+\n+--stable::\n+\tUse a symmetrical sum of hashes, such that order of\n+\thunks in the diff does not affect the ID.\n+\tThis is the default.\n+\n+--unstable::\n+\tUse a non-symmetrical sum of hashes, such that order of\n+\thunks in the diff affects the ID.\n+\tThis was the default value for git 1.9 and older.\n+\n ::\n \tThe diff to create the ID of.\n \n-- \nMST\n"},{"id":"237888","messageId":"1395912239-29663-3-git-send-email-mst@redhat.com","threadId":"36300","inReplyTo":"1395912239-29663-1-git-send-email-mst@redhat.com","subject":"[PATCH 3/3] patch-id-test: test new --stable and --unstable flags","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-03-27T09:25:22Z","receivedAt":"2014-03-27T09:25:22Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"Verify that patch ID is now stable against hunk reordering.\n\nSigned-off-by: Michael S. Tsirkin \n---\n t/t4204-patch-id.sh | 68 +++++++++++++++++++++++++++++++++++++++++++++++++----\n 1 file changed, 63 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t4204-patch-id.sh b/t/t4204-patch-id.sh\nindex d2c930d..75f77ef 100755\n--- a/t/t4204-patch-id.sh\n+++ b/t/t4204-patch-id.sh\n@@ -5,12 +5,27 @@ test_description='git patch-id'\n . ./test-lib.sh\n \n test_expect_success 'setup' '\n-\ttest_commit initial foo a &&\n-\ttest_commit first foo b &&\n+\ttest_commit initial-foo foo a &&\n+\ttest_commit initial-bar bar a &&\n+\techo b > foo &&\n+\techo b > bar &&\n+\tgit commit -a -m first &&\n \tgit checkout -b same HEAD^ &&\n-\ttest_commit same-msg foo b &&\n+\techo b > foo &&\n+\techo b > bar &&\n+\tgit commit -a -m same-msg &&\n \tgit checkout -b notsame HEAD^ &&\n-\ttest_commit notsame-msg foo c\n+\techo c > foo &&\n+\techo c > bar &&\n+\tgit commit -a -m notsame-msg &&\n+\tcat > bar-then-foo < foo-then-bar < patch-id_\"$1\"\n }\n \n+calc_patch_id_unstable () {\n+\tgit patch-id --unstable |\n+\t\tsed \"s# .*##\" > patch-id_\"$1\"\n+}\n+\n+calc_patch_id_stable () {\n+\tgit patch-id --stable |\n+\t\tsed \"s# .*##\" > patch-id_\"$1\"\n+}\n+\n+\n get_patch_id () {\n-\tgit log -p -1 \"$1\" | git patch-id |\n+\tgit log -p -1 \"$1\" -O bar-then-foo -- | git patch-id |\n+\t\tsed \"s# .*##\" > patch-id_\"$1\"\n+}\n+\n+get_patch_id_stable () {\n+\tgit log -p -1 \"$1\" -O bar-then-foo | git patch-id --stable |\n+\t\tsed \"s# .*##\" > patch-id_\"$1\"\n+}\n+\n+get_patch_id_unstable () {\n+\tgit log -p -1 \"$1\" -O bar-then-foo | git patch-id --unstable |\n \t\tsed \"s# .*##\" > patch-id_\"$1\"\n }\n \n+\n test_expect_success 'patch-id detects equality' '\n \tget_patch_id master &&\n \tget_patch_id same &&\n@@ -56,6 +93,27 @@ test_expect_success 'whitespace is irrelevant in footer' '\n \ttest_cmp patch-id_master patch-id_same\n '\n \n+test_expect_success 'file order is irrelevant by default' '\n+\tget_patch_id master &&\n+\tgit checkout same &&\n+\tgit format-patch -1 --stdout -O foo-then-bar | calc_patch_id same &&\n+\ttest_cmp patch-id_master patch-id_same\n+'\n+\n+test_expect_success 'file order is irrelevant with --stable' '\n+\tget_patch_id_stable master &&\n+\tgit checkout same &&\n+\tgit format-patch -1 --stdout -O foo-then-bar | calc_patch_id_stable same &&\n+\ttest_cmp patch-id_master patch-id_same\n+'\n+\n+test_expect_success 'file order is relevant with --unstable' '\n+\tget_patch_id_unstable master &&\n+\tgit checkout same &&\n+\tgit format-patch -1 --stdout -O foo-then-bar | calc_patch_id_unstable notsame &&\n+\t! test_cmp patch-id_master patch-id_notsame\n+'\n+\n test_expect_success 'patch-id supports git-format-patch MIME output' '\n \tget_patch_id master &&\n \tgit checkout same &&\n-- \nMST\n"},{"id":"237914","messageId":"xmqqvbuzefy6.fsf@gitster.dls.corp.google.com","threadId":"36300","inReplyTo":"1395912239-29663-1-git-send-email-mst@redhat.com","subject":"Re: [PATCH 1/3] patch-id: make it stable against hunk reordering","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-27T16:58:41Z","receivedAt":"2014-03-27T16:58:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Michael S. Tsirkin\" writes:\n\n> Patch id changes if you reorder hunks in a diff.\n\nIf you reorder hunks, the patch should no longer apply [*1*], so a\nfeature to make patch-id stable across such move would have no\npractical use ;-), but I am guessing you meant something else.\n\nPerhaps this is about using \"-O \" option, even though you\nhappened to have implemented the id mixing at per-hunk level?\n\n\n[Footnote]\n\n*1* It has been a long time since I looked at the code, and I do not\nknow offhand if \"git apply\" has such a bug not to diagnose a hunk\nfor a file for an earlier part that comes later in its input stream\nafter seeing another hunk for the same file as a bug. If it does\nnot, perhaps we should.\n"},{"id":"237918","messageId":"20140327173455.GB2593@redhat.com","threadId":"36300","inReplyTo":"xmqqvbuzefy6.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/3] patch-id: make it stable against hunk reordering","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-03-27T17:34:55Z","receivedAt":"2014-03-27T17:34:55Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Thu, Mar 27, 2014 at 09:58:41AM -0700, Junio C Hamano wrote:\n> \"Michael S. Tsirkin\" writes:\n> \n> > Patch id changes if you reorder hunks in a diff.\n> \n> If you reorder hunks, the patch should no longer apply [*1*], so a\n> feature to make patch-id stable across such move would have no\n> practical use ;-), but I am guessing you meant something else.\n> \n> Perhaps this is about using \"-O \" option, even though you\n> happened to have implemented the id mixing at per-hunk level?\n\nYes.\n\n> \n> [Footnote]\n> \n> *1* It has been a long time since I looked at the code, and I do not\n> know offhand if \"git apply\" has such a bug not to diagnose a hunk\n> for a file for an earlier part that comes later in its input stream\n> after seeing another hunk for the same file as a bug. If it does\n> not, perhaps we should.\n\nHmm you are right.\nFor some reason I thought that it does work.\nI'll drop this part then, less code this way.\n\nThanks!\n\nAny more comments before I spin v2?\n\n-- \nMST\n"},{"id":"237921","messageId":"20140327175746.GA3853@redhat.com","threadId":"36300","inReplyTo":"xmqqvbuzefy6.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/3] patch-id: make it stable against hunk reordering","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-03-27T17:57:46Z","receivedAt":"2014-03-27T17:57:46Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Thu, Mar 27, 2014 at 09:58:41AM -0700, Junio C Hamano wrote:\n> \"Michael S. Tsirkin\" writes:\n> \n> > Patch id changes if you reorder hunks in a diff.\n> \n> If you reorder hunks, the patch should no longer apply [*1*], so a\n> feature to make patch-id stable across such move would have no\n> practical use ;-), but I am guessing you meant something else.\n> \n> Perhaps this is about using \"-O \" option, even though you\n> happened to have implemented the id mixing at per-hunk level?\n> \n> \n> [Footnote]\n> \n> *1* It has been a long time since I looked at the code, and I do not\n> know offhand if \"git apply\" has such a bug not to diagnose a hunk\n> for a file for an earlier part that comes later in its input stream\n> after seeing another hunk for the same file as a bug. If it does\n> not, perhaps we should.\n\nI started to remove that code, but then I recalled why I did it like\nthis. There is a good reason. Yes, you can't simply reorder hunks just\nlike this. But you can get the same effect by prefixing the header:\n\n\n\n--- x.txt\t2014-03-27 19:31:18.166115449 +0200\n+++ y.txt\t2014-03-27 19:31:46.731116998 +0200\n@@ -30,8 +31,6 @@ a\n a\n a\n a\n-a\n-b\n b\n b\n b\n@@ -60,6 +59,7 @@ b\n b\n b\n b\n+Y\n b\n b\n b\n--- x.txt\t2014-03-27 19:31:18.166115449 +0200\n+++ y.txt\t2014-03-27 19:31:46.731116998 +0200\n@@ -11,6 +11,7 @@ a\n a\n a\n a\n+X\n a\n a\n a\n\n\nIs equivalent to \n\n--- x.txt\t2014-03-27 19:31:18.166115449 +0200\n+++ y.txt\t2014-03-27 19:31:46.731116998 +0200\n@@ -30,8 +31,6 @@ a\n a\n a\n a\n-a\n-b\n b\n b\n b\n@@ -60,6 +59,7 @@ b\n b\n b\n b\n+Y\n b\n b\n b\n--- x.txt\t2014-03-27 19:31:18.166115449 +0200\n+++ y.txt\t2014-03-27 19:31:46.731116998 +0200\n@@ -11,6 +11,7 @@ a\n a\n a\n a\n+X\n a\n a\n a\n\n\nAnd this works fine with regular tools like patch\nso I think it should work for git too, anything else\nwould be surprising.\n\n\n-- \nMST\n"},{"id":"237922","messageId":"xmqqfvm3ecxp.fsf@gitster.dls.corp.google.com","threadId":"36300","inReplyTo":"20140327175746.GA3853@redhat.com","subject":"Re: [PATCH 1/3] patch-id: make it stable against hunk reordering","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-27T18:03:46Z","receivedAt":"2014-03-27T18:03:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Michael S. Tsirkin\" writes:\n\n> I started to remove that code, but then I recalled why I did it like\n> this. There is a good reason. Yes, you can't simply reorder hunks just\n> like this. But you can get the same effect by prefixing the header:\n\nYes, that is one of the things I personally have on the chopping\nblock. Having to deal with more than occurrences of the same\npathname in the input made things in builtin/apply.c unnecessarily\ncomplex and I do not see a real gain for being able to concatenate\ntwo patches and feed it into a single \"git apply\" invocation.\n"},{"id":"237924","messageId":"20140327183917.GA3980@redhat.com","threadId":"36300","inReplyTo":"xmqqfvm3ecxp.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/3] patch-id: make it stable against hunk reordering","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-03-27T18:39:17Z","receivedAt":"2014-03-27T18:39:17Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Thu, Mar 27, 2014 at 11:03:46AM -0700, Junio C Hamano wrote:\n> \"Michael S. Tsirkin\" writes:\n> \n> > I started to remove that code, but then I recalled why I did it like\n> > this. There is a good reason. Yes, you can't simply reorder hunks just\n> > like this. But you can get the same effect by prefixing the header:\n> \n> Yes, that is one of the things I personally have on the chopping\n> block. Having to deal with more than occurrences of the same\n> pathname in the input made things in builtin/apply.c unnecessarily\n> complex and I do not see a real gain for being able to concatenate\n> two patches and feed it into a single \"git apply\" invocation.\n\nWell - I expect that this will surprise some people: gnu\npatch accepts this, and it's a natural assumption\nthat it works. There could be tools producing such diffs, too.\n\nAnyway - we can drop this from patch-id and git apply at\nthe same time?\n\n-- \nMST\n"},{"id":"237925","messageId":"20140327184515.GA4056@redhat.com","threadId":"36300","inReplyTo":"20140327183917.GA3980@redhat.com","subject":"Re: [PATCH 1/3] patch-id: make it stable against hunk reordering","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-03-27T18:45:15Z","receivedAt":"2014-03-27T18:45:15Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Thu, Mar 27, 2014 at 08:39:17PM +0200, Michael S. Tsirkin wrote:\n> On Thu, Mar 27, 2014 at 11:03:46AM -0700, Junio C Hamano wrote:\n> > \"Michael S. Tsirkin\" writes:\n> > \n> > > I started to remove that code, but then I recalled why I did it like\n> > > this. There is a good reason. Yes, you can't simply reorder hunks just\n> > > like this. But you can get the same effect by prefixing the header:\n> > \n> > Yes, that is one of the things I personally have on the chopping\n> > block. Having to deal with more than occurrences of the same\n> > pathname in the input made things in builtin/apply.c unnecessarily\n> > complex and I do not see a real gain for being able to concatenate\n> > two patches and feed it into a single \"git apply\" invocation.\n> \n> Well - I expect that this will surprise some people: gnu\n> patch accepts this, and it's a natural assumption\n> that it works. There could be tools producing such diffs, too.\n\nThis behaviour also seems to be explicitly required by POSIX:\n\nhttp://pubs.opengroup.org/onlinepubs/7908799/xcu/patch.html\nIf the patch file contains more than one patch, patch will attempt to\napply each of them as if they came from separate patch files. (In this\ncase the name of the patch file must be determinable for each diff\nlisting.) \n\nIt's better to stick to standards even if it does require\na bit more code, isn't it?\n\n> Anyway - we can drop this from patch-id and git apply at\n> the same time?\n> \n> -- \n> MST\n"},{"id":"237961","messageId":"CAPig+cQ4=joCcjyhOLGAiTV2WCLk7a1Vb5R=wNCLwX1gTp8V9A@mail.gmail.com","threadId":"36300","inReplyTo":"1395912239-29663-3-git-send-email-mst@redhat.com","subject":"Re: [PATCH 3/3] patch-id-test: test new --stable and --unstable flags","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-03-28T00:25:04Z","receivedAt":"2014-03-28T00:25:04Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Mar 27, 2014 at 5:25 AM, Michael S. Tsirkin wrote:\n> Verify that patch ID is now stable against hunk reordering.\n>\n> Signed-off-by: Michael S. Tsirkin \n> ---\n> t/t4204-patch-id.sh | 68 +++++++++++++++++++++++++++++++++++++++++++++++++----\n> 1 file changed, 63 insertions(+), 5 deletions(-)\n>\n> diff --git a/t/t4204-patch-id.sh b/t/t4204-patch-id.sh\n> index d2c930d..75f77ef 100755\n> --- a/t/t4204-patch-id.sh\n> +++ b/t/t4204-patch-id.sh\n> @@ -5,12 +5,27 @@ test_description='git patch-id'\n> . ./test-lib.sh\n>\n> test_expect_success 'setup' '\n> - test_commit initial foo a &&\n> - test_commit first foo b &&\n> + test_commit initial-foo foo a &&\n> + test_commit initial-bar bar a &&\n> + echo b > foo &&\n> + echo b > bar &&\n> + git commit -a -m first &&\n> git checkout -b same HEAD^ &&\n> - test_commit same-msg foo b &&\n> + echo b > foo &&\n> + echo b > bar &&\n> + git commit -a -m same-msg &&\n> git checkout -b notsame HEAD^ &&\n> - test_commit notsame-msg foo c\n> + echo c > foo &&\n> + echo c > bar &&\n> + git commit -a -m notsame-msg &&\n> + cat > bar-then-foo < +bar\n> +foo\n> +EOF\n> + cat > foo-then-bar < +foo\n> +bar\n> +EOF\n> '\n>\n> test_expect_success 'patch-id output is well-formed' '\n> @@ -23,11 +38,33 @@ calc_patch_id () {\n> sed \"s# .*##\" > patch-id_\"$1\"\n> }\n>\n> +calc_patch_id_unstable () {\n> + git patch-id --unstable |\n> + sed \"s# .*##\" > patch-id_\"$1\"\n> +}\n> +\n> +calc_patch_id_stable () {\n> + git patch-id --stable |\n> + sed \"s# .*##\" > patch-id_\"$1\"\n> +}\n> +\n> +\n> get_patch_id () {\n> - git log -p -1 \"$1\" | git patch-id |\n> + git log -p -1 \"$1\" -O bar-then-foo -- | git patch-id |\n> + sed \"s# .*##\" > patch-id_\"$1\"\n> +}\n> +\n> +get_patch_id_stable () {\n> + git log -p -1 \"$1\" -O bar-then-foo | git patch-id --stable |\n> + sed \"s# .*##\" > patch-id_\"$1\"\n> +}\n> +\n> +get_patch_id_unstable () {\n> + git log -p -1 \"$1\" -O bar-then-foo | git patch-id --unstable |\n> sed \"s# .*##\" > patch-id_\"$1\"\n> }\n>\n> +\n> test_expect_success 'patch-id detects equality' '\n> get_patch_id master &&\n> get_patch_id same &&\n> @@ -56,6 +93,27 @@ test_expect_success 'whitespace is irrelevant in footer' '\n> test_cmp patch-id_master patch-id_same\n> '\n>\n> +test_expect_success 'file order is irrelevant by default' '\n> + get_patch_id master &&\n> + git checkout same &&\n> + git format-patch -1 --stdout -O foo-then-bar | calc_patch_id same &&\n> + test_cmp patch-id_master patch-id_same\n> +'\n> +\n> +test_expect_success 'file order is irrelevant with --stable' '\n> + get_patch_id_stable master &&\n> + git checkout same &&\n> + git format-patch -1 --stdout -O foo-then-bar | calc_patch_id_stable same &&\n> + test_cmp patch-id_master patch-id_same\n> +'\n> +\n> +test_expect_success 'file order is relevant with --unstable' '\n> + get_patch_id_unstable master &&\n> + git checkout same &&\n> + git format-patch -1 --stdout -O foo-then-bar | calc_patch_id_unstable notsame &&\n> + ! test_cmp patch-id_master patch-id_notsame\n> +'\n> +\n> test_expect_success 'patch-id supports git-format-patch MIME output' '\n> get_patch_id master &&\n> git checkout same &&\n> --\n> MST\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"}]}