{"thread":{"id":"10193","subject":"Fix embarrassing \"git log --follow\" bug","startedAt":"2007-10-08T20:42:41Z","lastAt":"2007-10-09T16:35:22Z","messageCount":4,"participants":["Linus Torvalds"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"55205","messageId":"alpine.LFD.0.999.0710081337490.4964@woody.linux-foundation.org","threadId":"10193","inReplyTo":null,"subject":"Fix embarrassing \"git log --follow\" bug","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-10-08T20:42:41Z","receivedAt":"2007-10-08T20:42:41Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nIt turns out that I completely broke \"git log --follow\" with my recent \npatch to revision.c (\"Fix revision log diff setup, avoid unnecessary diff \ngeneration\", commit b7bb760d5ed4881422673d32f869d140221d3564).\n\nWhy? Because --follow obviously requires the diff machinery to function, \nexactly the same way pickaxe does.\n\nSo everybody is away right now, but considering that nobody even noticed \nthis bug, I don't think it matters. But for the record, here's the trivial \none-liner fix (well, two, since I also fixed the comment).\n\nBecause of the nature of the bug, if you ask for patches when following \n(which is one of the things I normally do), the bug is hidden, because \nthen the request for diff output will automatically also enable the diffs \nthemselves.\n\nSo while \"git log --follow <filename>\" didn't work, adding a \"-p\" \nmagically made it work again even without this fix.\n\nSigned-off-by: Linus Torvalds <torvalds@linux-foundation.org>\n---\n revision.c |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 5d294be..e76da0d 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1241,8 +1241,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch\n \tif (revs->diffopt.output_format & ~DIFF_FORMAT_NO_OUTPUT)\n \t\trevs->diff = 1;\n \n-\t/* Pickaxe needs diffs */\n-\tif (revs->diffopt.pickaxe)\n+\t/* Pickaxe and rename following needs diffs */\n+\tif (revs->diffopt.pickaxe || revs->diffopt.follow_renames)\n \t\trevs->diff = 1;\n \n \tif (revs->topo_order)\n"},{"id":"55206","messageId":"alpine.LFD.0.999.0710081342530.4964@woody.linux-foundation.org","threadId":"10193","inReplyTo":"alpine.LFD.0.999.0710081337490.4964@woody.linux-foundation.org","subject":"Clean up \"git log\" format with DIFF_FORMAT_NO_OUTPUT","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-10-08T20:46:52Z","receivedAt":"2007-10-08T20:46:52Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nThis fixes an unnecessary empty line that we add to the log message when \nwe generate diffs, but don't actually end up printing any due to having \nDIFF_FORMAT_NO_OUTPUT set.\n\nThis can happen with pickaxe or with rename following. The reason is that \nwe normally add an empty line between the commit and the diff, but we do \nthat even for the case where we've then suppressed the actual printing of \nthe diff.\n\nSigned-off-by: Linus Torvalds <torvalds@linux-foundation.org>\n---\n log-tree.c |    3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex 2319154..62edd34 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -304,7 +304,8 @@ int log_tree_diff_flush(struct rev_info *opt)\n \t\t * output for readability.\n \t\t */\n \t\tshow_log(opt, opt->diffopt.msg_sep);\n-\t\tif (opt->verbose_header &&\n+\t\tif ((opt->diffopt.output_format & ~DIFF_FORMAT_NO_OUTPUT) &&\n+\t\t    opt->verbose_header &&\n \t\t    opt->commit_format != CMIT_FMT_ONELINE) {\n \t\t\tint pch = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_PATCH;\n \t\t\tif ((pch & opt->diffopt.output_format) == pch)\n"},{"id":"55207","messageId":"alpine.LFD.0.999.0710081353290.4964@woody.linux-foundation.org","threadId":"10193","inReplyTo":"alpine.LFD.0.999.0710081342530.4964@woody.linux-foundation.org","subject":"Re: Clean up \"git log\" format with DIFF_FORMAT_NO_OUTPUT","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-10-08T20:55:07Z","receivedAt":"2007-10-08T20:55:07Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 8 Oct 2007, Linus Torvalds wrote:\n> \n> This fixes an unnecessary empty line that we add to the log message when \n> we generate diffs, but don't actually end up printing any due to having \n> DIFF_FORMAT_NO_OUTPUT set.\n\nBtw, this seems to break a few tests, where the test assumed the extra \nnewlines at the end of commands like \"git show -s\" (which also triggered \nthe extra unnecessary padding between commit text and the suppressed \ndiff).\n\nI think the patch is correct, but the tests obviously would need fixing \nbefore this patch should be applied.\n\n\t\tLinus\n"},{"id":"55291","messageId":"alpine.LFD.0.999.0710090932560.5039@woody.linux-foundation.org","threadId":"10193","inReplyTo":"alpine.LFD.0.999.0710081353290.4964@woody.linux-foundation.org","subject":"[take 2] Clean up \"git log\" format with DIFF_FORMAT_NO_OUTPUT","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-10-09T16:35:22Z","receivedAt":"2007-10-09T16:35:22Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nClean up \"git log\" format with DIFF_FORMAT_NO_OUTPUT\n\nThis fixes an unnecessary empty line that we add to the log message when\nwe generate diffs, but don't actually end up printing any due to having\nDIFF_FORMAT_NO_OUTPUT set.\n\nThis can happen with pickaxe or with rename following. The reason is that\nwe normally add an empty line between the commit and the diff, but we do\nthat even for the case where we've then suppressed the actual printing of\nthe diff.\n\nThis also updates a couple of tests that assumed the extraneous empty\nline would exist at the end of output.\n\nSigned-off-by: Linus Torvalds <torvalds@linux-foundation.org>\n---\n\nThis is a resend of the original patch, with just the changes to the tests \nadded to make it pass them all. Both of the tests actually got cleaner, I \nthink (ie one had the sed script blindly just delete the last line in \norder to match it up with the known input, for example).\n\n\t\t\tLinus\n\n\n log-tree.c                  |    3 ++-\n t/t3900-i18n-commit.sh      |    2 +-\n t/t4013/diff.log_-SF_master |    1 -\n 3 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex 2319154..62edd34 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -304,7 +304,8 @@ int log_tree_diff_flush(struct rev_info *opt)\n \t\t * output for readability.\n \t\t */\n \t\tshow_log(opt, opt->diffopt.msg_sep);\n-\t\tif (opt->verbose_header &&\n+\t\tif ((opt->diffopt.output_format & ~DIFF_FORMAT_NO_OUTPUT) &&\n+\t\t    opt->verbose_header &&\n \t\t    opt->commit_format != CMIT_FMT_ONELINE) {\n \t\t\tint pch = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_PATCH;\n \t\t\tif ((pch & opt->diffopt.output_format) == pch)\ndiff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh\nindex fcbabe8..94b1c24 100755\n--- a/t/t3900-i18n-commit.sh\n+++ b/t/t3900-i18n-commit.sh\n@@ -8,7 +8,7 @@ test_description='commit and log output encodings'\n . ./test-lib.sh\n \n compare_with () {\n-\tgit show -s $1 | sed -e '1,/^$/d' -e 's/^    //' -e '$d' >current &&\n+\tgit show -s $1 | sed -e '1,/^$/d' -e 's/^    //' >current &&\n \tgit diff current \"$2\"\n }\n \ndiff --git a/t/t4013/diff.log_-SF_master b/t/t4013/diff.log_-SF_master\nindex 6162ed2..c1599f2 100644\n--- a/t/t4013/diff.log_-SF_master\n+++ b/t/t4013/diff.log_-SF_master\n@@ -4,5 +4,4 @@ Author: A U Thor <author@example.com>\n Date:   Mon Jun 26 00:02:00 2006 +0000\n \n     Third\n-\n $\n"}]}