{"thread":{"id":"50586","subject":"[BUG] git log -L ... -s does not suppress diff output","startedAt":"2019-02-25T17:04:07Z","lastAt":"2019-02-25T18:55:25Z","messageCount":6,"participants":["Matthew Booth","Jeff King","SZEDER Gábor","Randall S. Becker"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"370117","messageId":"CAEkQehdFu5zM4AY3ihN0pn1aCNEomY0WV07pryfAB45JN-tDDA@mail.gmail.com","threadId":"50586","inReplyTo":null,"subject":"[BUG] git log -L ... -s does not suppress diff output","fromName":"Matthew Booth","fromEmail":"mbooth@redhat.com","sentAt":"2019-02-25T17:03:50Z","receivedAt":"2019-02-25T17:04:07Z","isPatch":false,"sender":{"key":"mbooth@redhat.com","avatar":null},"body":"Example output:\n\n=========\n$ git --version\ngit version 2.20.1\n\n$ git log -L 2957,3107:nova/compute/manager.py -s\ncommit 35ce77835bb271bad3c18eaf22146edac3a42ea0\n<snip>\n\ndiff --git a/nova/compute/manager.py b/nova/compute/manager.py\n--- a/nova/compute/manager.py\n+++ b/nova/compute/manager.py\n@@ -2937,152 +2921,151 @@\n     def rebuild_instance(self, context, instance, orig_image_ref, image_ref,\n                          injected_files, new_pass, orig_sys_metadata,\n<snip>\n=========\n\ngit log docs suggest it should not do this:\n\n       -s, --no-patch\n           Suppress diff output. Useful for commands like git show\nthat show the patch by default, or to cancel\n           the effect of --patch.\n\nCouldn't find anything in a search of the archives of this mailing\nlist, although that's obviously far from conclusive. Seems to be\nlongstanding, as it was mentioned on StackOverflow back in 2015:\n\nhttps://stackoverflow.com/questions/31709785/git-line-log-git-l-suppress-diff\n\nMatt\n-- \nMatthew Booth\nRed Hat OpenStack Engineer, Compute DFG\n\nPhone: +442070094448 (UK)\n"},{"id":"370118","messageId":"20190225171817.GA17524@sigill.intra.peff.net","threadId":"50586","inReplyTo":"CAEkQehdFu5zM4AY3ihN0pn1aCNEomY0WV07pryfAB45JN-tDDA@mail.gmail.com","subject":"Re: [BUG] git log -L ... -s does not suppress diff output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-25T17:18:17Z","receivedAt":"2019-02-25T17:18:26Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 25, 2019 at 05:03:50PM +0000, Matthew Booth wrote:\n\n> Example output:\n> \n> =========\n> $ git --version\n> git version 2.20.1\n> \n> $ git log -L 2957,3107:nova/compute/manager.py -s\n> commit 35ce77835bb271bad3c18eaf22146edac3a42ea0\n> <snip>\n> \n> diff --git a/nova/compute/manager.py b/nova/compute/manager.py\n> --- a/nova/compute/manager.py\n> +++ b/nova/compute/manager.py\n> @@ -2937,152 +2921,151 @@\n>      def rebuild_instance(self, context, instance, orig_image_ref, image_ref,\n>                           injected_files, new_pass, orig_sys_metadata,\n> <snip>\n> =========\n\nAt first I wondered why you would want to do this, since the point of -L\nis to walk through that diff. But I suppose you might want to see just\nthe commits, without the actual patch, and that's what \"-s\" ought to do.\n\n> git log docs suggest it should not do this:\n> \n>        -s, --no-patch\n>            Suppress diff output. Useful for commands like git show\n> that show the patch by default, or to cancel\n>            the effect of --patch.\n> \n> Couldn't find anything in a search of the archives of this mailing\n> list, although that's obviously far from conclusive. Seems to be\n> longstanding, as it was mentioned on StackOverflow back in 2015:\n\nI think the issue is just that \"-L\" follows a very different code path\nthan the normal diff generator. Perhaps something like this helps?\n\ndiff --git a/line-log.c b/line-log.c\nindex 63df51a08f..ed46a3a493 100644\n--- a/line-log.c\n+++ b/line-log.c\n@@ -1106,7 +1106,8 @@ int line_log_print(struct rev_info *rev, struct commit *commit)\n \tstruct line_log_data *range = lookup_line_range(rev, commit);\n \n \tshow_log(rev);\n-\tdump_diff_hacky(rev, range);\n+\tif (!(rev->diffopt.output_format & DIFF_FORMAT_NO_OUTPUT))\n+\t\tdump_diff_hacky(rev, range);\n \treturn 1;\n }\n \n\n-Peff\n"},{"id":"370119","messageId":"20190225173248.GB17524@sigill.intra.peff.net","threadId":"50586","inReplyTo":"20190225171817.GA17524@sigill.intra.peff.net","subject":"[PATCH] line-log: suppress diff output with \"-s\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-25T17:32:49Z","receivedAt":"2019-02-25T17:32:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 25, 2019 at 12:18:17PM -0500, Jeff King wrote:\n\n> > git log docs suggest it should not do this:\n> > \n> >        -s, --no-patch\n> >            Suppress diff output. Useful for commands like git show\n> > that show the patch by default, or to cancel\n> >            the effect of --patch.\n> > \n> > Couldn't find anything in a search of the archives of this mailing\n> > list, although that's obviously far from conclusive. Seems to be\n> > longstanding, as it was mentioned on StackOverflow back in 2015:\n> \n> I think the issue is just that \"-L\" follows a very different code path\n> than the normal diff generator. Perhaps something like this helps?\n\nHere it is with a test and a commit message (I don't think any doc\nupdate is necessary; as you noted, the docs already imply this is what\nshould happen).\n\n-- >8 --\nSubject: [PATCH] line-log: suppress diff output with \"-s\"\n\nWhen \"-L\" is in use, we ignore any diff output format that the user\nprovides to us, and just always print a patch (with extra context lines\ncovering the whole area of interest). It's not entirely clear what we\nshould do with all formats (e.g., should \"--stat\" show just the diffstat\nof the touched lines, or the stat for the whole file?).\n\nBut \"-s\" is pretty clear: the user probably wants to see just the\ncommits that touched those lines, without any diff at all. Let's at\nleast make that work.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThis punts completely on the larger question of what should happen with\nother formats like \"--stat\", \"--raw\", etc. They'll continue to be\nignored entirely and we'll generate the line-log patch. Possibly we\nshould detect and complain?\n\n line-log.c          | 3 ++-\n t/t4211-line-log.sh | 7 +++++++\n 2 files changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/line-log.c b/line-log.c\nindex 24e21731c4..863f5cbe0f 100644\n--- a/line-log.c\n+++ b/line-log.c\n@@ -1106,7 +1106,8 @@ int line_log_print(struct rev_info *rev, struct commit *commit)\n \tstruct line_log_data *range = lookup_line_range(rev, commit);\n \n \tshow_log(rev);\n-\tdump_diff_hacky(rev, range);\n+\tif (!(rev->diffopt.output_format & DIFF_FORMAT_NO_OUTPUT))\n+\t\tdump_diff_hacky(rev, range);\n \treturn 1;\n }\n \ndiff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh\nindex bd5fe4d148..c9f2036f68 100755\n--- a/t/t4211-line-log.sh\n+++ b/t/t4211-line-log.sh\n@@ -115,4 +115,11 @@ test_expect_success 'range_set_union' '\n \tgit log $(for x in $(test_seq 200); do echo -L $((2*x)),+1:c.c; done)\n '\n \n+test_expect_success '-s shows only line-log commits' '\n+\tgit log --format=\"commit %s\" -L1,24:b.c >expect.raw &&\n+\tgrep ^commit expect.raw >expect &&\n+\tgit log --format=\"commit %s\" -L1,24:b.c -s >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.21.0.672.g12e864cee7\n\n"},{"id":"370120","messageId":"20190225175918.GV1622@szeder.dev","threadId":"50586","inReplyTo":"20190225173248.GB17524@sigill.intra.peff.net","subject":"Re: [PATCH] line-log: suppress diff output with \"-s\"","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-02-25T17:59:18Z","receivedAt":"2019-02-25T17:59:24Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Feb 25, 2019 at 12:32:49PM -0500, Jeff King wrote:\n> diff --git a/line-log.c b/line-log.c\n> index 24e21731c4..863f5cbe0f 100644\n> --- a/line-log.c\n> +++ b/line-log.c\n> @@ -1106,7 +1106,8 @@ int line_log_print(struct rev_info *rev, struct commit *commit)\n>  \tstruct line_log_data *range = lookup_line_range(rev, commit);\n\nNote that the result of this lookup_line_range() call is only used\nwhen we do show the diff below; if we don't, there is no use calling\nit.\n\n>  \tshow_log(rev);\n> -\tdump_diff_hacky(rev, range);\n> +\tif (!(rev->diffopt.output_format & DIFF_FORMAT_NO_OUTPUT))\n> +\t\tdump_diff_hacky(rev, range);\n>  \treturn 1;\n>  }\n>  \n"},{"id":"370122","messageId":"001701d4cd3a$6441d420$2cc57c60$@nexbridge.com","threadId":"50586","inReplyTo":"20190225171817.GA17524@sigill.intra.peff.net","subject":"RE: [BUG] git log -L ... -s does not suppress diff output","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2019-02-25T18:46:11Z","receivedAt":"2019-02-25T18:46:26Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On February 25, 2019 12:18, Jeff King wrote:\n> To: Matthew Booth <mbooth@redhat.com>\n> Cc: git@vger.kernel.org\n> Subject: Re: [BUG] git log -L ... -s does not suppress diff output\n> \n> On Mon, Feb 25, 2019 at 05:03:50PM +0000, Matthew Booth wrote:\n> \n> > Example output:\n> >\n> > =========\n> > $ git --version\n> > git version 2.20.1\n> >\n> > $ git log -L 2957,3107:nova/compute/manager.py -s commit\n> > 35ce77835bb271bad3c18eaf22146edac3a42ea0\n> > <snip>\n> >\n> > diff --git a/nova/compute/manager.py b/nova/compute/manager.py\n> > --- a/nova/compute/manager.py\n> > +++ b/nova/compute/manager.py\n> > @@ -2937,152 +2921,151 @@\n> >      def rebuild_instance(self, context, instance, orig_image_ref, image_ref,\n> >                           injected_files, new_pass, orig_sys_metadata,\n> > <snip> =========\n> \n> At first I wondered why you would want to do this, since the point of -L is to\n> walk through that diff. But I suppose you might want to see just the commits,\n> without the actual patch, and that's what \"-s\" ought to do.\n> \n> > git log docs suggest it should not do this:\n> >\n> >        -s, --no-patch\n> >            Suppress diff output. Useful for commands like git show\n> > that show the patch by default, or to cancel\n> >            the effect of --patch.\n> >\n> > Couldn't find anything in a search of the archives of this mailing\n> > list, although that's obviously far from conclusive. Seems to be\n> > longstanding, as it was mentioned on StackOverflow back in 2015:\n> \n> I think the issue is just that \"-L\" follows a very different code path than the\n> normal diff generator. Perhaps something like this helps?\n> \n> diff --git a/line-log.c b/line-log.c\n> index 63df51a08f..ed46a3a493 100644\n> --- a/line-log.c\n> +++ b/line-log.c\n> @@ -1106,7 +1106,8 @@ int line_log_print(struct rev_info *rev, struct\n> commit *commit)\n>  \tstruct line_log_data *range = lookup_line_range(rev, commit);\n> \n>  \tshow_log(rev);\n> -\tdump_diff_hacky(rev, range);\n> +\tif (!(rev->diffopt.output_format & DIFF_FORMAT_NO_OUTPUT))\n> +\t\tdump_diff_hacky(rev, range);\n>  \treturn 1;\n>  }\n\nI hit this about 6 months ago while trying to show off git to some colleagues - it was on 2.8.5. Sadly I forgot about it. Glad it came back.\nThanks.\n\n"},{"id":"370123","messageId":"20190225185519.GA31135@sigill.intra.peff.net","threadId":"50586","inReplyTo":"20190225175918.GV1622@szeder.dev","subject":"Re: [PATCH] line-log: suppress diff output with \"-s\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-25T18:55:19Z","receivedAt":"2019-02-25T18:55:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 25, 2019 at 06:59:18PM +0100, SZEDER Gábor wrote:\n\n> On Mon, Feb 25, 2019 at 12:32:49PM -0500, Jeff King wrote:\n> > diff --git a/line-log.c b/line-log.c\n> > index 24e21731c4..863f5cbe0f 100644\n> > --- a/line-log.c\n> > +++ b/line-log.c\n> > @@ -1106,7 +1106,8 @@ int line_log_print(struct rev_info *rev, struct commit *commit)\n> >  \tstruct line_log_data *range = lookup_line_range(rev, commit);\n> \n> Note that the result of this lookup_line_range() call is only used\n> when we do show the diff below; if we don't, there is no use calling\n> it.\n\nThanks. I think sometimes we can depend on an optimizing compiler to\ndelay the initialization until use, but in this case the function call\nis much too complex to be reordered. Here's a v2 which bumps it down.\n\n-- >8 --\nSubject: [PATCH v2] line-log: suppress diff output with \"-s\"\n\nWhen \"-L\" is in use, we ignore any diff output format that the user\nprovides to us, and just always print a patch (with extra context lines\ncovering the whole area of interest). It's not entirely clear what we\nshould do with all formats (e.g., should \"--stat\" show just the diffstat\nof the touched lines, or the stat for the whole file?).\n\nBut \"-s\" is pretty clear: the user probably wants to see just the\ncommits that touched those lines, without any diff at all. Let's at\nleast make that work.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n line-log.c          | 6 ++++--\n t/t4211-line-log.sh | 7 +++++++\n 2 files changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/line-log.c b/line-log.c\nindex 24e21731c4..59248e37cc 100644\n--- a/line-log.c\n+++ b/line-log.c\n@@ -1103,10 +1103,12 @@ static int process_all_files(struct line_log_data **range_out,\n \n int line_log_print(struct rev_info *rev, struct commit *commit)\n {\n-\tstruct line_log_data *range = lookup_line_range(rev, commit);\n \n \tshow_log(rev);\n-\tdump_diff_hacky(rev, range);\n+\tif (!(rev->diffopt.output_format & DIFF_FORMAT_NO_OUTPUT)) {\n+\t\tstruct line_log_data *range = lookup_line_range(rev, commit);\n+\t\tdump_diff_hacky(rev, range);\n+\t}\n \treturn 1;\n }\n \ndiff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh\nindex bd5fe4d148..c9f2036f68 100755\n--- a/t/t4211-line-log.sh\n+++ b/t/t4211-line-log.sh\n@@ -115,4 +115,11 @@ test_expect_success 'range_set_union' '\n \tgit log $(for x in $(test_seq 200); do echo -L $((2*x)),+1:c.c; done)\n '\n \n+test_expect_success '-s shows only line-log commits' '\n+\tgit log --format=\"commit %s\" -L1,24:b.c >expect.raw &&\n+\tgrep ^commit expect.raw >expect &&\n+\tgit log --format=\"commit %s\" -L1,24:b.c -s >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.21.0.674.ga8a7ac9a24\n\n"}]}