{"thread":{"id":"40985","subject":"[PATCH v2 0/3] format-patch: introduce option to suppress commit hashes","startedAt":"2015-12-13T17:27:15Z","lastAt":"2015-12-15T00:35:04Z","messageCount":10,"participants":["brian m. carlson","Torsten Bögershausen","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":2,"patchTotal":3},"messages":[{"id":"274348","messageId":"1450027638-788102-1-git-send-email-sandals@crustytoothpaste.net","threadId":"40985","inReplyTo":null,"subject":"[PATCH v2 0/3] format-patch: introduce option to suppress commit hashes","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2015-12-13T17:27:15Z","receivedAt":"2015-12-13T17:27:15Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"git format-patch is often used to create patches that are then stored in\nversion control or displayed with diff. Having the commit hash in the\n\"From \" line usually just creates diff noise in these cases, so this\nseries introduces --zero-commit to set that to all zeros.\n\nChanges from v1:\n* Rename the option --zero-commit.\n* Improve the tests to look for a 40-hex hash value in \"From \" header.\n\nbrian m. carlson (3):\n Introduce a null_oid constant.\n format-patch: add an option to suppress commit hash\n format-patch: check that header line has expected format\n\n Documentation/git-format-patch.txt | 4 ++++\n builtin/log.c | 5 +++++\n cache.h | 1 +\n log-tree.c | 3 ++-\n revision.h | 1 +\n sha1_file.c | 1 +\n t/t4014-format-patch.sh | 12 ++++++++++++\n 7 files changed, 26 insertions(+), 1 deletion(-)\n"},{"id":"274349","messageId":"1450027638-788102-2-git-send-email-sandals@crustytoothpaste.net","threadId":"40985","inReplyTo":"1450027638-788102-1-git-send-email-sandals@crustytoothpaste.net","subject":"[PATCH v2 1/3] Introduce a null_oid constant.","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2015-12-13T17:27:16Z","receivedAt":"2015-12-13T17:27:16Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"null_oid is the struct object_id equivalent to null_sha1.\n\nSigned-off-by: brian m. carlson \n---\n cache.h | 1 +\n sha1_file.c | 1 +\n 2 files changed, 2 insertions(+)\n\ndiff --git a/cache.h b/cache.h\nindex 5ab6cb50..c63fcc11 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -831,6 +831,7 @@ extern const char *find_unique_abbrev(const unsigned char *sha1, int len);\n extern int find_unique_abbrev_r(char *hex, const unsigned char *sha1, int len);\n \n extern const unsigned char null_sha1[GIT_SHA1_RAWSZ];\n+extern const struct object_id null_oid;\n \n static inline int hashcmp(const unsigned char *sha1, const unsigned char *sha2)\n {\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 27ce7b70..a54deb05 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -36,6 +36,7 @@\n static inline uintmax_t sz_fmt(size_t s) { return s; }\n \n const unsigned char null_sha1[20];\n+const struct object_id null_oid;\n \n /*\n * This is meant to hold a *small* number of objects that you would\n"},{"id":"274351","messageId":"1450027638-788102-3-git-send-email-sandals@crustytoothpaste.net","threadId":"40985","inReplyTo":"1450027638-788102-1-git-send-email-sandals@crustytoothpaste.net","subject":"[PATCH v2 2/3] format-patch: add an option to suppress commit hash","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2015-12-13T17:27:17Z","receivedAt":"2015-12-13T17:27:17Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Oftentimes, patches created by git format-patch will be stored in\nversion control or compared with diff. In these cases, two otherwise\nidentical patches can have different commit hashes, leading to diff\nnoise. Teach git format-patch a --zero-commit option that instead\nproduces an all-zero hash to avoid this diff noise.\n\nSigned-off-by: brian m. carlson \n---\n Documentation/git-format-patch.txt | 4 ++++\n builtin/log.c | 5 +++++\n log-tree.c | 3 ++-\n revision.h | 1 +\n t/t4014-format-patch.sh | 6 ++++++\n 5 files changed, 18 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 40356491..e3cdaeb9 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -256,6 +256,10 @@ you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`.\n \tusing this option cannot be applied properly, but they are\n \tstill useful for code review.\n \n+--zero-commit::\n+ Output an all-zero hash in each patch's From header instead\n+ of the hash of the commit.\n+\n --root::\n \tTreat the revision argument as a , even if it\n \tis just a single commit (that would normally be treated as a\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 069bd3a9..e00cea75 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1196,6 +1196,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tint cover_letter = -1;\n \tint boundary_count = 0;\n \tint no_binary_diff = 0;\n+\tint zero_commit = 0;\n \tstruct commit *origin = NULL;\n \tconst char *in_reply_to = NULL;\n \tstruct patch_ids ids;\n@@ -1236,6 +1237,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t PARSE_OPT_NOARG | PARSE_OPT_NONEG, keep_callback },\n \t\tOPT_BOOL(0, \"no-binary\", &no_binary_diff,\n \t\t\t N_(\"don't output binary diffs\")),\n+\t\tOPT_BOOL(0, \"zero-commit\", &zero_commit,\n+\t\t\t N_(\"output all-zero hash in From header\")),\n \t\tOPT_BOOL(0, \"ignore-if-in-upstream\", &ignore_if_in_upstream,\n \t\t\t N_(\"don't include a patch matching a commit upstream\")),\n \t\t{ OPTION_SET_INT, 'p', \"no-stat\", &use_patch_format, NULL,\n@@ -1380,6 +1383,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t/* Always generate a patch */\n \trev.diffopt.output_format |= DIFF_FORMAT_PATCH;\n \n+\trev.zero_commit = zero_commit;\n+\n \tif (!DIFF_OPT_TST(&rev.diffopt, TEXT) && !no_binary_diff)\n \t\tDIFF_OPT_SET(&rev.diffopt, BINARY);\n \ndiff --git a/log-tree.c b/log-tree.c\nindex 35e78017..f70a30e1 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -342,7 +342,8 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,\n {\n \tconst char *subject = NULL;\n \tconst char *extra_headers = opt->extra_headers;\n-\tconst char *name = oid_to_hex(&commit->object.oid);\n+\tconst char *name = oid_to_hex(opt->zero_commit ?\n+\t\t\t\t &null_oid : &commit->object.oid);\n \n \t*need_8bit_cte_p = 0; /* unknown */\n \tif (opt->total > 0) {\ndiff --git a/revision.h b/revision.h\nindex 5bc96868..23857c0e 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -135,6 +135,7 @@ struct rev_info {\n \t\t\tpretty_given:1,\n \t\t\tabbrev_commit:1,\n \t\t\tabbrev_commit_given:1,\n+\t\t\tzero_commit:1,\n \t\t\tuse_terminator:1,\n \t\t\tmissing_newline:1,\n \t\t\tdate_mode_explicit:1,\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 890db117..b740e3da 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -1431,4 +1431,10 @@ test_expect_success 'cover letter auto user override' '\n \ttest_line_count = 2 list\n '\n \n+test_expect_success 'format-patch --zero-commit' '\n+\tgit format-patch --zero-commit --stdout v2..v1 >patch2 &&\n+\tcnt=$(egrep \"^From 0{40} Mon Sep 17 00:00:00 2001\" patch2 | wc -l) &&\n+\ttest $cnt = 3\n+'\n+\n test_done\n"},{"id":"274350","messageId":"1450027638-788102-4-git-send-email-sandals@crustytoothpaste.net","threadId":"40985","inReplyTo":"1450027638-788102-1-git-send-email-sandals@crustytoothpaste.net","subject":"[PATCH v2 3/3] format-patch: check that header line has expected format","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2015-12-13T17:27:18Z","receivedAt":"2015-12-13T17:27:18Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"The format of the \"From \" header line is very specific to allow\nutilities to detect Git-style patches. Add a test that the patches\ncreated are in the expected format.\n\nSigned-off-by: brian m. carlson \n---\n t/t4014-format-patch.sh | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex b740e3da..362bc228 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -1437,4 +1437,10 @@ test_expect_success 'format-patch --zero-commit' '\n \ttest $cnt = 3\n '\n \n+test_expect_success 'From line has expected format' '\n+\tgit format-patch --stdout v2..v1 >patch2 &&\n+\tcnt=$(egrep \"^From [0-9a-f]{40} Mon Sep 17 00:00:00 2001\" patch2 | wc -l) &&\n+\ttest $cnt = 3\n+'\n+\n test_done\n"},{"id":"274368","messageId":"566E6CDF.3020004@web.de","threadId":"40985","inReplyTo":"1450027638-788102-4-git-send-email-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 3/3] format-patch: check that header line has expected format","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-12-14T07:16:47Z","receivedAt":"2015-12-14T07:16:47Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 13.12.15 18:27, brian m. carlson wrote:\n> The format of the \"From \" header line is very specific to allow\n> utilities to detect Git-style patches. Add a test that the patches\n> created are in the expected format.\n> \n> Signed-off-by: brian m. carlson \n> ---\n> t/t4014-format-patch.sh | 6 ++++++\n> 1 file changed, 6 insertions(+)\n> \n> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n> index b740e3da..362bc228 100755\n> --- a/t/t4014-format-patch.sh\n> +++ b/t/t4014-format-patch.sh\n> @@ -1437,4 +1437,10 @@ test_expect_success 'format-patch --zero-commit' '\n> \ttest $cnt = 3\n> '\n> \n> +test_expect_success 'From line has expected format' '\n> +\tgit format-patch --stdout v2..v1 >patch2 &&\n> +\tcnt=$(egrep \"^From [0-9a-f]{40} Mon Sep 17 00:00:00 2001\" patch2 | wc -l) &&\n> +\ttest $cnt = 3\n\nFor these kind of things:\ntest_line_count() is your friend\n"},{"id":"274395","messageId":"xmqqmvtckfpm.fsf@gitster.mtv.corp.google.com","threadId":"40985","inReplyTo":"1450027638-788102-4-git-send-email-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 3/3] format-patch: check that header line has expected format","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-14T19:11:49Z","receivedAt":"2015-12-14T19:11:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> The format of the \"From \" header line is very specific to allow\n> utilities to detect Git-style patches. Add a test that the patches\n> created are in the expected format.\n>\n> Signed-off-by: brian m. carlson \n> ---\n> t/t4014-format-patch.sh | 6 ++++++\n> 1 file changed, 6 insertions(+)\n>\n> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n> index b740e3da..362bc228 100755\n> --- a/t/t4014-format-patch.sh\n> +++ b/t/t4014-format-patch.sh\n> @@ -1437,4 +1437,10 @@ test_expect_success 'format-patch --zero-commit' '\n> \ttest $cnt = 3\n> '\n> \n> +test_expect_success 'From line has expected format' '\n> +\tgit format-patch --stdout v2..v1 >patch2 &&\n> +\tcnt=$(egrep \"^From [0-9a-f]{40} Mon Sep 17 00:00:00 2001\" patch2 | wc -l) &&\n\nDon't you want to anchor the pattern to the right as well?\n\n> +\ttest $cnt = 3\n> +'\n> +\n> test_done\n"},{"id":"274438","messageId":"20151214213239.GH14788@sigill.intra.peff.net","threadId":"40985","inReplyTo":"1450027638-788102-1-git-send-email-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 0/3] format-patch: introduce option to suppress commit hashes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-12-14T21:32:39Z","receivedAt":"2015-12-14T21:32:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Dec 13, 2015 at 05:27:15PM +0000, brian m. carlson wrote:\n\n> git format-patch is often used to create patches that are then stored in\n> version control or displayed with diff. Having the commit hash in the\n> \"From \" line usually just creates diff noise in these cases, so this\n> series introduces --zero-commit to set that to all zeros.\n> \n> Changes from v1:\n> * Rename the option --zero-commit.\n> * Improve the tests to look for a 40-hex hash value in \"From \" header.\n> \n> brian m. carlson (3):\n> Introduce a null_oid constant.\n> format-patch: add an option to suppress commit hash\n> format-patch: check that header line has expected format\n\nThe intent here makes sense to me, and with the exception of the\ntest_line_count thing that Torsten mentioned, the code looks good.\n\nI briefly wondered if the option should simply be \"--diffable\" or\nsomething like that, and trigger this new behavior as well as implying\n--no-signature. Along with any other relevant options (if any; I don't\nrecall if --stat-width is terminal-dependent for format-patch, for\nexample).\n\nBut that is probably overkill. People can flip those switches\nindividually if they want to (and even if somebody did want\n\"--diffable\", it may make sense to build it on top, so they can flip the\nzero-commit thing individually if they want).\n\n-Peff\n"},{"id":"274440","messageId":"xmqq6100iu4s.fsf@gitster.mtv.corp.google.com","threadId":"40985","inReplyTo":"1450027638-788102-3-git-send-email-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 2/3] format-patch: add an option to suppress commit hash","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-14T21:43:15Z","receivedAt":"2015-12-14T21:43:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> +test_expect_success 'format-patch --zero-commit' '\n> +\tgit format-patch --zero-commit --stdout v2..v1 >patch2 &&\n> +\tcnt=$(egrep \"^From 0{40} Mon Sep 17 00:00:00 2001\" patch2 | wc -l) &&\n> +\ttest $cnt = 3\n> +'\n\nThis test is not wrong per-se, but it makes the test as a whole\nsomewhat brittle. People cannot add new commits in the history\nbeing tested, which would add to the number of patches, without\nadjusting this test, even though all this test cares about is that\nall the mbox \"From \" lines record the zeroed commit object name.\n\n git format-patch --zero-commit --stdout v2..v1 |\n grep \"^From \" | sort | uniq >actual &&\n echo \"From $_z40 Mon Sep 17 00:00:00 2001\" >expect &&\n test_cmp expect actual\n\nor something like that instead?\n"},{"id":"274441","messageId":"xmqqy4cwhfgy.fsf@gitster.mtv.corp.google.com","threadId":"40985","inReplyTo":"1450027638-788102-4-git-send-email-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 3/3] format-patch: check that header line has expected format","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-14T21:45:17Z","receivedAt":"2015-12-14T21:45:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> +test_expect_success 'From line has expected format' '\n> +\tgit format-patch --stdout v2..v1 >patch2 &&\n> +\tcnt=$(egrep \"^From [0-9a-f]{40} Mon Sep 17 00:00:00 2001\" patch2 | wc -l) &&\n\nAlso, with $_x40, you do not need egrep.\n\n> +\ttest $cnt = 3\n> +'\n> +\n> test_done\n"},{"id":"274473","messageId":"20151215003504.GN990758@vauxhall.crustytoothpaste.net","threadId":"40985","inReplyTo":"20151214213239.GH14788@sigill.intra.peff.net","subject":"Re: [PATCH v2 0/3] format-patch: introduce option to suppress commit hashes","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2015-12-15T00:35:04Z","receivedAt":"2015-12-15T00:35:04Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Mon, Dec 14, 2015 at 04:32:39PM -0500, Jeff King wrote:\n> The intent here makes sense to me, and with the exception of the\n> test_line_count thing that Torsten mentioned, the code looks good.\n> \n> I briefly wondered if the option should simply be \"--diffable\" or\n> something like that, and trigger this new behavior as well as implying\n> --no-signature. Along with any other relevant options (if any; I don't\n> recall if --stat-width is terminal-dependent for format-patch, for\n> example).\n> \n> But that is probably overkill. People can flip those switches\n> individually if they want to (and even if somebody did want\n> \"--diffable\", it may make sense to build it on top, so they can flip the\n> zero-commit thing individually if they want).\n\nThat does sound like a potentially worthwhile thing to build on top at\nsome point.\n\nI'll reroll with the other suggested changes and a slight tweak to make\nthe tests less dependent on the history in both cases.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"}]}