{"thread":{"id":"36479","subject":"[PATCH v4 3/6] tests: new test for orderfile options","startedAt":"2014-04-23T12:14:53Z","lastAt":"2014-04-24T18:13:19Z","messageCount":15,"participants":["Michael S. Tsirkin","Junio C Hamano"],"isPatch":true,"patchVersion":4,"patchTotal":6},"messages":[{"id":"239419","messageId":"1398255277-26303-1-git-send-email-mst@redhat.com","threadId":"36479","inReplyTo":null,"subject":"[PATCH v4 1/6] diff: add a config option to control orderfile","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-04-23T12:14:53Z","receivedAt":"2014-04-23T12:14:53Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"I always want my diffs to show header files first,\nthen .c files, then the rest. Make it possible to\nset orderfile though a config option to achieve this.\n\nSigned-off-by: Michael S. Tsirkin <mst@redhat.com>\n---\n diff.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/diff.c b/diff.c\nindex b79432b..6bcb26b 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -264,6 +264,9 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"diff.orderfile\"))\n+\t\treturn git_config_string(&default_diff_options.orderfile, var, value);\n+\n \tif (starts_with(var, \"submodule.\"))\n \t\treturn parse_submodule_config_option(var, value);\n \n-- \nMST\n"},{"id":"239423","messageId":"1398255277-26303-2-git-send-email-mst@redhat.com","threadId":"36479","inReplyTo":"1398255277-26303-1-git-send-email-mst@redhat.com","subject":"[PATCH v4 2/6] test: add test_write_lines helper","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-04-23T12:14:55Z","receivedAt":"2014-04-23T12:14:55Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"As suggested by Junio.\n\nSigned-off-by: Michael S. Tsirkin <mst@redhat.com>\n---\n t/test-lib-functions.sh | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex aeae3ca..2fa6453 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -712,6 +712,13 @@ test_ln_s_add () {\n \tfi\n }\n \n+# This function writes out its parameters, one per line\n+test_write_lines () {\n+\tfor line in \"$@\"; do\n+\t\techo \"$line\"\n+\tdone\n+}\n+\n perl () {\n \tcommand \"$PERL_PATH\" \"$@\"\n }\n-- \nMST\n"},{"id":"239418","messageId":"1398255277-26303-3-git-send-email-mst@redhat.com","threadId":"36479","inReplyTo":"1398255277-26303-1-git-send-email-mst@redhat.com","subject":"[PATCH v4 3/6] tests: new test for orderfile options","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-04-23T12:14:57Z","receivedAt":"2014-04-23T12:14:57Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"The test is very basic and can be extended.\nCouldn't find a good existing place to put it,\nso created a new file.\n\nSigned-off-by: Michael S. Tsirkin <mst@redhat.com>\n---\n t/t4056-diff-order.sh | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 63 insertions(+)\n create mode 100755 t/t4056-diff-order.sh\n\ndiff --git a/t/t4056-diff-order.sh b/t/t4056-diff-order.sh\nnew file mode 100755\nindex 0000000..a247b7a\n--- /dev/null\n+++ b/t/t4056-diff-order.sh\n@@ -0,0 +1,63 @@\n+#!/bin/sh\n+\n+test_description='diff orderfile'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+        as=\"a a a a a a a a\" && # eight a\n+        test_write_lines $as >foo &&\n+        test_write_lines $as >bar &&\n+\tgit add foo bar &&\n+\tgit commit -a -m initial &&\n+        test_write_lines $as b >foo &&\n+        test_write_lines $as b >bar &&\n+\tgit commit -a -m first &&\n+\ttest_write_lines bar foo >bar-then-foo &&\n+\ttest_write_lines foo bar >foo-then-bar &&\n+\tgit diff -Ofoo-then-bar HEAD~1..HEAD >diff-foo-then-bar &&\n+\tgit diff -Obar-then-foo HEAD~1..HEAD >diff-bar-then-foo\n+'\n+\n+test_diff_well_formed () {\n+\tgrep ^+b \"$1\" >added\n+\tgrep ^-b \"$1\" >removed\n+\tgrep ^+++ \"$1\" >oldfiles\n+\tgrep ^--- \"$1\" >newfiles\n+\ttest_line_count = 2 added &&\n+\ttest_line_count = 0 removed &&\n+\ttest_line_count = 2 oldfiles &&\n+\ttest_line_count = 2 newfiles\n+}\n+\n+test_expect_success 'diff output with -O is well-formed' '\n+\ttest_diff_well_formed diff-foo-then-bar &&\n+\ttest_diff_well_formed diff-bar-then-foo\n+'\n+\n+test_expect_success 'flag -O affects diff output' '\n+\t! test_cmp diff-foo-then-bar diff-bar-then-foo\n+'\n+\n+test_expect_success 'orderfile is same as -O' '\n+\ttest_config diff.orderfile foo-then-bar &&\n+\tgit diff HEAD~1..HEAD >diff-foo-then-bar-config &&\n+\ttest_config diff.orderfile bar-then-foo &&\n+\tgit diff HEAD~1..HEAD >diff-bar-then-foo-config &&\n+\ttest_cmp diff-foo-then-bar diff-foo-then-bar-config &&\n+\ttest_cmp diff-bar-then-foo diff-bar-then-foo-config\n+'\n+\n+test_expect_success '-O overrides orderfile' '\n+\ttest_config diff.orderfile foo-then-bar &&\n+\tgit diff -Obar-then-foo HEAD~1..HEAD >diff-bar-then-foo-flag &&\n+\ttest_cmp diff-bar-then-foo diff-bar-then-foo-flag\n+'\n+\n+test_expect_success '/dev/null is same as no orderfile' '\n+\tgit diff -O/dev/null HEAD~1..HEAD>diff-null-orderfile &&\n+\tgit diff HEAD~1..HEAD >diff-default &&\n+\ttest_cmp diff-null-orderfile diff-default\n+'\n+\n+test_done\n-- \nMST\n"},{"id":"239420","messageId":"1398255277-26303-4-git-send-email-mst@redhat.com","threadId":"36479","inReplyTo":"1398255277-26303-1-git-send-email-mst@redhat.com","subject":"[PATCH v4 4/6] patch-id: make it stable against hunk reordering","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-04-23T12:15:00Z","receivedAt":"2014-04-23T12:15:00Z","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\nAdd an option to change patch-id behaviour making it stable against\nthese two kinds of 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\nThe new behaviour is enabled\n- when diff.orderfile config option is present\n  (as that is likely to reorder patches)\n- when patchid.stable is true\n- when --stable flag is present\n\nUsing a new flag --unstable or setting patchid.stable to false force\nthe historical behaviour.\n\nSigned-off-by: Michael S. Tsirkin <mst@redhat.com>\n---\n builtin/patch-id.c | 94 ++++++++++++++++++++++++++++++++++++++++++++----------\n 1 file changed, 78 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/patch-id.c b/builtin/patch-id.c\nindex 3cfe02d..e154405 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,68 @@ 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+static int git_patch_id_config(const char *var, const char *value, void *cb)\n+{\n+\tint *stable = cb;\n+\n+\tif (!strcmp(var, \"patchid.stable\")) {\n+\t\t*stable = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n+\tif (!strcmp(var, \"diff.orderfile\") && *stable < 0) {\n+\t\t*stable = 1;\n+\t\treturn 0;\n+\t}\n+\n+\treturn git_default_config(var, value, cb);\n+}\n \n int cmd_patch_id(int argc, const char **argv, const char *prefix)\n {\n-\tif (argc != 1)\n+\tint stable = -1;\n+\n+\tgit_config(git_patch_id_config, &stable);\n+\n+\t/* If nothing is set, default to unstable. */\n+\tif (stable < 0)\n+\t\tstable = 0;\n+\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\tusage(patch_id_usage);\n \n-\tgenerate_id_list();\n+\tgenerate_id_list(stable);\n \treturn 0;\n }\n-- \nMST\n"},{"id":"239421","messageId":"1398255277-26303-5-git-send-email-mst@redhat.com","threadId":"36479","inReplyTo":"1398255277-26303-1-git-send-email-mst@redhat.com","subject":"[PATCH v4 5/6] patch-id: document new behaviour","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-04-23T12:15:02Z","receivedAt":"2014-04-23T12:15:02Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"Clarify that patch ID can now be a sum of hashes, not a hash.\nDocument how command line and config options affect the\nbehaviour.\n\nSigned-off-by: Michael S. Tsirkin <mst@redhat.com>\n---\n Documentation/git-patch-id.txt | 25 ++++++++++++++++++++-----\n 1 file changed, 20 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-patch-id.txt b/Documentation/git-patch-id.txt\nindex 312c3b1..61d4a67 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,21 @@ 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 if patchid.stable is set to true, or if patchid.stable\n+\tis not set and diff.orderfile is set to some value.\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 is the default if patchid.stable is set to false, or if neither patchid.stable\n+\tnor diff.orderfile are set.\n+\n <patch>::\n \tThe diff to create the ID of.\n \n-- \nMST\n"},{"id":"239422","messageId":"1398255277-26303-6-git-send-email-mst@redhat.com","threadId":"36479","inReplyTo":"1398255277-26303-1-git-send-email-mst@redhat.com","subject":"[PATCH v4 6/6] patch-id-test: test stable and unstable behaviour","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-04-23T12:15:04Z","receivedAt":"2014-04-23T12:15:04Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"Verify that patch ID supports an algorithm\nthat is stable against diff split and reordering.\n\nSigned-off-by: Michael S. Tsirkin <mst@redhat.com>\n---\n t/t4204-patch-id.sh | 140 +++++++++++++++++++++++++++++++++++++++++++++++-----\n 1 file changed, 129 insertions(+), 11 deletions(-)\n\ndiff --git a/t/t4204-patch-id.sh b/t/t4204-patch-id.sh\nindex d2c930d..54f2fb8 100755\n--- a/t/t4204-patch-id.sh\n+++ b/t/t4204-patch-id.sh\n@@ -5,27 +5,51 @@ 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+        as=\"a a a a a a a a\" && # eight a\n+\ttest_write_lines $as >foo &&\n+\ttest_write_lines $as >bar &&\n+\tgit add foo bar &&\n+\tgit commit -a -m initial &&\n+\ttest_write_lines $as b >foo &&\n+\ttest_write_lines $as b >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+\ttest_write_lines d $as b >foo &&\n+\ttest_write_lines d $as b >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+\ttest_write_lines bar foo >bar-then-foo &&\n+\ttest_write_lines foo bar >foo-then-bar\n '\n \n test_expect_success 'patch-id output is well-formed' '\n-\tgit log -p -1 | git patch-id > output &&\n+\tgit log -p -1 | git patch-id >output &&\n \tgrep \"^[a-f0-9]\\{40\\} $(git rev-parse HEAD)$\" output\n '\n \n+#calculate patch id. Make sure output is not empty.\n calc_patch_id () {\n-\tgit patch-id |\n-\t\tsed \"s# .*##\" > patch-id_\"$1\"\n+\tname=\"$1\"\n+\tshift\n+\tgit patch-id \"$@\" |\n+\tsed \"s/ .*//\" >patch-id_\"$name\" &&\n+\ttest_line_count -gt 0 patch-id_\"$name\"\n+}\n+\n+get_top_diff () {\n+\tgit log -p -1 \"$@\" -O bar-then-foo --\n }\n \n get_patch_id () {\n-\tgit log -p -1 \"$1\" | git patch-id |\n-\t\tsed \"s# .*##\" > patch-id_\"$1\"\n+\tget_top_diff \"$1\" | calc_patch_id \"$@\"\n }\n \n test_expect_success 'patch-id detects equality' '\n@@ -56,6 +80,100 @@ test_expect_success 'whitespace is irrelevant in footer' '\n \ttest_cmp patch-id_master patch-id_same\n '\n \n+cmp_patch_id () {\n+\tif\n+\t\ttest \"$1\" = \"relevant\"\n+\tthen\n+\t\t! test_cmp patch-id_\"$2\" patch-id_\"$3\"\n+\telse\n+\t\ttest_cmp patch-id_\"$2\" patch-id_\"$3\"\n+\tfi\n+}\n+\n+test_patch_id_file_order () {\n+\trelevant=\"$1\"\n+\tshift\n+\tname=\"order-${1}-$relevant\"\n+\tshift\n+\tget_top_diff \"master\" | calc_patch_id \"$name\" \"$@\" &&\n+\tgit checkout same &&\n+\tgit format-patch -1 --stdout -O foo-then-bar |\n+\t\tcalc_patch_id \"ordered-$name\" \"$@\" &&\n+\tcmp_patch_id $relevant \"$name\" \"ordered-$name\"\n+\t\t\n+}\n+\n+test_patch_id_split () {\n+\trelevant=\"$1\"\n+\tshift\n+\tname=\"split-${1}-$relevant\"\n+\tshift\n+\tget_top_diff merged | calc_patch_id \"$name\" \"$@\" &&\n+\t(git log -p -1 -O foo-then-bar split~1; git diff split~1..split) |\n+\t\tcalc_patch_id \"split-$name\" \"$@\" &&\n+\tcmp_patch_id \"$relevant\" \"$name\" \"split-$name\"\n+}\n+\n+# combined test for options\n+test_patch_id () {\n+\ttest_patch_id_file_order \"$@\" &&\n+\ttest_patch_id_split \"$@\"\n+}\n+\n+# small tests with detailed diagnostic for basic options.\n+test_expect_success 'file order is irrelevant with --stable' '\n+\ttest_patch_id_file_order irrelevant --stable --stable\n+'\n+\n+test_expect_success 'file order is relevant with --unstable' '\n+\ttest_patch_id_file_order relevant --unstable --unstable\n+'\n+\n+test_expect_success 'splitting patch is irrelevant with --stable' '\n+\ttest_patch_id_split irrelevant --stable --stable\n+'\n+\n+test_expect_success 'splitting patch affects id with --unstable' '\n+\ttest_patch_id_split relevant --unstable --unstable\n+'\n+\n+#Now test various option combinations.\n+test_expect_success 'default is unstable' '\n+\ttest_patch_id relevant default\n+'\n+\n+test_expect_success 'patchid.stable = true is stable' '\n+\ttest_config patchid.stable true &&\n+\ttest_patch_id irrelevant patchid.stable=true\n+'\n+\n+test_expect_success 'patchid.stable = false is unstable' '\n+\ttest_config patchid.stable false &&\n+\ttest_patch_id relevant patchid.stable=false\n+'\n+\n+test_expect_success 'diff.orderfile implies stable' '\n+\ttest_config diff.orderfile /dev/null &&\n+\ttest_patch_id irrelevant diff.orderfile\n+'\n+\n+test_expect_success 'patchid.stable = false overrides diff.orderfile' '\n+\ttest_config patchid.stable false &&\n+\ttest_config diff.orderfile /dev/null &&\n+\ttest_patch_id relevant patchid.stable=false-diff.orderfile\n+'\n+\n+test_expect_success '--unstable overrides both orderfile and patchid.stable = true' '\n+\ttest_config diff.orderfile /dev/null &&\n+\ttest_config patchid.stable true &&\n+\ttest_patch_id relevant patchid.stable=true--unstable --unstable\n+'\n+\n+test_expect_success '--stable overrides patchid.stable = false' '\n+\ttest_config patchid.stable false &&\n+\ttest_patch_id irrelevant patchid.stable=false--stable --stable\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":"239451","messageId":"xmqqd2g80x21.fsf@gitster.dls.corp.google.com","threadId":"36479","inReplyTo":"1398255277-26303-2-git-send-email-mst@redhat.com","subject":"Re: [PATCH v4 2/6] test: add test_write_lines helper","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-23T17:34:30Z","receivedAt":"2014-04-23T17:34:30Z","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> As suggested by Junio.\n>\n> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>\n> ---\n\nEhh, I would probably not suggest such an implementation though.\n\n\ttest_write_lines () {\n\t\tprintf \"%s\\n\" \"$@\"\n\t}\n\nmight be, but not with \"echo\" and semicolon on the same line as\n\"for\" ;-).\n\n>  t/test-lib-functions.sh | 7 +++++++\n>  1 file changed, 7 insertions(+)\n>\n> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\n> index aeae3ca..2fa6453 100644\n> --- a/t/test-lib-functions.sh\n> +++ b/t/test-lib-functions.sh\n> @@ -712,6 +712,13 @@ test_ln_s_add () {\n>  \tfi\n>  }\n>  \n> +# This function writes out its parameters, one per line\n> +test_write_lines () {\n> +\tfor line in \"$@\"; do\n> +\t\techo \"$line\"\n> +\tdone\n> +}\n> +\n>  perl () {\n>  \tcommand \"$PERL_PATH\" \"$@\"\n>  }\n"},{"id":"239452","messageId":"xmqq8uqw0ww0.fsf@gitster.dls.corp.google.com","threadId":"36479","inReplyTo":"1398255277-26303-3-git-send-email-mst@redhat.com","subject":"Re: [PATCH v4 3/6] tests: new test for orderfile options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-23T17:38:07Z","receivedAt":"2014-04-23T17:38:07Z","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 test is very basic and can be extended.\n> Couldn't find a good existing place to put it,\n> so created a new file.\n>\n> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>\n> ---\n>  t/t4056-diff-order.sh | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++\n>  1 file changed, 63 insertions(+)\n>  create mode 100755 t/t4056-diff-order.sh\n>\n> diff --git a/t/t4056-diff-order.sh b/t/t4056-diff-order.sh\n> new file mode 100755\n> index 0000000..a247b7a\n> --- /dev/null\n> +++ b/t/t4056-diff-order.sh\n> @@ -0,0 +1,63 @@\n> +#!/bin/sh\n> +\n> +test_description='diff orderfile'\n> +\n> +. ./test-lib.sh\n> +\n> +test_expect_success 'setup' '\n> +        as=\"a a a a a a a a\" && # eight a\n> +        test_write_lines $as >foo &&\n> +        test_write_lines $as >bar &&\n> +\tgit add foo bar &&\n> +\tgit commit -a -m initial &&\n> +        test_write_lines $as b >foo &&\n> +        test_write_lines $as b >bar &&\n\nThese lines are curiously jaggy; are you indenting with spaces?\nPlease don't.\n\nMy running \"git am -s3c\" may auto-fix these indentation issues, so\nplease wait before seeing what may be pushed out on 'pu'.  It may\nturn out that there is no other need for rerolling this patch in the\nseries.\n\nThanks.\n\n> +\tgit commit -a -m first &&\n> +\ttest_write_lines bar foo >bar-then-foo &&\n> +\ttest_write_lines foo bar >foo-then-bar &&\n> +\tgit diff -Ofoo-then-bar HEAD~1..HEAD >diff-foo-then-bar &&\n> +\tgit diff -Obar-then-foo HEAD~1..HEAD >diff-bar-then-foo\n> +'\n> +\n> +test_diff_well_formed () {\n> +\tgrep ^+b \"$1\" >added\n> +\tgrep ^-b \"$1\" >removed\n> +\tgrep ^+++ \"$1\" >oldfiles\n> +\tgrep ^--- \"$1\" >newfiles\n> +\ttest_line_count = 2 added &&\n> +\ttest_line_count = 0 removed &&\n> +\ttest_line_count = 2 oldfiles &&\n> +\ttest_line_count = 2 newfiles\n> +}\n> +\n> +test_expect_success 'diff output with -O is well-formed' '\n> +\ttest_diff_well_formed diff-foo-then-bar &&\n> +\ttest_diff_well_formed diff-bar-then-foo\n> +'\n> +\n> +test_expect_success 'flag -O affects diff output' '\n> +\t! test_cmp diff-foo-then-bar diff-bar-then-foo\n> +'\n> +\n> +test_expect_success 'orderfile is same as -O' '\n> +\ttest_config diff.orderfile foo-then-bar &&\n> +\tgit diff HEAD~1..HEAD >diff-foo-then-bar-config &&\n> +\ttest_config diff.orderfile bar-then-foo &&\n> +\tgit diff HEAD~1..HEAD >diff-bar-then-foo-config &&\n> +\ttest_cmp diff-foo-then-bar diff-foo-then-bar-config &&\n> +\ttest_cmp diff-bar-then-foo diff-bar-then-foo-config\n> +'\n> +\n> +test_expect_success '-O overrides orderfile' '\n> +\ttest_config diff.orderfile foo-then-bar &&\n> +\tgit diff -Obar-then-foo HEAD~1..HEAD >diff-bar-then-foo-flag &&\n> +\ttest_cmp diff-bar-then-foo diff-bar-then-foo-flag\n> +'\n> +\n> +test_expect_success '/dev/null is same as no orderfile' '\n> +\tgit diff -O/dev/null HEAD~1..HEAD>diff-null-orderfile &&\n> +\tgit diff HEAD~1..HEAD >diff-default &&\n> +\ttest_cmp diff-null-orderfile diff-default\n> +'\n> +\n> +test_done\n"},{"id":"239453","messageId":"xmqq4n1k0wtw.fsf@gitster.dls.corp.google.com","threadId":"36479","inReplyTo":"1398255277-26303-4-git-send-email-mst@redhat.com","subject":"Re: [PATCH v4 4/6] patch-id: make it stable against hunk reordering","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-23T17:39:23Z","receivedAt":"2014-04-23T17:39:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Are these three patches the same as what has been queued on\nmt/patch-id-stable topic and cooking in 'next' for a few weeks?\n"},{"id":"239455","messageId":"20140423175717.GB28308@redhat.com","threadId":"36479","inReplyTo":"xmqq4n1k0wtw.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v4 4/6] patch-id: make it stable against hunk reordering","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-04-23T17:57:17Z","receivedAt":"2014-04-23T17:57:17Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Wed, Apr 23, 2014 at 10:39:23AM -0700, Junio C Hamano wrote:\n> Are these three patches the same as what has been queued on\n> mt/patch-id-stable topic and cooking in 'next' for a few weeks?\n\nNot exactly - at your request I implemented git config\noptions to control patch id behaviour.\nDocumentation and tests updated to match.\n"},{"id":"239456","messageId":"20140423175834.GC28308@redhat.com","threadId":"36479","inReplyTo":"xmqqd2g80x21.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v4 2/6] test: add test_write_lines helper","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-04-23T17:58:34Z","receivedAt":"2014-04-23T17:58:34Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Wed, Apr 23, 2014 at 10:34:30AM -0700, Junio C Hamano wrote:\n> \"Michael S. Tsirkin\" <mst@redhat.com> writes:\n> \n> > As suggested by Junio.\n> >\n> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>\n> > ---\n> \n> Ehh, I would probably not suggest such an implementation though.\n> \n> \ttest_write_lines () {\n> \t\tprintf \"%s\\n\" \"$@\"\n> \t}\n> \n> might be, but not with \"echo\" and semicolon on the same line as\n> \"for\" ;-).\n\nOkay I didn't know printf reuses format in bash, cute trick.\n\nDo you want to rewrite it yourself or want me to post a\nnew version?\n\n\n> >  t/test-lib-functions.sh | 7 +++++++\n> >  1 file changed, 7 insertions(+)\n> >\n> > diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\n> > index aeae3ca..2fa6453 100644\n> > --- a/t/test-lib-functions.sh\n> > +++ b/t/test-lib-functions.sh\n> > @@ -712,6 +712,13 @@ test_ln_s_add () {\n> >  \tfi\n> >  }\n> >  \n> > +# This function writes out its parameters, one per line\n> > +test_write_lines () {\n> > +\tfor line in \"$@\"; do\n> > +\t\techo \"$line\"\n> > +\tdone\n> > +}\n> > +\n> >  perl () {\n> >  \tcommand \"$PERL_PATH\" \"$@\"\n> >  }\n"},{"id":"239458","messageId":"20140423180003.GA28334@redhat.com","threadId":"36479","inReplyTo":"xmqq8uqw0ww0.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v4 3/6] tests: new test for orderfile options","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-04-23T18:00:03Z","receivedAt":"2014-04-23T18:00:03Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Wed, Apr 23, 2014 at 10:38:07AM -0700, Junio C Hamano wrote:\n> \"Michael S. Tsirkin\" <mst@redhat.com> writes:\n> \n> > The test is very basic and can be extended.\n> > Couldn't find a good existing place to put it,\n> > so created a new file.\n> >\n> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>\n> > ---\n> >  t/t4056-diff-order.sh | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++\n> >  1 file changed, 63 insertions(+)\n> >  create mode 100755 t/t4056-diff-order.sh\n> >\n> > diff --git a/t/t4056-diff-order.sh b/t/t4056-diff-order.sh\n> > new file mode 100755\n> > index 0000000..a247b7a\n> > --- /dev/null\n> > +++ b/t/t4056-diff-order.sh\n> > @@ -0,0 +1,63 @@\n> > +#!/bin/sh\n> > +\n> > +test_description='diff orderfile'\n> > +\n> > +. ./test-lib.sh\n> > +\n> > +test_expect_success 'setup' '\n> > +        as=\"a a a a a a a a\" && # eight a\n> > +        test_write_lines $as >foo &&\n> > +        test_write_lines $as >bar &&\n> > +\tgit add foo bar &&\n> > +\tgit commit -a -m initial &&\n> > +        test_write_lines $as b >foo &&\n> > +        test_write_lines $as b >bar &&\n> \n> These lines are curiously jaggy; are you indenting with spaces?\n> Please don't.\n> \n> My running \"git am -s3c\" may auto-fix these indentation issues, so\n> please wait before seeing what may be pushed out on 'pu'.  It may\n> turn out that there is no other need for rerolling this patch in the\n> series.\n> \n> Thanks.\n\nNot normally - but this did creep in here, somehow :(\n\n> > +\tgit commit -a -m first &&\n> > +\ttest_write_lines bar foo >bar-then-foo &&\n> > +\ttest_write_lines foo bar >foo-then-bar &&\n> > +\tgit diff -Ofoo-then-bar HEAD~1..HEAD >diff-foo-then-bar &&\n> > +\tgit diff -Obar-then-foo HEAD~1..HEAD >diff-bar-then-foo\n> > +'\n> > +\n> > +test_diff_well_formed () {\n> > +\tgrep ^+b \"$1\" >added\n> > +\tgrep ^-b \"$1\" >removed\n> > +\tgrep ^+++ \"$1\" >oldfiles\n> > +\tgrep ^--- \"$1\" >newfiles\n> > +\ttest_line_count = 2 added &&\n> > +\ttest_line_count = 0 removed &&\n> > +\ttest_line_count = 2 oldfiles &&\n> > +\ttest_line_count = 2 newfiles\n> > +}\n> > +\n> > +test_expect_success 'diff output with -O is well-formed' '\n> > +\ttest_diff_well_formed diff-foo-then-bar &&\n> > +\ttest_diff_well_formed diff-bar-then-foo\n> > +'\n> > +\n> > +test_expect_success 'flag -O affects diff output' '\n> > +\t! test_cmp diff-foo-then-bar diff-bar-then-foo\n> > +'\n> > +\n> > +test_expect_success 'orderfile is same as -O' '\n> > +\ttest_config diff.orderfile foo-then-bar &&\n> > +\tgit diff HEAD~1..HEAD >diff-foo-then-bar-config &&\n> > +\ttest_config diff.orderfile bar-then-foo &&\n> > +\tgit diff HEAD~1..HEAD >diff-bar-then-foo-config &&\n> > +\ttest_cmp diff-foo-then-bar diff-foo-then-bar-config &&\n> > +\ttest_cmp diff-bar-then-foo diff-bar-then-foo-config\n> > +'\n> > +\n> > +test_expect_success '-O overrides orderfile' '\n> > +\ttest_config diff.orderfile foo-then-bar &&\n> > +\tgit diff -Obar-then-foo HEAD~1..HEAD >diff-bar-then-foo-flag &&\n> > +\ttest_cmp diff-bar-then-foo diff-bar-then-foo-flag\n> > +'\n> > +\n> > +test_expect_success '/dev/null is same as no orderfile' '\n> > +\tgit diff -O/dev/null HEAD~1..HEAD>diff-null-orderfile &&\n> > +\tgit diff HEAD~1..HEAD >diff-default &&\n> > +\ttest_cmp diff-null-orderfile diff-default\n> > +'\n> > +\n> > +test_done\n"},{"id":"239503","messageId":"xmqqzjjbwvk9.fsf@gitster.dls.corp.google.com","threadId":"36479","inReplyTo":"20140423175717.GB28308@redhat.com","subject":"Re: [PATCH v4 4/6] patch-id: make it stable against hunk reordering","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-23T22:05:42Z","receivedAt":"2014-04-23T22:05:42Z","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 Wed, Apr 23, 2014 at 10:39:23AM -0700, Junio C Hamano wrote:\n>> Are these three patches the same as what has been queued on\n>> mt/patch-id-stable topic and cooking in 'next' for a few weeks?\n>\n> Not exactly - at your request I implemented git config\n> options to control patch id behaviour.\n> Documentation and tests updated to match.\n\nAfter comparing the patches 4-6 and the one that has been in 'next'\nfor a few weeks, I tried to like the new one, but I couldn't.\n\nThe new one, if I am reading the patch correctly, makes the stable\nvariant the default if\n\n - the configuration explicitly asks to use it; or\n\n - diff.orderfile, which is a new configuration that did not exist,\n   is used.\n\nAt the first glance, the latter makes it look as if that will not\nhurt any existing users/repositories, but the thing is, the producer\nof the patches is different from the consumer of the patches.  There\nneeds to be a way for a consumer to say \"I need stable variant\" on\nthe patches when computing \"patch-id\" on a patch that arrived.  As\nlong as two different producers use two different orders, the\nconsumer of the patches from these two sources is forced to use the\nstable variant if some sort of cache is kept keyed with the\npatch-ids.\n\nBut \"diff.orderfile\" configuration is all and only about the\nproducer, and does not help the case at all.\n\nCompared to it, the previous version forced people who do not have a\nparticular reason to choose between the variants to use the new\n\"stable\" variant, which was a lot simpler solution.\n\nThe reason why I merged the previous one to 'next' was because I\nwanted to see if the behaviour change is as grave as I thought, or I\nwas just being unnecessary cautious.  If there is no third-party\nscript that precomputes something and stores them under these hashes\nthat breaks with this change, I do not see any reason not to make\nthe new \"stable\" one the default.\n\nI however suspect that the ideal \"stable\" implementation may be\ndifferent from what you implemented.  What if we compute a patch-id\non a reordered patch as if the files came in the \"usual\" order?\nThat would be another way to achieve the stable-ness for sane cases\n(i.e. no funny \"you could split one patch with two hunks into two\npatches with one hunk each twice mentioning the same path\" which is\ntotally an uninteresting use case---diff.orderfile would not even\nproduce such a patch) and a huge difference would be that it would\nproduce the same patch-id as existing versions of Git, if the patch\nis not reordered.  Is that asking for a moon (I admit that I haven't\nlooked at the code involved too deeply)?\n"},{"id":"239539","messageId":"20140424062938.GA30231@redhat.com","threadId":"36479","inReplyTo":"xmqqzjjbwvk9.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v4 4/6] patch-id: make it stable against hunk reordering","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-04-24T06:29:38Z","receivedAt":"2014-04-24T06:29:38Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Wed, Apr 23, 2014 at 03:05:42PM -0700, Junio C Hamano wrote:\n> \"Michael S. Tsirkin\" <mst@redhat.com> writes:\n> \n> > On Wed, Apr 23, 2014 at 10:39:23AM -0700, Junio C Hamano wrote:\n> >> Are these three patches the same as what has been queued on\n> >> mt/patch-id-stable topic and cooking in 'next' for a few weeks?\n> >\n> > Not exactly - at your request I implemented git config\n> > options to control patch id behaviour.\n> > Documentation and tests updated to match.\n> \n> After comparing the patches 4-6 and the one that has been in 'next'\n> for a few weeks, I tried to like the new one, but I couldn't.\n\nI'm fine with the one in next too.\nI was under the impression that you wanted me to change\nthe behaviour so I worked on this, but previous version was sufficient\nfor my purposes (which is really all about putting diff.orderfile\nupstream).\n\n> The new one, if I am reading the patch correctly, makes the stable\n> variant the default if\n> \n>  - the configuration explicitly asks to use it; or\n> \n>  - diff.orderfile, which is a new configuration that did not exist,\n>    is used.\n> \n> At the first glance, the latter makes it look as if that will not\n> hurt any existing users/repositories, but the thing is, the producer\n> of the patches is different from the consumer of the patches.  There\n> needs to be a way for a consumer to say \"I need stable variant\" on\n> the patches when computing \"patch-id\" on a patch that arrived.  As\n> long as two different producers use two different orders, the\n> consumer of the patches from these two sources is forced to use the\n> stable variant if some sort of cache is kept keyed with the\n> patch-ids.\n> \n> But \"diff.orderfile\" configuration is all and only about the\n> producer, and does not help the case at all.\n\nOK, we can just drop that, that's easy.\n\n> Compared to it, the previous version forced people who do not have a\n> particular reason to choose between the variants to use the new\n> \"stable\" variant, which was a lot simpler solution.\n> \n> The reason why I merged the previous one to 'next' was because I\n> wanted to see if the behaviour change is as grave as I thought, or I\n> was just being unnecessary cautious.  If there is no third-party\n> script that precomputes something and stores them under these hashes\n> that breaks with this change, I do not see any reason not to make\n> the new \"stable\" one the default.\n\nAh okay, so we just wait a bit and see and if all is quiet the\nexisting patch will graduate to master with time?\nTotally cool.\nI thought you wanted me to add the config option, but if everyone\nis happy as is, I don't need it personally at all.\n\n> I however suspect that the ideal \"stable\" implementation may be\n> different from what you implemented.  What if we compute a patch-id\n> on a reordered patch as if the files came in the \"usual\" order?\n\nATM patch id does not have any concept of the usual order,\nso that's one problem - how does one figure out what the order would be?\nI have no idea - is this documented anywhere?\nAlso I'm guessing this would depend on the state of the git tree which\nwould be another behaviour change: previously patch-id worked\nfine outside any git tree.\n\n>\n> That would be another way to achieve the stable-ness for sane cases\n> (i.e. no funny \"you could split one patch with two hunks into two\n> patches with one hunk each twice mentioning the same path\" which is\n> totally an uninteresting use case---diff.orderfile would not even\n> produce such a patch)\n\nYes I'm thinking we should drop this hunk in the patch:\nlet's support reordering files but not splitting them.\nThis makes the change even smaller, so I now think we should\njust go for it.\n\n> and a huge difference would be that it would\n> produce the same patch-id as existing versions of Git, if the patch\n> is not reordered.  Is that asking for a moon (I admit that I haven't\n> looked at the code involved too deeply)?\n\nYes this would be a bunch of code to write - certainly much more complex than\nthe existing small patch which just tweaks the checksum slightly to make\nit stable.\n\n-- \nMST\n"},{"id":"239581","messageId":"xmqq7g6ewq80.fsf@gitster.dls.corp.google.com","threadId":"36479","inReplyTo":"20140424062938.GA30231@redhat.com","subject":"Re: [PATCH v4 4/6] patch-id: make it stable against hunk reordering","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-24T18:13:19Z","receivedAt":"2014-04-24T18:13:19Z","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 Wed, Apr 23, 2014 at 03:05:42PM -0700, Junio C Hamano wrote:\n>\n>> After comparing the patches 4-6 and the one that has been in 'next'\n>> for a few weeks, I tried to like the new one, but I couldn't.\n>\n> I'm fine with the one in next too.\n> I was under the impression that you wanted me to change\n> the behaviour so I worked on this,...\n\nWhat I wanted to see was to make sure this would not kick in unless\nthe user explicitly asks.  \"patchid.stable\" configuration variable\nis very much welcome, and if it defaulted to false with or without\n\"diff.orderfile\", that would have been very much welcome.  Then\nnobody will be surprised.\n\n>> But \"diff.orderfile\" configuration is all and only about the\n>> producer, and does not help the case at all.\n>\n> OK, we can just drop that, that's easy.\n>\n>> Compared to it, the previous version forced people who do not have a\n>> particular reason to choose between the variants to use the new\n>> \"stable\" variant, which was a lot simpler solution.\n>> \n>> The reason why I merged the previous one to 'next' was because I\n>> wanted to see if the behaviour change is as grave as I thought, or I\n>> was just being unnecessary cautious.  If there is no third-party\n>> script that precomputes something and stores them under these hashes\n>> that breaks with this change, I do not see any reason not to make\n>> the new \"stable\" one the default.\n>\n> Ah okay, so we just wait a bit and see and if all is quiet the\n> existing patch will graduate to master with time?\n> Totally cool.\n> I thought you wanted me to add the config option, but if everyone\n> is happy as is, I don't need it personally at all.\n\n... or if we see problems, then build a fix on top to introduce\n\"patchid.stable\" that defaults to false and not linking the feature\nwith \"diff.ordefile\".\n\nAdding a new feature turned-off by default is the safer thing to do.\nWhen nothing changes, by defintion there won't be a new breakage.\nThat is the default stance this project has taken for a long time,\nand what made us trusted by projects that manage their source files\nusing our product.\n\nIt however is to favour stability and \"no surprise\" over progress,\nand it may not be an optimal thing to do if the new feature is\ncompellingly good.  In some cases like this one, we may need to\nexperiment to find out the need of the users, and introducing the\nfeature with a configuration that is explicitly opt-in is one way to\ndo so.  We may or may not see many people using that feature without\ndisrupting existing users that way.\n\nCooking in 'next' with the feature turned-on by default is another\nway that is riskier, but in this particular case, the population\nthat is likely to be affected are power users, which would match the\naudience of 'next'---those who still want stability but want to be\ncloser to the cutting edge.\n"}]}