{"thread":{"id":"17067","subject":"[PATCH] Get format-patch to show first commit after root commit","startedAt":"2009-01-09T21:33:07Z","lastAt":"2009-01-10T20:41:33Z","messageCount":11,"participants":["Nathan W. Panike","Junio C Hamano","Alexander Potashev"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"99820","messageId":"1231536787-20685-1-git-send-email-nathan.panike@gmail.com","threadId":"17067","inReplyTo":null,"subject":"[PATCH] Get format-patch to show first commit after root commit","fromName":"Nathan W. Panike","fromEmail":"nathan.panike@gmail.com","sentAt":"2009-01-09T21:33:07Z","receivedAt":"2009-01-09T21:33:07Z","isPatch":true,"sender":{"key":"nathan.panike@gmail.com","avatar":"https://avatars.githubusercontent.com/u/389447?v=4"},"body":"Rework this patch to try to handle the case where one does\n\ngit format-patch -n ...\n\nand n is a number larger than 1.  Currently, the command\n\ngit format-patch -1 e83c5163316f89bfbde\n\nin the git repository creates an empty file.  Instead, one is\nforced to do\n\ngit format-patch -1 --root e83c5163316f89bfbde\n\nThis seems arbitrary.  This patch fixes this case, so that\n\ngit format-patch -1 e83c5163316f89bfbde\n\nwill produce an actual patch.\n\nSigned-off-by: Nathan W. Panike <nathan.panike@gmail.com>\n---\n builtin-log.c |    3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 4a02ee9..0eca15f 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -975,6 +975,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tnr++;\n \t\tlist = xrealloc(list, nr * sizeof(list[0]));\n \t\tlist[nr - 1] = commit;\n+\t\tif(!commit->parents){\n+\t\t\trev.show_root_diff=1;\n+\t\t}\n \t}\n \ttotal = nr;\n \tif (!keep_subject && auto_number && total > 1)\n-- \n1.6.1.76.gc123b.dirty\n"},{"id":"99829","messageId":"7vmye0yohu.fsf@gitster.siamese.dyndns.org","threadId":"17067","inReplyTo":"1231536787-20685-1-git-send-email-nathan.panike@gmail.com","subject":"Re: [PATCH] Get format-patch to show first commit after root commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-10T00:49:01Z","receivedAt":"2009-01-10T00:49:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Nathan W. Panike\" <nathan.panike@gmail.com> writes:\n\n> Rework this patch to try to handle the case where one does\n>\n> git format-patch -n ...\n>\n> and n is a number larger than 1.\n\nIt is unclear what \"this patch\" is in the context of this proposed commit\nmessage.\n\n> git format-patch -1 e83c5163316f89bfbde\n> ...\n\nI do not think the current backward compatibile behaviour to avoid\nsurprising the end user by creating a huge initial import diff is\nparticularly a good idea.\n\nI do not see anything special you do for \"one commit\" case in your patch,\nyet the proposed commit message keeps stressing \"-1\", which puzzles me.\n\nWouldn't it suffice to simply say something like:\n\n    You need to explicitly ask for --root to obtain a patch for the root\n    commit.  This may have been a good way to make sure that the user\n    realizes that a patch from the root commit won't be applicable to a\n    history with existing data, but we should assume the user knows what\n    he is doing when the user explicitly specifies a range of commits that\n    includes the root commit.\n\nPerhaps there are some other downsides I may not remember why --root is\nnot the default, though.\n\n> Signed-off-by: Nathan W. Panike <nathan.panike@gmail.com>\n> ---\n>  builtin-log.c |    3 +++\n>  1 files changed, 3 insertions(+), 0 deletions(-)\n>\n> diff --git a/builtin-log.c b/builtin-log.c\n> index 4a02ee9..0eca15f 100644\n> --- a/builtin-log.c\n> +++ b/builtin-log.c\n> @@ -975,6 +975,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n>  \t\tnr++;\n>  \t\tlist = xrealloc(list, nr * sizeof(list[0]));\n>  \t\tlist[nr - 1] = commit;\n> +\t\tif(!commit->parents){\n> +\t\t\trev.show_root_diff=1;\n> +\t\t}\n\nThree issues.\n\n - The \"if(){\" violates style by not having one SP before \"(\" and after \")\",\n   and surrounds a single statement with needless { } pair.  You need one SP\n   on each side of the = (assignment) as well.\n\n - Because rev.show_root_diff is a no-op for non-root commit anyway, I do not\n   think you even want a conditional there.\n\n - It is a bad style to muck with rev.* while it is actively used for\n   iteration (note that the above part is in a while loop that iterates over\n   &rev).\n\nI think the attached would be a better patch.  We already have a\nconfiguration to control if we show the patch for a root commit by\ndefault, and we can use reuse it here.  The configuration defaults to true\nthese days.\n\nBecause the code before the hunk must check if the user said \"--root\ncommit\" or just \"commit\" from the command line and behave quite\ndifferently by looking at rev.show_root_diff, we cannot do this assignment\nbefore the command line parsing like other commands in the log family.\n\n builtin-log.c |    8 ++++++++\n 1 files changed, 8 insertions(+), 0 deletions(-)\n\ndiff --git c/builtin-log.c w/builtin-log.c\nindex 4a02ee9..2d2c111 100644\n--- c/builtin-log.c\n+++ w/builtin-log.c\n@@ -935,6 +935,14 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t * get_revision() to do the usual traversal.\n \t\t */\n \t}\n+\n+\t/*\n+\t * We cannot move this anywhere earlier because we do want to\n+\t * know if --root was given explicitly from the comand line.\n+\t */\n+\tif (default_show_root)\n+\t\trev.show_root_diff = 1;\n+\n \tif (cover_letter) {\n \t\t/* remember the range */\n \t\tint i;\n"},{"id":"99840","messageId":"d77df1110901091737k6c4fb826tb2287072db2e36a@mail.gmail.com","threadId":"17067","inReplyTo":"7vmye0yohu.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Get format-patch to show first commit after root commit","fromName":"Nathan W. Panike","fromEmail":"nathan.panike@gmail.com","sentAt":"2009-01-10T01:37:41Z","receivedAt":"2009-01-10T01:37:41Z","isPatch":true,"sender":{"key":"nathan.panike@gmail.com","avatar":"https://avatars.githubusercontent.com/u/389447?v=4"},"body":"Hi:\n\nOn Fri, Jan 9, 2009 at 6:49 PM, Junio C Hamano <gitster@pobox.com> wrote:\n\n> I do not see anything special you do for \"one commit\" case in your patch,\n> yet the proposed commit message keeps stressing \"-1\", which puzzles me.\n\nI was trying to address Alexander's concerns he brought up previously\nin the thread.\n\n> Wouldn't it suffice to simply say something like:\n>\n>    You need to explicitly ask for --root to obtain a patch for the root\n>    commit.  This may have been a good way to make sure that the user\n>    realizes that a patch from the root commit won't be applicable to a\n>    history with existing data, but we should assume the user knows what\n>    he is doing when the user explicitly specifies a range of commits that\n>    includes the root commit.\n>\n\nIndeed it would.  I was giving a specific case that shows what problem\nthis patch addresses.\n\n> Three issues.\n>\n>  - The \"if(){\" violates style by not having one SP before \"(\" and after \")\",\n>   and surrounds a single statement with needless { } pair.  You need one SP\n>   on each side of the = (assignment) as well.\n>\n>  - Because rev.show_root_diff is a no-op for non-root commit anyway, I do not\n>   think you even want a conditional there.\n>\n>  - It is a bad style to muck with rev.* while it is actively used for\n>   iteration (note that the above part is in a while loop that iterates over\n>   &rev).\n\nThanks for the advice.  I shall adhere to it next time I submit a patch.\n\n> I think the attached would be a better patch.  We already have a\n> configuration to control if we show the patch for a root commit by\n> default, and we can use reuse it here.  The configuration defaults to true\n> these days.\n\nI did not realize this configuration was available.  The patch below\nis much more elegant.\n\n> Because the code before the hunk must check if the user said \"--root\n> commit\" or just \"commit\" from the command line and behave quite\n> differently by looking at rev.show_root_diff, we cannot do this assignment\n> before the command line parsing like other commands in the log family.\n>\n>  builtin-log.c |    8 ++++++++\n>  1 files changed, 8 insertions(+), 0 deletions(-)\n>\n> diff --git c/builtin-log.c w/builtin-log.c\n> index 4a02ee9..2d2c111 100644\n> --- c/builtin-log.c\n> +++ w/builtin-log.c\n> @@ -935,6 +935,14 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n>                 * get_revision() to do the usual traversal.\n>                 */\n>        }\n> +\n> +       /*\n> +        * We cannot move this anywhere earlier because we do want to\n> +        * know if --root was given explicitly from the comand line.\n> +        */\n> +       if (default_show_root)\n> +               rev.show_root_diff = 1;\n> +\n>        if (cover_letter) {\n>                /* remember the range */\n>                int i;\n>\n\nThanks,\n\nNathan Panike\n"},{"id":"99873","messageId":"20090110113642.GA25723@myhost","threadId":"17067","inReplyTo":"7vmye0yohu.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Get format-patch to show first commit after root commit","fromName":"Alexander Potashev","fromEmail":"aspotashev@gmail.com","sentAt":"2009-01-10T11:36:42Z","receivedAt":"2009-01-10T11:36:42Z","isPatch":true,"sender":{"key":"aspotashev@gmail.com","avatar":null},"body":"Hello, Junio!\n\n> I think the attached would be a better patch.  We already have a\n> configuration to control if we show the patch for a root commit by\n> default, and we can use reuse it here.  The configuration defaults to true\n> these days.\n> \n> Because the code before the hunk must check if the user said \"--root\n> commit\" or just \"commit\" from the command line and behave quite\n> differently by looking at rev.show_root_diff, we cannot do this assignment\n> before the command line parsing like other commands in the log family.\n\nI think the problem was not only format-patch misbehaviour. If you use\n\"log.showroot = no\", you still get an empty patch file, which is not\nvery good, because format-patch doesn't work very well, it creates\n_corrupt_ patches! It's much better to not create the patch file at all\nin this case.\n\nHowever, it has nothing in common with your patch, but there's room for\nanother commit.\n\n> \n>  builtin-log.c |    8 ++++++++\n>  1 files changed, 8 insertions(+), 0 deletions(-)\n> \n> diff --git c/builtin-log.c w/builtin-log.c\n> index 4a02ee9..2d2c111 100644\n> --- c/builtin-log.c\n> +++ w/builtin-log.c\n> @@ -935,6 +935,14 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n>  \t\t * get_revision() to do the usual traversal.\n>  \t\t */\n>  \t}\n> +\n> +\t/*\n> +\t * We cannot move this anywhere earlier because we do want to\n> +\t * know if --root was given explicitly from the comand line.\n> +\t */\n> +\tif (default_show_root)\n> +\t\trev.show_root_diff = 1;\n> +\n>  \tif (cover_letter) {\n>  \t\t/* remember the range */\n>  \t\tint i;\n"},{"id":"99875","messageId":"20090110113903.GB25723@myhost","threadId":"17067","inReplyTo":"20090110113642.GA25723@myhost","subject":"[PATCH] format-patch: avoid generation of empty patches","fromName":"Alexander Potashev","fromEmail":"aspotashev@gmail.com","sentAt":"2009-01-10T11:39:03Z","receivedAt":"2009-01-10T11:39:03Z","isPatch":true,"sender":{"key":"aspotashev@gmail.com","avatar":null},"body":"If 'log.showroot' is not set, format-patch shouldn't even try to create\na patch for the root commit.\n\nSigned-off-by: Alexander Potashev <aspotashev@gmail.com>\n---\n builtin-log.c |    3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 4a02ee9..62134d4 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -972,6 +972,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t\thas_commit_patch_id(commit, &ids))\n \t\t\tcontinue;\n \n+\t\tif (!commit->parents && !rev.show_root_diff)\n+\t\t\tbreak;\n+\n \t\tnr++;\n \t\tlist = xrealloc(list, nr * sizeof(list[0]));\n \t\tlist[nr - 1] = commit;\n-- \n1.6.1.77.g569c.dirty\n"},{"id":"99902","messageId":"d77df1110901100801s463bb43bt701a95df14f167d8@mail.gmail.com","threadId":"17067","inReplyTo":"20090110113903.GB25723@myhost","subject":"Re: [PATCH] format-patch: avoid generation of empty patches","fromName":"Nathan W. Panike","fromEmail":"nathan.panike@gmail.com","sentAt":"2009-01-10T16:01:00Z","receivedAt":"2009-01-10T16:01:00Z","isPatch":true,"sender":{"key":"nathan.panike@gmail.com","avatar":"https://avatars.githubusercontent.com/u/389447?v=4"},"body":"Hi:\n\nOn Sat, Jan 10, 2009 at 5:39 AM, Alexander Potashev\n<aspotashev@gmail.com> wrote:\n...\n>\n> +               if (!commit->parents && !rev.show_root_diff)\n> +                       break;\n\nDo you really want to stop getting commits?  It seems like the break\nstatement here should be a continue.\n\nNathan Panike\n"},{"id":"99904","messageId":"20090110161722.GA18859@myhost","threadId":"17067","inReplyTo":"d77df1110901100801s463bb43bt701a95df14f167d8@mail.gmail.com","subject":"Re: [PATCH] format-patch: avoid generation of empty patches","fromName":"Alexander Potashev","fromEmail":"aspotashev@gmail.com","sentAt":"2009-01-10T16:17:22Z","receivedAt":"2009-01-10T16:17:22Z","isPatch":true,"sender":{"key":"aspotashev@gmail.com","avatar":null},"body":"On 10:01 Sat 10 Jan     , Nathan W. Panike wrote:\n> Hi:\n> \n> On Sat, Jan 10, 2009 at 5:39 AM, Alexander Potashev\n> <aspotashev@gmail.com> wrote:\n> ...\n> >\n> > +               if (!commit->parents && !rev.show_root_diff)\n> > +                       break;\n> \n> Do you really want to stop getting commits?  It seems like the break\n> statement here should be a continue.\n\nAFAIU get_revision stops revision iteration when it appears to stay at\nthe root commit. So, if we will replace 'break' with 'continue', the\n'while' loop will finish right after that 'continue'.\n\nHowever, I might be wrong... please, correct me then.\n\n> \n> Nathan Panike\n"},{"id":"99908","messageId":"1231605577-26148-1-git-send-email-aspotashev@gmail.com","threadId":"17067","inReplyTo":"20090110113903.GB25723@myhost","subject":"[PATCH] Add new testcases for format-patch root commits","fromName":"Alexander Potashev","fromEmail":"aspotashev@gmail.com","sentAt":"2009-01-10T16:39:37Z","receivedAt":"2009-01-10T16:39:37Z","isPatch":true,"sender":{"key":"aspotashev@gmail.com","avatar":null},"body":"1. format-patch'ing root commit shouldn't create empty patches\n2. With --root it should create a patch for the root commit\n3. Similar testcases with two commits in the tree\n\nSigned-off-by: Alexander Potashev <aspotashev@gmail.com>\n---\n\ngit format-patch lacks a '--no-root' option, so I used\n'git config log.showroot false' to emulate it.\n\n\n\n t/t4033-format-patch-root-commit.sh |   52 +++++++++++++++++++++++++++++++++++\n 1 files changed, 52 insertions(+), 0 deletions(-)\n create mode 100755 t/t4033-format-patch-root-commit.sh\n\ndiff --git a/t/t4033-format-patch-root-commit.sh b/t/t4033-format-patch-root-commit.sh\nnew file mode 100755\nindex 0000000..846c11c\n--- /dev/null\n+++ b/t/t4033-format-patch-root-commit.sh\n@@ -0,0 +1,52 @@\n+#!/bin/sh\n+\n+test_description='Format-patch root commit skipping/allowing'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\tgit config log.showroot false\n+\tgit config format.numbered false\n+\techo A > file &&\n+\tgit add file &&\n+\tgit commit -m First\n+'\n+\n+test_patch_count() {\n+\tcnt=$(grep \"^Subject: \\[PATCH\\]\" $1 | wc -l) &&\n+\ttest $cnt = $2\n+}\n+\n+test_patch_is_single() {\n+\tcnt=$(grep \"^Subject: \\[PATCH\\] $2\" $1 | wc -l) &&\n+\ttest $cnt = 1\n+}\n+\n+test_expect_success 'format-patch root commit with showroot = false' '\n+\tgit format-patch -1 &&\n+\ttest_must_fail cat 0001-First.patch\n+'\n+\n+test_expect_success 'format-patch root commit' '\n+\tgit format-patch --root --stdout -5 >root-only.patch &&\n+\ttest_patch_count root-only.patch 1 &&\n+\ttest_patch_is_single root-only.patch First\n+'\n+\n+test_expect_success 'format-patch 2 commits without root' '\n+\techo B > file &&\n+\tgit commit -a -m Second &&\n+\n+\tgit format-patch --stdout -2 >two-except-root.patch &&\n+\ttest_patch_count two-except-root.patch 1 &&\n+\ttest_patch_is_single two-except-root.patch Second\n+'\n+\n+test_expect_success 'format-patch 2 commits including root' '\n+\tgit format-patch --root --stdout -2 >two-with-root.patch &&\n+\ttest_patch_count two-with-root.patch 2 &&\n+\ttest_patch_is_single two-with-root.patch First &&\n+\ttest_patch_is_single two-with-root.patch Second\n+'\n+\n+test_done\n-- \n1.6.1.81.g61cf1\n"},{"id":"99915","messageId":"d77df1110901101007o1d17142aub6971b72ba55c89e@mail.gmail.com","threadId":"17067","inReplyTo":"20090110161722.GA18859@myhost","subject":"Re: [PATCH] format-patch: avoid generation of empty patches","fromName":"Nathan W. Panike","fromEmail":"nathan.panike@gmail.com","sentAt":"2009-01-10T18:07:06Z","receivedAt":"2009-01-10T18:07:06Z","isPatch":true,"sender":{"key":"nathan.panike@gmail.com","avatar":"https://avatars.githubusercontent.com/u/389447?v=4"},"body":"Hi:\n\nOn Sat, Jan 10, 2009 at 10:17 AM, Alexander Potashev\n<aspotashev@gmail.com> wrote:\n\n...\n\n> AFAIU get_revision stops revision iteration when it appears to stay at\n> the root commit. So, if we will replace 'break' with 'continue', the\n> 'while' loop will finish right after that 'continue'.\n\n> However, I might be wrong... please, correct me then.\n\nI was thinking of the case where there is more than one root.  Maybe\nthe code does the right thing then, but I confess I have not looked at\nit deeply enough to know.\n\nNathan Panike\n"},{"id":"99916","messageId":"20090110183339.GA30548@myhost","threadId":"17067","inReplyTo":"1231605577-26148-1-git-send-email-aspotashev@gmail.com","subject":"Re: [PATCH] Add new testcases for format-patch root commits","fromName":"Alexander Potashev","fromEmail":"aspotashev@gmail.com","sentAt":"2009-01-10T18:33:39Z","receivedAt":"2009-01-10T18:33:39Z","isPatch":true,"sender":{"key":"aspotashev@gmail.com","avatar":null},"body":"On 19:39 Sat 10 Jan     , Alexander Potashev wrote:\n> 1. format-patch'ing root commit shouldn't create empty patches\n> 2. With --root it should create a patch for the root commit\n> 3. Similar testcases with two commits in the tree\n> \n> Signed-off-by: Alexander Potashev <aspotashev@gmail.com>\n> ---\n> \n> git format-patch lacks a '--no-root' option, so I used\n> 'git config log.showroot false' to emulate it.\n\nSorry, --root option has nothing in common with log.showroot, but the\ntestcases are still valid.\n\n> \n> \n> \n>  t/t4033-format-patch-root-commit.sh |   52 +++++++++++++++++++++++++++++++++++\n>  1 files changed, 52 insertions(+), 0 deletions(-)\n>  create mode 100755 t/t4033-format-patch-root-commit.sh\n> \n> diff --git a/t/t4033-format-patch-root-commit.sh b/t/t4033-format-patch-root-commit.sh\n> new file mode 100755\n> index 0000000..846c11c\n> --- /dev/null\n> +++ b/t/t4033-format-patch-root-commit.sh\n> @@ -0,0 +1,52 @@\n> +#!/bin/sh\n> +\n> +test_description='Format-patch root commit skipping/allowing'\n> +\n> +. ./test-lib.sh\n> +\n> +test_expect_success setup '\n> +\tgit config log.showroot false\n> +\tgit config format.numbered false\n> +\techo A > file &&\n> +\tgit add file &&\n> +\tgit commit -m First\n> +'\n> +\n> +test_patch_count() {\n> +\tcnt=$(grep \"^Subject: \\[PATCH\\]\" $1 | wc -l) &&\n> +\ttest $cnt = $2\n> +}\n> +\n> +test_patch_is_single() {\n> +\tcnt=$(grep \"^Subject: \\[PATCH\\] $2\" $1 | wc -l) &&\n> +\ttest $cnt = 1\n> +}\n> +\n> +test_expect_success 'format-patch root commit with showroot = false' '\n> +\tgit format-patch -1 &&\n> +\ttest_must_fail cat 0001-First.patch\n> +'\n> +\n> +test_expect_success 'format-patch root commit' '\n> +\tgit format-patch --root --stdout -5 >root-only.patch &&\n> +\ttest_patch_count root-only.patch 1 &&\n> +\ttest_patch_is_single root-only.patch First\n> +'\n> +\n> +test_expect_success 'format-patch 2 commits without root' '\n> +\techo B > file &&\n> +\tgit commit -a -m Second &&\n> +\n> +\tgit format-patch --stdout -2 >two-except-root.patch &&\n> +\ttest_patch_count two-except-root.patch 1 &&\n> +\ttest_patch_is_single two-except-root.patch Second\n> +'\n> +\n> +test_expect_success 'format-patch 2 commits including root' '\n> +\tgit format-patch --root --stdout -2 >two-with-root.patch &&\n> +\ttest_patch_count two-with-root.patch 2 &&\n> +\ttest_patch_is_single two-with-root.patch First &&\n> +\ttest_patch_is_single two-with-root.patch Second\n> +'\n> +\n> +test_done\n> -- \n> 1.6.1.81.g61cf1\n> \n"},{"id":"99924","messageId":"7vy6xivqpu.fsf@gitster.siamese.dyndns.org","threadId":"17067","inReplyTo":"d77df1110901100801s463bb43bt701a95df14f167d8@mail.gmail.com","subject":"Re: [PATCH] format-patch: avoid generation of empty patches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-10T20:41:33Z","receivedAt":"2009-01-10T20:41:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Nathan W. Panike\" <nathan.panike@gmail.com> writes:\n\n> On Sat, Jan 10, 2009 at 5:39 AM, Alexander Potashev\n> <aspotashev@gmail.com> wrote:\n> ...\n>>\n>> +               if (!commit->parents && !rev.show_root_diff)\n>> +                       break;\n>\n> Do you really want to stop getting commits?  It seems like the break\n> statement here should be a continue.\n\nYou can give a commit range that has two independent roots.  The above\n\"break\" is wrong.\n\nThe variable is called show_root_DIFF, not show_root_COMMIT; even if you\nhave \"log.showroot = false\", \"git log -p\" output would still give you the\ninitial commit, but without the patch text, no?\n\nBut that is not Alexander's fault; it is mine.\n\nI think \"log -p\" and \"format-patch\" can and should behave differently in\nthis case.  \"log -p\" is for people who already _have_ the history and\nwould want to inspect how it evolved, and it is reasonable if some people\nwant to say \"the very initial huge import is not interesting to me while\nreviewing the history\", and turning it off makes sense for them (in fact,\nthe default was initially that way).\n\nOn the other hand, \"format-patch\" is about exporting a part of your\nhistory so that you can mechanincally replay it elsewhere, and I do not\nthink of a reasonable justification not to export a root commit fully if\nthe range user asked for happens to contain one.\n\nI agree with Alexander that we should not output just the message without\nthe patch text, but I think the right solution is to show both. not to\nskip root.\n\n-- >8 --\nformat-patch: show patch text for the root commit\n\nEven without --root specified, if the range given on the command line\nhappens to include a root commit, we should include its patch text in the\noutput.\n\nThis fix deliberately ignores log.showroot configuration variable because\n\"format-patch\" and \"log -p\" can and should behave differently in this\ncase, as the former is about exporting a part of your history in a form\nthat is replayable elsewhere and just giving the commit log message\nwithout the patch text does not make any sense for that purpose.\n\nNoticed and fix originally attempted by Nathan W. Panike; credit goes to\nAlexander Potashev for injecting sanity to my initial (broken) fix that\nused the value from log.showroot configuration, which was misguided.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-log.c |    7 +++++++\n 1 files changed, 7 insertions(+), 0 deletions(-)\n\ndiff --git c/builtin-log.c w/builtin-log.c\nindex 4a02ee9..91e5412 100644\n--- c/builtin-log.c\n+++ w/builtin-log.c\n@@ -935,6 +935,13 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t * get_revision() to do the usual traversal.\n \t\t */\n \t}\n+\n+\t/*\n+\t * We cannot move this anywhere earlier because we do want to\n+\t * know if --root was given explicitly from the comand line.\n+\t */\n+\trev.show_root_diff = 1;\n+\n \tif (cover_letter) {\n \t\t/* remember the range */\n \t\tint i;\n"}]}