{"thread":{"id":"36314","subject":"[PATCH v2 3/3] patch-id-test: test new --stable and --unstable flags","startedAt":"2014-03-28T12:30:11Z","lastAt":"2014-03-30T10:52:02Z","messageCount":8,"participants":["Michael S. Tsirkin","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":3},"messages":[{"id":"238004","messageId":"1396009159-2078-1-git-send-email-mst@redhat.com","threadId":"36314","inReplyTo":null,"subject":"[PATCH v2 1/3] patch-id: make it stable against hunk reordering","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-03-28T12:30:11Z","receivedAt":"2014-03-28T12:30:11Z","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).\nIn git, it is often done e.g. using the \"-O <orderfile>\" option,\nso supporting it better has value.\n\nHunks within file can be reordered manually provided\nthe same pathname can appear more than once in the input.\n\nChange patch-id behaviour making it stable against\nhunk reodering:\n\t- prepend header to each hunk (if not there)\n\t\tNote: POSIX requires patch to be robust against hunk reordering\n\t\tprovided each diff hunk has a header:\n\t\thttp://pubs.opengroup.org/onlinepubs/7908799/xcu/patch.html\n\t\tIf the patch file contains more than one patch, patch will attempt to\n\t\tapply each of them as if they came from separate patch files. (In this\n\t\tcase the name of the patch file must be determinable for each diff\n\t\tlisting.)\n\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 <mst@redhat.com>\n---\n\nChanges from v1: documented motivation for supporting\nhunk reordering (and not just file reordering).\nNo code changes.\n\nJunio, you didn't respond so I'm not sure whether I convinced\nyou that supporting hunk reordering within file has value.\nSo I kept this functionality around for now, if\nyou think I should drop this, please let me know explicitly.\nThanks, and sorry about being dense!\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":"238003","messageId":"1396009159-2078-2-git-send-email-mst@redhat.com","threadId":"36314","inReplyTo":"1396009159-2078-1-git-send-email-mst@redhat.com","subject":"[PATCH v2 2/3] patch-id: document new behaviour","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-03-28T12:30:15Z","receivedAt":"2014-03-28T12:30:15Z","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 <mst@redhat.com>\n---\n\nNo change from v1.\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' < <patch>\n+'git patch-id' [--stable | --unstable] < <patch>\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 <patch>::\n \tThe diff to create the ID of.\n \n-- \nMST\n"},{"id":"238002","messageId":"1396009159-2078-3-git-send-email-mst@redhat.com","threadId":"36314","inReplyTo":"1396009159-2078-1-git-send-email-mst@redhat.com","subject":"[PATCH v2 3/3] patch-id-test: test new --stable and --unstable flags","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-03-28T12:30:18Z","receivedAt":"2014-03-28T12:30:18Z","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 <mst@redhat.com>\n---\n\nchanges from v1:\n\tUse <<-\\EOF to address comment by Eric Sunshine\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..44dfd33 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 <<-\\EOF &&\n+\t\tbar\n+\t\tfoo\n+\t\tEOF\n+\tcat > foo-then-bar <<-\\EOF\n+\t\tfoo\n+\t\tbar\n+\t\tEOF\n '\n \n test_expect_success 'patch-id output is well-formed' '\n@@ -23,11 +38,33 @@ calc_patch_id () {\n \t\tsed \"s# .*##\" > 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":"238026","messageId":"xmqq38i2b2x2.fsf@gitster.dls.corp.google.com","threadId":"36314","inReplyTo":"1396009159-2078-1-git-send-email-mst@redhat.com","subject":"Re: [PATCH v2 1/3] patch-id: make it stable against hunk reordering","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-28T18:20:41Z","receivedAt":"2014-03-28T18:20:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Michael S. Tsirkin\" <mst@redhat.com> writes:\n\n> Patch id changes if you reorder hunks in a diff.\n\nReording \"files\" is fine, and as we discussed, having multiple\npatches that touch the same path is fine, but do not sound as if you\nare allowing to reorder \"hunks\" inside a single patch that touch a\nsingle file.\n\nThanks.\n"},{"id":"238038","messageId":"xmqqha6i9lle.fsf@gitster.dls.corp.google.com","threadId":"36314","inReplyTo":"1396009159-2078-1-git-send-email-mst@redhat.com","subject":"Re: [PATCH v2 1/3] patch-id: make it stable against hunk reordering","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-28T19:20:13Z","receivedAt":"2014-03-28T19:20:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Michael S. Tsirkin\" <mst@redhat.com> writes:\n\n> Patch id changes if you reorder hunks in a diff.\n> As the result is functionally equivalent, this is surprising to many\n> people.\n> In particular, reordering hunks is helpful to make patches\n> more readable (e.g. API header diff before implementation diff).\n> In git, it is often done e.g. using the \"-O <orderfile>\" option,\n> so supporting it better has value.\n>\n> Hunks within file can be reordered manually provided\n> the same pathname can appear more than once in the input.\n>\n> Change patch-id behaviour making it stable against\n> hunk reodering:\n> \t- prepend header to each hunk (if not there)\n> \t\tNote: POSIX requires patch to be robust against hunk reordering\n> \t\tprovided each diff hunk has a header:\n> \t\thttp://pubs.opengroup.org/onlinepubs/7908799/xcu/patch.html\n> \t\tIf the patch file contains more than one patch, patch will attempt to\n> \t\tapply each of them as if they came from separate patch files. (In this\n> \t\tcase the name of the patch file must be determinable for each diff\n> \t\tlisting.)\n>\n> \t- calculate SHA1 hash for each hunk separately\n> \t- sum all hashes to get patch id\n>\n> Add a new flag --unstable to get the historical behaviour.\n>\n> Add --stable which is a nop, for symmetry.\n>\n> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>\n> ---\n>\n> Changes from v1: documented motivation for supporting\n> hunk reordering (and not just file reordering).\n> No code changes.\n>\n> Junio, you didn't respond so I'm not sure whether I convinced\n> you that supporting hunk reordering within file has value.\n> So I kept this functionality around for now, if\n> you think I should drop this, please let me know explicitly.\n> Thanks, and sorry about being dense!\n\nThe motivation I read from the exchange was that:\n\n (1) we definitely want to support a mode that is stable with use of\n     \"diff -O\" (i.e. reordering patches per paths);\n\n (2) supporting a patch with swapped \"hunks\" does not have any\n     practical value.  When you have a patch to the same file F with\n     two hunks starting at lines 20 and 40, manually reordering\n     hunks to create a patch that shows the hunk that starts at line\n     40 and then the other hunk that starts at line 20 is not a\n     useful exercise;\n\n (3) but supporting a patch that touches the same path more than\n     once is in line with what \"patch\" and \"git apply\" after\n     7a07841c (git-apply: handle a patch that touches the same path\n     more than once better, 2008-06-27) do.\n\nIn other words, the goal of this change would be to give the same id\nfor all these three:\n\n    (A) straight-forward:\n\n        diff --git a/foo.c b/foo.c\n        --- a/foo.c\n        +++ b/foo.c\n        @@ -20,1 +20,1 @@\n\n        -foo\n        +bar\n\n        @@ -40,1 +40,1 @@\n\n        -frotz\n        +xyzzy\n\n    (B) as two patches concatenated together:\n\n        diff --git a/foo.c b/foo.c\n        --- a/foo.c\n        +++ b/foo.c\n        @@ -20,1 +20,1 @@\n\n        -foo\n        +bar\n\n        diff --git a/foo.c b/foo.c\n        --- a/foo.c\n        +++ b/foo.c\n        @@ -40,1 +40,1 @@\n\n        -frotz\n        +xyzzy\n\n    (C) the same as (B) but with a swapped order:\n\n        diff --git a/foo.c b/foo.c\n        --- a/foo.c\n        +++ b/foo.c\n        @@ -40,1 +40,1 @@\n\n        -frotz\n        +xyzzy\n        diff --git a/foo.c b/foo.c\n        --- a/foo.c\n        +++ b/foo.c\n        @@ -20,1 +20,1 @@\n\n        -foo\n        +bar\n\nAm I reading you correctly?\n\nIf that is the case, I think I can buy such a change.  It appears to\nme that in addition to changing the way the bytes form multiple\nhunks are hashed, it would need to hash the file-level headers\ntogether with each hunk when processing input (A) in order to make\nthe output consistent with the output for the latter two.\n\nAlternatively, you could hash the header for the same path only once\nwhen processing input like (B) or (C) and mix.  That would also give\nyou the same result as processing (A) in a straight-forward way.\n\n>  builtin/patch-id.c | 71 ++++++++++++++++++++++++++++++++++++++++++------------\n>  1 file changed, 55 insertions(+), 16 deletions(-)\n>\n> diff --git a/builtin/patch-id.c b/builtin/patch-id.c\n> index 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"},{"id":"238045","messageId":"xmqqzjka7z26.fsf@gitster.dls.corp.google.com","threadId":"36314","inReplyTo":"1396009159-2078-1-git-send-email-mst@redhat.com","subject":"Re: [PATCH v2 1/3] patch-id: make it stable against hunk reordering","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-28T22:12:17Z","receivedAt":"2014-03-28T22:12:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Michael S. Tsirkin\" <mst@redhat.com> writes:\n\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\nWas there a reason why bitwise xor is not sufficient for mixing\nthese two indenendent hashes?  If the 20-byte sums do not offer\nbenefit over that, the code for bitwise xor would be certainly be\nsimpler, I would imagine?\n\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\nWhat I read from these changes is that you do not do anything\nspecial about the per-file header, so two no overlapping patches\nwith a single hunk each that touches the same path concatenated\ntogether would not result in the same patch-id as a single-patch\nthat has the same two hunks.  Which would break your earlier 'Yes,\nreordering only the hunks will not make sense, but before each hunk\nyou could insert the same \"diff --git a/... b/...\" to make them a\nconcatenation of patches that touch the same file', I would think.\n\nIs that what we want to happen?  Or is my reading mistaken?\n"},{"id":"238047","messageId":"xmqqppl67ydr.fsf@gitster.dls.corp.google.com","threadId":"36314","inReplyTo":"xmqqzjka7z26.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 1/3] patch-id: make it stable against hunk reordering","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-28T22:26:56Z","receivedAt":"2014-03-28T22:26:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\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> What I read from these changes is that you do not do anything\n> special about the per-file header, so two no overlapping patches\n> with a single hunk each that touches the same path concatenated\n> together would not result in the same patch-id as a single-patch\n> that has the same two hunks.  Which would break your earlier 'Yes,\n> reordering only the hunks will not make sense, but before each hunk\n> you could insert the same \"diff --git a/... b/...\" to make them a\n> concatenation of patches that touch the same file', I would think.\n>\n> Is that what we want to happen?  Or is my reading mistaken?\n\nHeh, I was blind---copying into header_ctx and then restoring that\nin preparation for the next round is exactly for duplicating the\nper-file header sum to each and every hunk, so you are already doing\nthat.\n"},{"id":"238076","messageId":"20140330105202.GA24476@redhat.com","threadId":"36314","inReplyTo":"xmqqha6i9lle.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 1/3] patch-id: make it stable against hunk reordering","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-03-30T10:52:02Z","receivedAt":"2014-03-30T10:52:02Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Fri, Mar 28, 2014 at 12:20:13PM -0700, Junio C Hamano wrote:\n> \"Michael S. Tsirkin\" <mst@redhat.com> writes:\n> \n> > Patch id changes if you reorder hunks in a diff.\n> > As the result is functionally equivalent, this is surprising to many\n> > people.\n> > In particular, reordering hunks is helpful to make patches\n> > more readable (e.g. API header diff before implementation diff).\n> > In git, it is often done e.g. using the \"-O <orderfile>\" option,\n> > so supporting it better has value.\n> >\n> > Hunks within file can be reordered manually provided\n> > the same pathname can appear more than once in the input.\n> >\n> > Change patch-id behaviour making it stable against\n> > hunk reodering:\n> > \t- prepend header to each hunk (if not there)\n> > \t\tNote: POSIX requires patch to be robust against hunk reordering\n> > \t\tprovided each diff hunk has a header:\n> > \t\thttp://pubs.opengroup.org/onlinepubs/7908799/xcu/patch.html\n> > \t\tIf the patch file contains more than one patch, patch will attempt to\n> > \t\tapply each of them as if they came from separate patch files. (In this\n> > \t\tcase the name of the patch file must be determinable for each diff\n> > \t\tlisting.)\n> >\n> > \t- calculate SHA1 hash for each hunk separately\n> > \t- sum all hashes to get patch id\n> >\n> > Add a new flag --unstable to get the historical behaviour.\n> >\n> > Add --stable which is a nop, for symmetry.\n> >\n> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>\n> > ---\n> >\n> > Changes from v1: documented motivation for supporting\n> > hunk reordering (and not just file reordering).\n> > No code changes.\n> >\n> > Junio, you didn't respond so I'm not sure whether I convinced\n> > you that supporting hunk reordering within file has value.\n> > So I kept this functionality around for now, if\n> > you think I should drop this, please let me know explicitly.\n> > Thanks, and sorry about being dense!\n> \n> The motivation I read from the exchange was that:\n> \n>  (1) we definitely want to support a mode that is stable with use of\n>      \"diff -O\" (i.e. reordering patches per paths);\n> \n>  (2) supporting a patch with swapped \"hunks\" does not have any\n>      practical value.  When you have a patch to the same file F with\n>      two hunks starting at lines 20 and 40, manually reordering\n>      hunks to create a patch that shows the hunk that starts at line\n>      40 and then the other hunk that starts at line 20 is not a\n>      useful exercise;\n\nI agree, especially since the resulting patch can't be applied with\neither patch or git apply.\n\n>  (3) but supporting a patch that touches the same path more than\n>      once is in line with what \"patch\" and \"git apply\" after\n>      7a07841c (git-apply: handle a patch that touches the same path\n>      more than once better, 2008-06-27) do.\n> \n> In other words, the goal of this change would be to give the same id\n> for all these three:\n> \n>     (A) straight-forward:\n> \n>         diff --git a/foo.c b/foo.c\n>         --- a/foo.c\n>         +++ b/foo.c\n>         @@ -20,1 +20,1 @@\n> \n>         -foo\n>         +bar\n> \n>         @@ -40,1 +40,1 @@\n> \n>         -frotz\n>         +xyzzy\n> \n>     (B) as two patches concatenated together:\n> \n>         diff --git a/foo.c b/foo.c\n>         --- a/foo.c\n>         +++ b/foo.c\n>         @@ -20,1 +20,1 @@\n> \n>         -foo\n>         +bar\n> \n>         diff --git a/foo.c b/foo.c\n>         --- a/foo.c\n>         +++ b/foo.c\n>         @@ -40,1 +40,1 @@\n> \n>         -frotz\n>         +xyzzy\n> \n>     (C) the same as (B) but with a swapped order:\n> \n>         diff --git a/foo.c b/foo.c\n>         --- a/foo.c\n>         +++ b/foo.c\n>         @@ -40,1 +40,1 @@\n> \n>         -frotz\n>         +xyzzy\n>         diff --git a/foo.c b/foo.c\n>         --- a/foo.c\n>         +++ b/foo.c\n>         @@ -20,1 +20,1 @@\n> \n>         -foo\n>         +bar\n> \n> Am I reading you correctly?\n\nAbsolutely.\n\n> If that is the case, I think I can buy such a change.  It appears to\n> me that in addition to changing the way the bytes form multiple\n> hunks are hashed, it would need to hash the file-level headers\n> together with each hunk when processing input (A) in order to make\n> the output consistent with the output for the latter two.\n\nExactly. This is what this change attempts to do:\n+                                   if (hunks) {\n+                                           flush_one_hunk(result, &ctx);\n+                                           memcpy(&ctx, &header_ctx,\n+                                                  sizeof ctx);\n+                                   } else {\n+                                           /* Save ctx for next hunk.  */\n+                                           memcpy(&header_ctx, &ctx,\n+                                                  sizeof ctx);\n+                                   }\n\nI'll add more code comments to clarify the logic here.\n\n> Alternatively, you could hash the header for the same path only once\n> when processing input like (B) or (C) and mix.  That would also give\n> you the same result as processing (A) in a straight-forward way.\n\n\nYes, that is also possible.\nI think I slightly prefer the former way because this\nvariant gives the same result for\n         diff --git a/foo.c b/foo.c\n         --- a/foo.c\n         +++ b/foo.c\n         @@ -20,2 +20,3 @@\n \n         +bar\n \n         diff --git a/bar.c b/bar.c\n         --- a/bar.c\n         +++ b/bar.c\n         @@ -40,2 +40,3 @@\n \n         +foo\n\nand\n         diff --git a/bar.c b/bar.c\n         --- a/bar.c\n         +++ b/bar.c\n         @@ -20,2 +20,3 @@\n \n         +foo\n \n         diff --git a/foo.c b/foo.c\n         --- a/foo.c\n         +++ b/foo.c\n         @@ -40,2 +40,3 @@\n \n         +bar\n\n\nbut if you disagree, pls let me know - it's not\na strong preference.\n\n> >  builtin/patch-id.c | 71 ++++++++++++++++++++++++++++++++++++++++++------------\n> >  1 file changed, 55 insertions(+), 16 deletions(-)\n> >\n> > diff --git a/builtin/patch-id.c b/builtin/patch-id.c\n> > index 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"}]}