{"thread":{"id":"36318","subject":"[PATCH v3 1/3] patch-id: make it stable against hunk reordering","startedAt":"2014-03-30T18:09:22Z","lastAt":"2014-04-03T15:42:46Z","messageCount":15,"participants":["Michael S. Tsirkin","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":3},"messages":[{"id":"238101","messageId":"1396202583-2572-1-git-send-email-mst@redhat.com","threadId":"36318","inReplyTo":null,"subject":"[PATCH v3 1/3] patch-id: make it stable against hunk reordering","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-03-30T18:09:22Z","receivedAt":"2014-03-30T18:09:22Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"Patch id changes if users\n1. reorder file diffs that make up a patch\nor\n2. split a patch up to multiple diffs that touch the same path\n(keeping hunks within a single diff ordered to make patch valid).\n\nAs the result is functionally equivalent, a different patch id is\nsurprising to many users.\nIn particular, reordering files using diff -O is helpful to make patches\nmore readable (e.g. API header diff before implementation diff).\n\nChange patch-id behaviour making it stable against these two kinds\nof patch change:\n1. calculate SHA1 hash for each hunk separately and sum all hashes\n(using a symmetrical sum) to get patch id\n2. hash the file-level headers together with each hunk (not just the\nfirst hunk)\n\nWe use a 20byte sum and not xor - since xor would give 0 output\nfor patches that have two identical diffs, which isn't all that\nunlikely (e.g. append the same line in two places).\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 v2:\n\tseveral bugfixes\nchanges from v1:\n\thanges from v1: documented motivation for supporting\n\tdiff splitting (and not just file reordering).\n\tNo code changes.\n\n builtin/patch-id.c | 72 ++++++++++++++++++++++++++++++++++++++++++------------\n 1 file changed, 56 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/patch-id.c b/builtin/patch-id.c\nindex 3cfe02d..7fd7007 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,31 @@ 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+\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@@ -98,7 +116,19 @@ static int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx, struct st\n \t\tif (before == 0 && after == 0) {\n \t\t\tif (!memcmp(line, \"@@ -\", 4)) {\n \t\t\t\t/* Parse next hunk, but ignore line numbers.  */\n+\t\t\t\tif (stable) {\n+\t\t\t\t\t/* Hash the file-level headers together with each hunk. */\n+\t\t\t\t\tif (hunks) {\n+\t\t\t\t\t\tflush_one_hunk(result, &ctx);\n+\t\t\t\t\t\t/* Prepend saved header ctx for next hunk.  */\n+\t\t\t\t\t\tmemcpy(&ctx, &header_ctx, sizeof ctx);\n+\t\t\t\t\t} else {\n+\t\t\t\t\t\t/* Save header ctx for next hunk.  */\n+\t\t\t\t\t\tmemcpy(&header_ctx, &ctx, sizeof ctx);\n+\t\t\t\t\t}\n+\t\t\t\t}\n \t\t\t\tscan_hunk_header(line, &before, &after);\n+\t\t\t\thunks++;\n \t\t\t\tcontinue;\n \t\t\t}\n \n@@ -107,7 +137,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 +152,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":"238102","messageId":"1396202583-2572-2-git-send-email-mst@redhat.com","threadId":"36318","inReplyTo":"1396202583-2572-1-git-send-email-mst@redhat.com","subject":"[PATCH v3 2/3] patch-id: document new behaviour","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-03-30T18:09:26Z","receivedAt":"2014-03-30T18:09:26Z","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\nchanges from v2:\n\texplicitly list the kinds of changes against which patch ID is stable\n\n Documentation/git-patch-id.txt | 23 ++++++++++++++++++-----\n 1 file changed, 18 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-patch-id.txt b/Documentation/git-patch-id.txt\nindex 312c3b1..30923e0 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,19 @@ 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 as the patch ID.\n+\tWith this option, reordering file diffs that make up a patch or\n+\tsplitting a diff up to multiple diffs that touch the same path\n+\tdoes not affect the ID.\n+\tThis is the default.\n+\n+--unstable::\n+\tUse a non-symmetrical sum of hashes, such that reordering\n+\tor splitting the patch does affect 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":"238103","messageId":"1396202583-2572-3-git-send-email-mst@redhat.com","threadId":"36318","inReplyTo":"1396202583-2572-1-git-send-email-mst@redhat.com","subject":"[PATCH v3 3/3] patch-id-test: test --stable and --unstable flags","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-03-30T18:09:29Z","receivedAt":"2014-03-30T18:09:29Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"Verify that patch ID is now stable against diff split and reordering.\n\nSigned-off-by: Michael S. Tsirkin <mst@redhat.com>\n---\n\nChanges from v2:\n\tadded test to verify patch ID is stable against diff splitting\n\n t/t4204-patch-id.sh | 117 ++++++++++++++++++++++++++++++++++++++++++++++++----\n 1 file changed, 109 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t4204-patch-id.sh b/t/t4204-patch-id.sh\nindex d2c930d..1679714 100755\n--- a/t/t4204-patch-id.sh\n+++ b/t/t4204-patch-id.sh\n@@ -5,12 +5,46 @@ 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-\tgit checkout -b same HEAD^ &&\n-\ttest_commit same-msg foo b &&\n-\tgit checkout -b notsame HEAD^ &&\n-\ttest_commit notsame-msg foo c\n+\tcat > a <<-\\EOF &&\n+\t\ta\n+\t\ta\n+\t\ta\n+\t\ta\n+\t\ta\n+\t\ta\n+\t\ta\n+\t\ta\n+\t\tEOF\n+\t(cat a; echo b) > ab &&\n+\t(echo d; cat a; echo b) > dab &&\n+\tcp a foo &&\n+\tcp a bar &&\n+\tgit add foo bar &&\n+\tgit commit -a -m initial &&\n+\tcp ab foo &&\n+\tcp ab bar &&\n+\tgit commit -a -m first &&\n+\tgit checkout -b same master &&\n+\tgit commit --amend -m same-msg &&\n+\tgit checkout -b notsame master &&\n+\techo c > foo &&\n+\techo c > bar &&\n+\tgit commit --amend -a -m notsame-msg &&\n+\tgit checkout -b split master &&\n+\tcp dab foo &&\n+\tcp dab bar &&\n+\tgit commit -a -m split &&\n+\tgit checkout -b merged master &&\n+\tgit checkout split -- foo bar &&\n+\tgit commit --amend -a -m merged &&\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 +57,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@@ -52,10 +108,55 @@ test_expect_success 'patch-id supports git-format-patch output' '\n test_expect_success 'whitespace is irrelevant in footer' '\n \tget_patch_id master &&\n \tgit checkout same &&\n-\tgit format-patch -1 --stdout | sed \"s/ \\$//\" | calc_patch_id same &&\n+\tgit format-patch -1 --stdout | sed \"s/ \\$//\" |\n+\t\tcalc_patch_id same &&\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 |\n+\t\tcalc_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 |\n+\t\tcalc_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 'splitting patch does not affect id by default' '\n+\tget_patch_id merged &&\n+\t(git log -p -1 -O foo-then-bar split~1; git diff split~1..split) |\n+\t\tcalc_patch_id split &&\n+\ttest_cmp patch-id_merged patch-id_split\n+'\n+\n+test_expect_success 'splitting patch does not affect id with --stable' '\n+\tget_patch_id_stable merged &&\n+\t(git log -p -1 -O foo-then-bar split~1; git diff split~1..split) |\n+\t\tcalc_patch_id_stable split &&\n+\ttest_cmp patch-id_merged patch-id_split\n+'\n+\n+test_expect_success 'splitting patch affects id with --unstable' '\n+\tget_patch_id_unstable merged &&\n+\t(git log -p -1 -O foo-then-bar split~1; git diff split~1..split) |\n+\t\tcalc_patch_id_unstable split &&\n+\t! test_cmp patch-id_merged patch-id_split\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":"238117","messageId":"xmqqk3ba6yg9.fsf@gitster.dls.corp.google.com","threadId":"36318","inReplyTo":"1396202583-2572-1-git-send-email-mst@redhat.com","subject":"Re: [PATCH v3 1/3] patch-id: make it stable against hunk reordering","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-31T17:59:50Z","receivedAt":"2014-03-31T17:59:50Z","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 users\n> 1. reorder file diffs that make up a patch\n> or\n> 2. split a patch up to multiple diffs that touch the same path\n> (keeping hunks within a single diff ordered to make patch valid).\n>\n> As the result is functionally equivalent, a different patch id is\n> surprising to many users.\n> In particular, reordering files using diff -O is helpful to make patches\n> more readable (e.g. API header diff before implementation diff).\n>\n> Change patch-id behaviour making it stable against these two kinds\n> of patch change:\n> 1. calculate SHA1 hash for each hunk separately and sum all hashes\n> (using a symmetrical sum) to get patch id\n> 2. hash the file-level headers together with each hunk (not just the\n> first hunk)\n>\n> We use a 20byte sum and not xor - since xor would give 0 output\n> for patches that have two identical diffs, which isn't all that\n> unlikely (e.g. append the same line in two places).\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 v2:\n> \tseveral bugfixes\n> changes from v1:\n> \thanges from v1: documented motivation for supporting\n> \tdiff splitting (and not just file reordering).\n> \tNo code changes.\n>\n>  builtin/patch-id.c | 72 ++++++++++++++++++++++++++++++++++++++++++------------\n>  1 file changed, 56 insertions(+), 16 deletions(-)\n\nDoes this have to interact or be consistent with patch-ids.c which\nis the real patch-id machinery used to filter like-changes out by\n\"cherry-pick\" and \"log --cherry-pick\"?\n\nThis series opens a very interesting opportunity by making it\npossible to introduce the equivalence between two patches that touch\nthe same file and a single patch that concatenates hunks from these\ntwo patches.\n\nOne example I am wondering about is perhaps this could be used to\ndetect two branches, both sides with many patches cherry-picked from\nthe same source, but some patches squashed together on one branch\nbut not on the other.  It would be very nice if you can detect that\ntwo sets of patches are equivalent taken as a whole in such a\nsituation while rebasing one on top of the other.\n\nAnother example is that another mode that gives a set of broken-up\npatch-ids for each hunk contained in the input.  Suppose there is a\npatch that is only meant to be used on the proprietary fork of an\nopen source project, and the project releases the open source\nportion by cherry-picking topics from the development tree used for\nthe proprietary \"trunk\".  The integration service of such a project\nused to prepare the open source branch may want to have a\npre-receive hook that says \"do not merge any commit to cause this\nthis hunk appear in the result, no matter what other changes the\npatches in the commit may bring in\", and broken-down patch-ids\n(e.g. \"diff HEAD...$commit | patch-id --individual\") may be an\ningredient to implement such a hook.  There may be interesting\napplications other than such a \"broken-down patch-ids\" that can be\nbased on the enhancement you are presenting here.\n"},{"id":"238128","messageId":"20140331190444.GA12208@redhat.com","threadId":"36318","inReplyTo":"xmqqk3ba6yg9.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 1/3] patch-id: make it stable against hunk reordering","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-03-31T19:04:44Z","receivedAt":"2014-03-31T19:04:44Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Mon, Mar 31, 2014 at 10:59:50AM -0700, Junio C Hamano wrote:\n> \"Michael S. Tsirkin\" <mst@redhat.com> writes:\n> \n> > Patch id changes if users\n> > 1. reorder file diffs that make up a patch\n> > or\n> > 2. split a patch up to multiple diffs that touch the same path\n> > (keeping hunks within a single diff ordered to make patch valid).\n> >\n> > As the result is functionally equivalent, a different patch id is\n> > surprising to many users.\n> > In particular, reordering files using diff -O is helpful to make patches\n> > more readable (e.g. API header diff before implementation diff).\n> >\n> > Change patch-id behaviour making it stable against these two kinds\n> > of patch change:\n> > 1. calculate SHA1 hash for each hunk separately and sum all hashes\n> > (using a symmetrical sum) to get patch id\n> > 2. hash the file-level headers together with each hunk (not just the\n> > first hunk)\n> >\n> > We use a 20byte sum and not xor - since xor would give 0 output\n> > for patches that have two identical diffs, which isn't all that\n> > unlikely (e.g. append the same line in two places).\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 v2:\n> > \tseveral bugfixes\n> > changes from v1:\n> > \thanges from v1: documented motivation for supporting\n> > \tdiff splitting (and not just file reordering).\n> > \tNo code changes.\n> >\n> >  builtin/patch-id.c | 72 ++++++++++++++++++++++++++++++++++++++++++------------\n> >  1 file changed, 56 insertions(+), 16 deletions(-)\n> \n> Does this have to interact or be consistent with patch-ids.c which\n> is the real patch-id machinery used to filter like-changes out by\n> \"cherry-pick\" and \"log --cherry-pick\"?\n\nI don't know off-hand.\n\nSpecifically, this is diff_flush_patch_id and in diff.c, isn't it?\n\n> This series opens a very interesting opportunity by making it\n> possible to introduce the equivalence between two patches that touch\n> the same file and a single patch that concatenates hunks from these\n> two patches.\n> \n> One example I am wondering about is perhaps this could be used to\n> detect two branches, both sides with many patches cherry-picked from\n> the same source, but some patches squashed together on one branch\n> but not on the other.  It would be very nice if you can detect that\n> two sets of patches are equivalent taken as a whole in such a\n> situation while rebasing one on top of the other.\n> \n> Another example is that another mode that gives a set of broken-up\n> patch-ids for each hunk contained in the input.  Suppose there is a\n> patch that is only meant to be used on the proprietary fork of an\n> open source project, and the project releases the open source\n> portion by cherry-picking topics from the development tree used for\n> the proprietary \"trunk\".  The integration service of such a project\n> used to prepare the open source branch may want to have a\n> pre-receive hook that says \"do not merge any commit to cause this\n> this hunk appear in the result, no matter what other changes the\n> patches in the commit may bring in\", and broken-down patch-ids\n> (e.g. \"diff HEAD...$commit | patch-id --individual\") may be an\n> ingredient to implement such a hook.  There may be interesting\n> applications other than such a \"broken-down patch-ids\" that can be\n> based on the enhancement you are presenting here.\n\nOK sure, I can tweak that to use the same algorithm if desired,\nthough it does look like an unrelated enhancement to me.\nAgree?\n"},{"id":"238129","messageId":"xmqqmwg65gp7.fsf@gitster.dls.corp.google.com","threadId":"36318","inReplyTo":"1396202583-2572-2-git-send-email-mst@redhat.com","subject":"Re: [PATCH v3 2/3] patch-id: document new behaviour","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-31T19:08:36Z","receivedAt":"2014-03-31T19:08:36Z","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> Clarify that patch ID is now a sum of hashes, not a hash.\n> Document --stable and --unstable flags.\n>\n> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>\n> ---\n>\n> changes from v2:\n> \texplicitly list the kinds of changes against which patch ID is stable\n>\n>  Documentation/git-patch-id.txt | 23 ++++++++++++++++++-----\n>  1 file changed, 18 insertions(+), 5 deletions(-)\n>\n> diff --git a/Documentation/git-patch-id.txt b/Documentation/git-patch-id.txt\n> index 312c3b1..30923e0 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\nThanks.  It seems taht we are fairly inconsistent when writing\nalternatives on the SYNOPSIS line.  A small minority seems to spell\nthe above as \"[--stable|--unstable]\", which may want to be fixed\n(outside the context of this series, of course).\n\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\nPerhaps \"nothing but\" can go by now?\n\n>  \n>  IOW, you can use this thing to look for likely duplicate commits.\n>  \n> @@ -27,6 +27,19 @@ 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 as the patch ID.\n> +\tWith this option, reordering file diffs that make up a patch or\n> +\tsplitting a diff up to multiple diffs that touch the same path\n> +\tdoes not affect the ID.\n> +\tThis is the default.\n> +\n> +--unstable::\n> +\tUse a non-symmetrical sum of hashes, such that reordering\n> +\tor splitting the patch does affect the ID.\n> +\tThis was the default value for git 1.9 and older.\n\nI am not sure if swapping the default in this series is a wise\ndecision.  We typically introduce a new shiny toy to play with in a\nrelease and then later when the shiny toy proves to be useful, start\nto think about changing the default, but not before.\n\n>  <patch>::\n>  \tThe diff to create the ID of.\n"},{"id":"238131","messageId":"20140331192604.GF12208@redhat.com","threadId":"36318","inReplyTo":"xmqqmwg65gp7.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 2/3] patch-id: document new behaviour","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-03-31T19:26:04Z","receivedAt":"2014-03-31T19:26:04Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Mon, Mar 31, 2014 at 12:08:36PM -0700, Junio C Hamano wrote:\n> \"Michael S. Tsirkin\" <mst@redhat.com> writes:\n> \n> > Clarify that patch ID is now a sum of hashes, not a hash.\n> > Document --stable and --unstable flags.\n> >\n> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>\n> > ---\n> >\n> > changes from v2:\n> > \texplicitly list the kinds of changes against which patch ID is stable\n> >\n> >  Documentation/git-patch-id.txt | 23 ++++++++++++++++++-----\n> >  1 file changed, 18 insertions(+), 5 deletions(-)\n> >\n> > diff --git a/Documentation/git-patch-id.txt b/Documentation/git-patch-id.txt\n> > index 312c3b1..30923e0 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> Thanks.  It seems taht we are fairly inconsistent when writing\n> alternatives on the SYNOPSIS line.  A small minority seems to spell\n> the above as \"[--stable|--unstable]\", which may want to be fixed\n> (outside the context of this series, of course).\n> \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> Perhaps \"nothing but\" can go by now?\n\nSure. I was also wondering whether we should document the fact that we\nare using SHA-1 internally, or just say \"hashes\".\nIn the end people can always read the source to find out so ...\n\n> >  \n> >  IOW, you can use this thing to look for likely duplicate commits.\n> >  \n> > @@ -27,6 +27,19 @@ 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 as the patch ID.\n> > +\tWith this option, reordering file diffs that make up a patch or\n> > +\tsplitting a diff up to multiple diffs that touch the same path\n> > +\tdoes not affect the ID.\n> > +\tThis is the default.\n> > +\n> > +--unstable::\n> > +\tUse a non-symmetrical sum of hashes, such that reordering\n> > +\tor splitting the patch does affect the ID.\n> > +\tThis was the default value for git 1.9 and older.\n> \n> I am not sure if swapping the default in this series is a wise\n> decision.  We typically introduce a new shiny toy to play with in a\n> release and then later when the shiny toy proves to be useful, start\n> to think about changing the default, but not before.\n\nWell I would claim that this is really a bugfix: --unstable\nis here really just in case someone relies on the broken\nbehaviour.\n\nThe hash used is mostly an internal implementation detail, isn't it?\nOne of my motivators is to upstream\n\t[PATCH] diff: add a config option to control orderfile\nso that I can use ordered diffs more easily, sending them to people and\nnot worrying about broken patch IDs.\nIf people have to remember to use --stable, that's of course harder.\n\nIf we keep+--unstable as default, I'd say we'll need a configuration\noption to enable --stable: I can at least tell people to enable that.\nWe'll also need some way for people to discover what the default was.\n\nAs it is - it's simple: if --stable is there, it's the default.\n\n> >  <patch>::\n> >  \tThe diff to create the ID of.\n"},{"id":"238132","messageId":"xmqqioqu5fr3.fsf@gitster.dls.corp.google.com","threadId":"36318","inReplyTo":"1396202583-2572-3-git-send-email-mst@redhat.com","subject":"Re: [PATCH v3 3/3] patch-id-test: test --stable and --unstable flags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-31T19:29:04Z","receivedAt":"2014-03-31T19:29:04Z","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> Verify that patch ID is now stable against diff split and reordering.\n>\n> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>\n> ---\n>\n> Changes from v2:\n> \tadded test to verify patch ID is stable against diff splitting\n>\n>  t/t4204-patch-id.sh | 117 ++++++++++++++++++++++++++++++++++++++++++++++++----\n>  1 file changed, 109 insertions(+), 8 deletions(-)\n>\n> diff --git a/t/t4204-patch-id.sh b/t/t4204-patch-id.sh\n> index d2c930d..1679714 100755\n> --- a/t/t4204-patch-id.sh\n> +++ b/t/t4204-patch-id.sh\n> @@ -5,12 +5,46 @@ 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> -\tgit checkout -b same HEAD^ &&\n> -\ttest_commit same-msg foo b &&\n> -\tgit checkout -b notsame HEAD^ &&\n> -\ttest_commit notsame-msg foo c\n> +\tcat > a <<-\\EOF &&\n\nPlease do not add an extra space between the redirection operator\n(e.g. \">\", \"2>\", etc.) and its operand.  The same style violations\neverywhere in this patch.\n\n> +\t\ta\n> +\t\ta\n> +\t\ta\n> +\t\ta\n> +\t\ta\n> +\t\ta\n> +\t\ta\n> +\t\ta\n> +\t\tEOF\n\nPlease align EOF with the cat that receives it, and pretend that the\ncolumn the head of the cat is at is the leftmost column for the\npayload, when writing <<- here-document, e.g.\n\n        cat >a <<-\\EOF &&\n        a\n        EOF\n\n> +\t(cat a; echo b) > ab &&\n> +\t(echo d; cat a; echo b) > dab &&\n> +\tcp a foo &&\n> +\tcp a bar &&\n\nProbably a helper function would make it more apparent what is going\non?\n\n\tas=\"a a a a a a a a\" && # eight a\n\ttest_write_lines $as b >ab &&\n\ttest_write_lines d $as b >dab &&\n        test_write_lines $as >foo &&\n        test_write_lines $as >bar &&\n\nOr even better, use test_write_lines in places you do use the result\nto overwrite foo/bar and get rid of ab and dab?\n\nThat helper can also be used to prepare bar-then-foo.\n\n>  test_expect_success 'patch-id output is well-formed' '\n> @@ -23,11 +57,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\nNot a new problem, but can you align \"git patch-id\" and \"sed\" to the\nsame column?  Also, not using \"/\" when there is no slash involved in\nthe pattern makes readers waste their time wondering why the script\navoids it.\n\n> +}\n> +\n> +calc_patch_id_stable () {\n> +\tgit patch-id --stable |\n> +\t\tsed \"s# .*##\" > patch-id_\"$1\"\n> +}\n> +\n> +\n\nExtra blank line.\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\nWhy doesn't it use calc_patch_id_stable?\n\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\nDitto.\n\n> +}\n> +\n> +\n\nExtra blank line.\n\n>  test_expect_success 'patch-id detects equality' '\n>  \tget_patch_id master &&\n>  \tget_patch_id same &&\n> @@ -52,10 +108,55 @@ test_expect_success 'patch-id supports git-format-patch output' '\n>  test_expect_success 'whitespace is irrelevant in footer' '\n>  \tget_patch_id master &&\n>  \tgit checkout same &&\n> -\tgit format-patch -1 --stdout | sed \"s/ \\$//\" | calc_patch_id same &&\n> +\tgit format-patch -1 --stdout | sed \"s/ \\$//\" |\n> +\t\tcalc_patch_id same &&\n\nWhat is this change about?\n"},{"id":"238136","messageId":"xmqqeh1i5fg4.fsf@gitster.dls.corp.google.com","threadId":"36318","inReplyTo":"20140331190444.GA12208@redhat.com","subject":"Re: [PATCH v3 1/3] patch-id: make it stable against hunk reordering","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-31T19:35:39Z","receivedAt":"2014-03-31T19:35:39Z","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> though it does look like an unrelated enhancement to me.\n> Agree?\n\nYes, that is exactly why I said \"opens interesting opportunity\" and\n\"making it possible\" ;-) They are all very related, but they do not\nhave to graduate as parts of the same series.\n\nThe important thing is that the basic infrastructure of the new\ncodepath is designed in such a way that it later allows us to reuse\nit in these changes.\n\nI'm queuing these three patches almost as-is (I think I tweaked the\n\"sizeof\" thing) on 'pu', without fixing the test styles and other\nissues I may have pointed out in my reviews myself, expecting\nfurther clean-ups.\n"},{"id":"238138","messageId":"xmqq7g7a5ek9.fsf@gitster.dls.corp.google.com","threadId":"36318","inReplyTo":"20140331192604.GF12208@redhat.com","subject":"Re: [PATCH v3 2/3] patch-id: document new behaviour","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-31T19:54:46Z","receivedAt":"2014-03-31T19:54:46Z","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> The hash used is mostly an internal implementation detail, isn't it?\n\nYes, but that does not mean we can break people who keep an external\ndatabase indexed with the patch-id by changing the default under\nthem, and \"they can give --unstable option to work it around\" is a\nworkaround, not a fix.  Without this change, they did not have to do\nanything.\n\nI would imagine that most of these people will be using the plain\nvanilla \"git show\" output without any ordering or hunk splitting\nwhen coming up with such a key.  A possible way forward to allow the\nconfiguration that corresponds to \"-O<orderfile>\" while not breaking\nthe existing users could be to make the \"patch-id --stable\" kick in\nautomatically (of course, do this only when the user did not give\nthe \"--unstable\" command line option to override) when we see the\norderfile configuration in the repository, or when we see that the\nincoming patch looks like reordered (e.g. has multiple \"diff --git\"\nheader lines that refer to the same path, or the set of files\nmentioned by the \"diff --git\" lines are not in ascending order),\nperhaps?\n"},{"id":"238142","messageId":"20140331204205.GB12403@redhat.com","threadId":"36318","inReplyTo":"xmqq7g7a5ek9.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 2/3] patch-id: document new behaviour","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-03-31T20:42:05Z","receivedAt":"2014-03-31T20:42:05Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Mon, Mar 31, 2014 at 12:54:46PM -0700, Junio C Hamano wrote:\n> \"Michael S. Tsirkin\" <mst@redhat.com> writes:\n> \n> > The hash used is mostly an internal implementation detail, isn't it?\n> \n> Yes, but that does not mean we can break people who keep an external\n> database indexed with the patch-id by changing the default under\n> them, and \"they can give --unstable option to work it around\" is a\n> workaround, not a fix.  Without this change, they did not have to do\n> anything.\n> \n> I would imagine that most of these people will be using the plain\n> vanilla \"git show\" output without any ordering or hunk splitting\n> when coming up with such a key.  A possible way forward to allow the\n> configuration that corresponds to \"-O<orderfile>\" while not breaking\n> the existing users could be to make the \"patch-id --stable\" kick in\n> automatically (of course, do this only when the user did not give\n> the \"--unstable\" command line option to override) when we see the\n> orderfile configuration in the repository, or when we see that the\n> incoming patch looks like reordered (e.g. has multiple \"diff --git\"\n> header lines that refer to the same path,\n\nThis would require us to track affected files in memory.\nIssue?\n\n> or the set of files\n> mentioned by the \"diff --git\" lines are not in ascending order),\n> perhaps?\n\nI hope a patch-id configuration flag plus maybe checking the orderfile if not\nspecified together should be good enough.\n\n-- \nMST\n"},{"id":"238167","messageId":"xmqqfvly2fe9.fsf@gitster.dls.corp.google.com","threadId":"36318","inReplyTo":"1396202583-2572-1-git-send-email-mst@redhat.com","subject":"Re: [PATCH v3 1/3] patch-id: make it stable against hunk reordering","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-31T22:05:02Z","receivedAt":"2014-03-31T22:05:02Z","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> +\t\t\t\t\t\tmemcpy(&ctx, &header_ctx, sizeof ctx);\n> +\t\t\t\t\t} else {\n> +\t\t\t\t\t\t/* Save header ctx for next hunk.  */\n> +\t\t\t\t\t\tmemcpy(&header_ctx, &ctx, sizeof ctx);\n\nI'll fix these to sizeof(ctx) when queuing.\n\nThanks.\n"},{"id":"238325","messageId":"xmqqy4zntx1p.fsf@gitster.dls.corp.google.com","threadId":"36318","inReplyTo":"20140331204205.GB12403@redhat.com","subject":"Re: [PATCH v3 2/3] patch-id: document new behaviour","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-02T18:18:26Z","receivedAt":"2014-04-02T18:18:26Z","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> On Mon, Mar 31, 2014 at 12:54:46PM -0700, Junio C Hamano wrote:\n>> \"Michael S. Tsirkin\" <mst@redhat.com> writes:\n>> \n>> > The hash used is mostly an internal implementation detail, isn't it?\n>> \n>> Yes, but that does not mean we can break people who keep an external\n>> database indexed with the patch-id by changing the default under\n>> them, and \"they can give --unstable option to work it around\" is a\n>> workaround, not a fix.  Without this change, they did not have to do\n>> anything.\n>> \n>> I would imagine that most of these people will be using the plain\n>> vanilla \"git show\" output without any ordering or hunk splitting\n>> when coming up with such a key.  A possible way forward to allow the\n>> configuration that corresponds to \"-O<orderfile>\" while not breaking\n>> the existing users could be to make the \"patch-id --stable\" kick in\n>> automatically (of course, do this only when the user did not give\n>> the \"--unstable\" command line option to override) when we see the\n>> orderfile configuration in the repository, or when we see that the\n>> incoming patch looks like reordered (e.g. has multiple \"diff --git\"\n>> header lines that refer to the same path,\n>\n> This would require us to track affected files in memory.\n> Issue?\n\nDon't we already do that in order to handle a patch that touches the\nsame path more than once anyway?  I think a possibly larger issue\nmight be that you would still want to do the hashing in a single\npass so you may need to always keep two accumulated hashes, before\nyou can decide if the patch is or is not a straight-forward one and\nuse one of the two, but that hopefully should not require a rocket\nscientist.\n"},{"id":"238332","messageId":"20140402190243.GA13425@redhat.com","threadId":"36318","inReplyTo":"xmqqy4zntx1p.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 2/3] patch-id: document new behaviour","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-04-02T19:02:43Z","receivedAt":"2014-04-02T19:02:43Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Wed, Apr 02, 2014 at 11:18:26AM -0700, Junio C Hamano wrote:\n> \"Michael S. Tsirkin\" <mst@redhat.com> writes:\n> \n> > On Mon, Mar 31, 2014 at 12:54:46PM -0700, Junio C Hamano wrote:\n> >> \"Michael S. Tsirkin\" <mst@redhat.com> writes:\n> >> \n> >> > The hash used is mostly an internal implementation detail, isn't it?\n> >> \n> >> Yes, but that does not mean we can break people who keep an external\n> >> database indexed with the patch-id by changing the default under\n> >> them, and \"they can give --unstable option to work it around\" is a\n> >> workaround, not a fix.  Without this change, they did not have to do\n> >> anything.\n> >> \n> >> I would imagine that most of these people will be using the plain\n> >> vanilla \"git show\" output without any ordering or hunk splitting\n> >> when coming up with such a key.  A possible way forward to allow the\n> >> configuration that corresponds to \"-O<orderfile>\" while not breaking\n> >> the existing users could be to make the \"patch-id --stable\" kick in\n> >> automatically (of course, do this only when the user did not give\n> >> the \"--unstable\" command line option to override) when we see the\n> >> orderfile configuration in the repository, or when we see that the\n> >> incoming patch looks like reordered (e.g. has multiple \"diff --git\"\n> >> header lines that refer to the same path,\n> >\n> > This would require us to track affected files in memory.\n> > Issue?\n> \n> Don't we already do that in order to handle a patch that touches the\n> same path more than once anyway?\n\nAt least I don't see it in builtin/patch-id.c\n\n> I think a possibly larger issue\n> might be that you would still want to do the hashing in a single\n> pass so you may need to always keep two accumulated hashes, before\n> you can decide if the patch is or is not a straight-forward one and\n> use one of the two, but that hopefully should not require a rocket\n> scientist.\n\nBut the issue is that equivalent patches would get a different hash.\nThis is what I tried to fix, after all.\n\nSo I think I prefer using an option and not a heuristic if you\nare fine with that.\nAt some point in the future we might flip the default.\n\n-- \nMST\n"},{"id":"238352","messageId":"xmqqa9c2s9l5.fsf@gitster.dls.corp.google.com","threadId":"36318","inReplyTo":"20140402190243.GA13425@redhat.com","subject":"Re: [PATCH v3 2/3] patch-id: document new behaviour","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-03T15:42:46Z","receivedAt":"2014-04-03T15:42:46Z","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> So I think I prefer using an option and not a heuristic if you\n> are fine with that.\n\nSure. Changing behaviour only by explicit user request (command line\nor configuration) is much safer than heuristics that does not work\nreliably.\n"}]}