{"thread":{"id":"13922","subject":"[PATCH 2/2] git-svn: test that extra blank lines aren't inserted in commit messages.","startedAt":"2008-06-12T23:10:50Z","lastAt":"2008-06-14T08:43:56Z","messageCount":9,"participants":["Avery Pennarun","Junio C Hamano","Karl Hasselström","Andreas Ericsson"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"79653","messageId":"1213312251-8081-1-git-send-email-apenwarr@gmail.com","threadId":"13922","inReplyTo":null,"subject":"[PATCH 1/2] git-svn: don't append extra newlines at the end of commit messages.","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2008-06-12T23:10:50Z","receivedAt":"2008-06-12T23:10:50Z","isPatch":true,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"In git, all commits end in exactly one newline character.  In svn, commits\nend in zero or more newlines.  Thus, when importing commits from svn into\ngit, git-svn always appends two extra newlines to ensure that the\ngit-svn-id: line is separated from the main commit message by at least one\nblank line.\n\nCombined with the terminating newline that's always present in svn commits\nproduced by git, you usually end up with two blank lines instead of one\nbetween the commit message and git-svn-id: line, which is undesirable.\n\nInstead, let's remove all trailing whitespace from the git commit on the way\nthrough to svn.\n\nSigned-off-by: Avery Pennarun <apenwarr@gmail.com>\n---\n git-svn.perl |    8 +++++---\n 1 files changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 47b0c37..a54979d 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -1023,6 +1023,7 @@ sub get_commit_entry {\n \t\tmy $in_msg = 0;\n \t\tmy $author;\n \t\tmy $saw_from = 0;\n+\t\tmy $msgbuf = \"\";\n \t\twhile (<$msg_fh>) {\n \t\t\tif (!$in_msg) {\n \t\t\t\t$in_msg = 1 if (/^\\s*$/);\n@@ -1035,14 +1036,15 @@ sub get_commit_entry {\n \t\t\t\tif (/^From:/ || /^Signed-off-by:/) {\n \t\t\t\t\t$saw_from = 1;\n \t\t\t\t}\n-\t\t\t\tprint $log_fh $_ or croak $!;\n+\t\t\t\t$msgbuf .= $_;\n \t\t\t}\n \t\t}\n+\t\t$msgbuf =~ s/\\s+$//s;\n \t\tif ($Git::SVN::_add_author_from && defined($author)\n \t\t    && !$saw_from) {\n-\t\t\tprint $log_fh \"\\nFrom: $author\\n\"\n-\t\t\t      or croak $!;\n+\t\t\t$msgbuf .= \"\\n\\nFrom: $author\";\n \t\t}\n+\t\tprint $log_fh $msgbuf or croak $!;\n \t\tcommand_close_pipe($msg_fh, $ctx);\n \t}\n \tclose $log_fh or croak $!;\n-- \n1.5.4.3\n"},{"id":"79652","messageId":"1213312251-8081-2-git-send-email-apenwarr@gmail.com","threadId":"13922","inReplyTo":"1213312251-8081-1-git-send-email-apenwarr@gmail.com","subject":"[PATCH 2/2] git-svn: test that extra blank lines aren't inserted in commit messages.","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2008-06-12T23:10:51Z","receivedAt":"2008-06-12T23:10:51Z","isPatch":true,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"Improve the git-svn-author test to check that extra newlines aren't inserted\ninto commit messages as they take a round trip from git to svn and back.\n\nWe test both with and without the --add-author-from option to git-svn.\n\ngit-svn: test that svn repo doesn't have extra newlines.\n\nSigned-off-by: Avery Pennarun <apenwarr@gmail.com>\n---\n t/t9122-git-svn-author.sh |   16 +++++++++++++++-\n 1 files changed, 15 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t9122-git-svn-author.sh b/t/t9122-git-svn-author.sh\nindex 8c58f0b..1190576 100755\n--- a/t/t9122-git-svn-author.sh\n+++ b/t/t9122-git-svn-author.sh\n@@ -64,7 +64,21 @@ test_expect_success 'interact with it via git-svn' '\n \n \t# Make sure --add-author-from with --use-log-author affected\n \t# the authorship information\n-\tgrep \"^Author: A U Thor \" actual.4\n+\tgrep \"^Author: A U Thor \" actual.4 &&\n+\n+\t# Make sure there are no commit messages with excess blank lines\n+\ttest $(grep \"^ \" actual.2 | wc -l) = 3 &&\n+\ttest $(grep \"^ \" actual.3 | wc -l) = 5 &&\n+\ttest $(grep \"^ \" actual.4 | wc -l) = 5 &&\n+\n+\t# Make sure there are no svn commit messages with excess blank lines\n+\t(\n+\t\tcd work.svn &&\n+\t\tsvn up &&\n+\t\t\n+\t\ttest $(svn log -r2:2 | wc -l) = 5 &&\n+\t\ttest $(svn log -r4:4 | wc -l) = 7\n+\t)\n '\n \n test_done\n-- \n1.5.4.3\n"},{"id":"79675","messageId":"7vfxrhyjqd.fsf@gitster.siamese.dyndns.org","threadId":"13922","inReplyTo":"1213312251-8081-1-git-send-email-apenwarr@gmail.com","subject":"Re: [PATCH 1/2] git-svn: don't append extra newlines at the end of commit messages.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-13T05:41:46Z","receivedAt":"2008-06-13T05:41:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Avery Pennarun <apenwarr@gmail.com> writes:\n\n> In git, all commits end in exactly one newline character.  In svn, commits\n> end in zero or more newlines.  Thus, when importing commits from svn into\n> git, git-svn always appends two extra newlines to ensure that the\n> git-svn-id: line is separated from the main commit message by at least one\n> blank line.\n>\n> Combined with the terminating newline that's always present in svn commits\n> produced by git, you usually end up with two blank lines instead of one\n> between the commit message and git-svn-id: line, which is undesirable.\n>\n> Instead, let's remove all trailing whitespace from the git commit on the way\n> through to svn.\n\nPerl part of the code looks fine but I am unsure if we like the\nramifications of this patch on existing git-svn managed repositories.\nDoesn't this change the commit object name on our end for almost all of\nthem?\n\n> Signed-off-by: Avery Pennarun <apenwarr@gmail.com>\n> ---\n>  git-svn.perl |    8 +++++---\n>  1 files changed, 5 insertions(+), 3 deletions(-)\n>\n> diff --git a/git-svn.perl b/git-svn.perl\n> index 47b0c37..a54979d 100755\n> --- a/git-svn.perl\n> +++ b/git-svn.perl\n> @@ -1023,6 +1023,7 @@ sub get_commit_entry {\n>  \t\tmy $in_msg = 0;\n>  \t\tmy $author;\n>  \t\tmy $saw_from = 0;\n> +\t\tmy $msgbuf = \"\";\n>  \t\twhile (<$msg_fh>) {\n>  \t\t\tif (!$in_msg) {\n>  \t\t\t\t$in_msg = 1 if (/^\\s*$/);\n> @@ -1035,14 +1036,15 @@ sub get_commit_entry {\n>  \t\t\t\tif (/^From:/ || /^Signed-off-by:/) {\n>  \t\t\t\t\t$saw_from = 1;\n>  \t\t\t\t}\n> -\t\t\t\tprint $log_fh $_ or croak $!;\n> +\t\t\t\t$msgbuf .= $_;\n>  \t\t\t}\n>  \t\t}\n> +\t\t$msgbuf =~ s/\\s+$//s;\n>  \t\tif ($Git::SVN::_add_author_from && defined($author)\n>  \t\t    && !$saw_from) {\n> -\t\t\tprint $log_fh \"\\nFrom: $author\\n\"\n> -\t\t\t      or croak $!;\n> +\t\t\t$msgbuf .= \"\\n\\nFrom: $author\";\n>  \t\t}\n> +\t\tprint $log_fh $msgbuf or croak $!;\n>  \t\tcommand_close_pipe($msg_fh, $ctx);\n>  \t}\n>  \tclose $log_fh or croak $!;\n> -- \n> 1.5.4.3\n"},{"id":"79682","messageId":"20080613062951.GB24245@diana.vm.bytemark.co.uk","threadId":"13922","inReplyTo":"7vfxrhyjqd.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/2] git-svn: don't append extra newlines at the end of commit messages.","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-06-13T06:29:51Z","receivedAt":"2008-06-13T06:29:51Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2008-06-12 22:41:46 -0700, Junio C Hamano wrote:\n\n> Avery Pennarun <apenwarr@gmail.com> writes:\n>\n> > In git, all commits end in exactly one newline character. In svn,\n> > commits end in zero or more newlines. Thus, when importing commits\n> > from svn into git, git-svn always appends two extra newlines to\n> > ensure that the git-svn-id: line is separated from the main commit\n> > message by at least one blank line.\n> >\n> > Combined with the terminating newline that's always present in svn\n> > commits produced by git, you usually end up with two blank lines\n> > instead of one between the commit message and git-svn-id: line,\n> > which is undesirable.\n> >\n> > Instead, let's remove all trailing whitespace from the git commit\n> > on the way through to svn.\n>\n> Perl part of the code looks fine but I am unsure if we like the\n> ramifications of this patch on existing git-svn managed\n> repositories. Doesn't this change the commit object name on our end\n> for almost all of them?\n\nYou're correct that this will change the commit id of imported svn\nrevisions -- the new verson of git-svn will give other ids than the\nold version. However, the only thing that's going to \"break\" is that\ntwo separate imports of the same repository with the two different\nversions of git-svn will not give the exact same result on the git\nside.\n\nI think this is a good change, and it would be a pity if this and\nother changes like it had to be dropped out of a desire to keep the\nsvn -> git transformation function forever fixed. (IIRC, such changes\nhave been allowed in the past -- e.g. 0bed5eaa: \"enable follow-parent\nfunctionality by default\".)\n\nOne could imagine a policy of not introducing this kind of change\nduring a stable version, or some such, but historically that's not\nbeen the case as far as I can remember.\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"79685","messageId":"48522055.6060006@op5.se","threadId":"13922","inReplyTo":"1213312251-8081-1-git-send-email-apenwarr@gmail.com","subject":"Re: [PATCH 1/2] git-svn: don't append extra newlines at the end of commit messages.","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2008-06-13T07:23:01Z","receivedAt":"2008-06-13T07:23:01Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Avery Pennarun wrote:\n> In git, all commits end in exactly one newline character.  In svn, commits\n> end in zero or more newlines.  Thus, when importing commits from svn into\n> git, git-svn always appends two extra newlines to ensure that the\n> git-svn-id: line is separated from the main commit message by at least one\n> blank line.\n> \n> Combined with the terminating newline that's always present in svn commits\n> produced by git, you usually end up with two blank lines instead of one\n> between the commit message and git-svn-id: line, which is undesirable.\n> \n> Instead, let's remove all trailing whitespace from the git commit on the way\n> through to svn.\n> \n\nI'm not familiar with git-svn, and my perl is pretty weak as it is.\nAre you proposing to remove extra whitespace from git commits when they are\nsent back to svn via dcommit? If so, wouldn't it be better to always strip\nextra newlines when importing from svn so they're never there in the first\nplace?\n\n-- \nAndreas Ericsson                   andreas.ericsson@op5.se\nOP5 AB                             www.op5.se\nTel: +46 8-230225                  Fax: +46 8-230231\n"},{"id":"79687","messageId":"20080613080949.GA26817@diana.vm.bytemark.co.uk","threadId":"13922","inReplyTo":"48522055.6060006@op5.se","subject":"Re: [PATCH 1/2] git-svn: don't append extra newlines at the end of commit messages.","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-06-13T08:09:49Z","receivedAt":"2008-06-13T08:09:49Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2008-06-13 09:23:01 +0200, Andreas Ericsson wrote:\n\n> Are you proposing to remove extra whitespace from git commits when\n> they are sent back to svn via dcommit? If so, wouldn't it be better\n> to always strip extra newlines when importing from svn so they're\n> never there in the first place?\n\nThat's what the patch does, unless I'm misreading it.\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"79737","messageId":"32541b130806130917y23a55751tfccac0de8143ebe4@mail.gmail.com","threadId":"13922","inReplyTo":"7vfxrhyjqd.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/2] git-svn: don't append extra newlines at the end of commit messages.","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2008-06-13T16:17:44Z","receivedAt":"2008-06-13T16:17:44Z","isPatch":true,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"On 6/13/08, Junio C Hamano <gitster@pobox.com> wrote:\n> Avery Pennarun <apenwarr@gmail.com> writes:\n>  > Instead, let's remove all trailing whitespace from the git commit on the way\n>  > through to svn.\n>\n> Perl part of the code looks fine but I am unsure if we like the\n>  ramifications of this patch on existing git-svn managed repositories.\n>  Doesn't this change the commit object name on our end for almost all of\n>  them?\n\nUnless I got confused while coding this (I don't think I did), this\nshould *not* affect existing or re-imported svn or git-svn\nrepositories.  It only removes trailing whitespace the first time a\ngit commit is sent into svn, which should happen only once for a brand\nnew commit by someone who has made it in git and is now dcommiting it\nto svn.\n\nNaturally, the dcommit round-trip *always* produces a new sha1 hash\nfor that commit anyhow because of the added git-svn-id: line.  After\nthis change, the sha1 will be different than it would have been\nbefore, but it will still be the same for anyone who checks out from\nsvn again with git-svn.\n\nThanks,\n\nAvery\n"},{"id":"79738","messageId":"32541b130806130923x4c66f6ddybaaa5d7b00f6dead@mail.gmail.com","threadId":"13922","inReplyTo":"48522055.6060006@op5.se","subject":"Re: [PATCH 1/2] git-svn: don't append extra newlines at the end of commit messages.","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2008-06-13T16:23:46Z","receivedAt":"2008-06-13T16:23:46Z","isPatch":true,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"On 6/13/08, Andreas Ericsson <ae@op5.se> wrote:\n>  I'm not familiar with git-svn, and my perl is pretty weak as it is.\n>  Are you proposing to remove extra whitespace from git commits when they are\n>  sent back to svn via dcommit? If so, wouldn't it be better to always strip\n>  extra newlines when importing from svn so they're never there in the first\n>  place?\n\nI thought of doing it that way (in fact, I *did* do it that way the\nfirst time).  But I figured it would be too bad if you couldn't always\nreconstruct the precise svn commit message from the git commit\nmessage.  Currently you can, even though you sometimes have to remove\nmultiple newlines.  Interestingly, the addition of the git-svn-id:\nline seems to be the only reason the commit messages aren't corrupted,\nsince git normally trims whitespace from the end of its own commits.\n\nSo for the record, whether the sha1's would be affected after my patch\ndidn't even occur to me.  But it's very convenient that they're not.\n\nAvery\n"},{"id":"79831","messageId":"20080614084356.GA14282@diana.vm.bytemark.co.uk","threadId":"13922","inReplyTo":"20080613080949.GA26817@diana.vm.bytemark.co.uk","subject":"Re: [PATCH 1/2] git-svn: don't append extra newlines at the end of commit messages.","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-06-14T08:43:56Z","receivedAt":"2008-06-14T08:43:56Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2008-06-13 10:09:49 +0200, Karl Hasselström wrote:\n\n> On 2008-06-13 09:23:01 +0200, Andreas Ericsson wrote:\n>\n> > Are you proposing to remove extra whitespace from git commits when\n> > they are sent back to svn via dcommit? If so, wouldn't it be\n> > better to always strip extra newlines when importing from svn so\n> > they're never there in the first place?\n>\n> That's what the patch does, unless I'm misreading it.\n\nAs has been realized elsewhere in this thread, I was misreading it.\n:-P\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"}]}