[PATCH 1/2] git-svn: don't append extra newlines at the end of commit messages.

[PATCH 1/2] git-svn: don't append extra newlines at the end of commit messages.

[Avery Pennarun]

In git, all commits end in exactly one newline character.  In svn, commits
end in zero or more newlines.  Thus, when importing commits from svn into
git, git-svn always appends two extra newlines to ensure that the
git-svn-id: line is separated from the main commit message by at least one
blank line.

Combined with the terminating newline that's always present in svn commits
produced by git, you usually end up with two blank lines instead of one
between the commit message and git-svn-id: line, which is undesirable.

Instead, let's remove all trailing whitespace from the git commit on the way
through to svn.

Signed-off-by: Avery Pennarun <apenwarr@gmail.com>
---
 git-svn.perl |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 47b0c37..a54979d 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1023,6 +1023,7 @@ sub get_commit_entry {
 		my $in_msg = 0;
 		my $author;
 		my $saw_from = 0;
+		my $msgbuf = "";
 		while (<$msg_fh>) {
 			if (!$in_msg) {
 				$in_msg = 1 if (/^\s*$/);
@@ -1035,14 +1036,15 @@ sub get_commit_entry {
 				if (/^From:/ || /^Signed-off-by:/) {
 					$saw_from = 1;
 				}
-				print $log_fh $_ or croak $!;
+				$msgbuf .= $_;
 			}
 		}
+		$msgbuf =~ s/\s+$//s;
 		if ($Git::SVN::_add_author_from && defined($author)
 		    && !$saw_from) {
-			print $log_fh "\nFrom: $author\n"
-			      or croak $!;
+			$msgbuf .= "\n\nFrom: $author";
 		}
+		print $log_fh $msgbuf or croak $!;
 		command_close_pipe($msg_fh, $ctx);
 	}
 	close $log_fh or croak $!;
-- 
1.5.4.3

[PATCH 2/2] git-svn: test that extra blank lines aren't inserted in commit messages.

[Avery Pennarun]

Improve the git-svn-author test to check that extra newlines aren't inserted
into commit messages as they take a round trip from git to svn and back.

We test both with and without the --add-author-from option to git-svn.

git-svn: test that svn repo doesn't have extra newlines.

Signed-off-by: Avery Pennarun <apenwarr@gmail.com>
---
 t/t9122-git-svn-author.sh |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/t/t9122-git-svn-author.sh b/t/t9122-git-svn-author.sh
index 8c58f0b..1190576 100755
--- a/t/t9122-git-svn-author.sh
+++ b/t/t9122-git-svn-author.sh
@@ -64,7 +64,21 @@ test_expect_success 'interact with it via git-svn' '
 
 	# Make sure --add-author-from with --use-log-author affected
 	# the authorship information
-	grep "^Author: A U Thor " actual.4
+	grep "^Author: A U Thor " actual.4 &&
+
+	# Make sure there are no commit messages with excess blank lines
+	test $(grep "^ " actual.2 | wc -l) = 3 &&
+	test $(grep "^ " actual.3 | wc -l) = 5 &&
+	test $(grep "^ " actual.4 | wc -l) = 5 &&
+
+	# Make sure there are no svn commit messages with excess blank lines
+	(
+		cd work.svn &&
+		svn up &&
+		
+		test $(svn log -r2:2 | wc -l) = 5 &&
+		test $(svn log -r4:4 | wc -l) = 7
+	)
 '
 
 test_done
-- 
1.5.4.3

Re: [PATCH 1/2] git-svn: don't append extra newlines at the end of commit messages.

[Junio C Hamano]

Avery Pennarun <apenwarr@gmail.com> writes:

> In git, all commits end in exactly one newline character.  In svn, commits
> end in zero or more newlines.  Thus, when importing commits from svn into
> git, git-svn always appends two extra newlines to ensure that the
> git-svn-id: line is separated from the main commit message by at least one
> blank line.
>
> Combined with the terminating newline that's always present in svn commits
> produced by git, you usually end up with two blank lines instead of one
> between the commit message and git-svn-id: line, which is undesirable.
>
> Instead, let's remove all trailing whitespace from the git commit on the way
> through to svn.

Perl part of the code looks fine but I am unsure if we like the
ramifications of this patch on existing git-svn managed repositories.
Doesn't this change the commit object name on our end for almost all of
them?

> Signed-off-by: Avery Pennarun <apenwarr@gmail.com>
> ---
>  git-svn.perl |    8 +++++---
>  1 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/git-svn.perl b/git-svn.perl
> index 47b0c37..a54979d 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -1023,6 +1023,7 @@ sub get_commit_entry {
>  		my $in_msg = 0;
>  		my $author;
>  		my $saw_from = 0;
> +		my $msgbuf = "";
>  		while (<$msg_fh>) {
>  			if (!$in_msg) {
>  				$in_msg = 1 if (/^\s*$/);
> @@ -1035,14 +1036,15 @@ sub get_commit_entry {
>  				if (/^From:/ || /^Signed-off-by:/) {
>  					$saw_from = 1;
>  				}
> -				print $log_fh $_ or croak $!;
> +				$msgbuf .= $_;
>  			}
>  		}
> +		$msgbuf =~ s/\s+$//s;
>  		if ($Git::SVN::_add_author_from && defined($author)
>  		    && !$saw_from) {
> -			print $log_fh "\nFrom: $author\n"
> -			      or croak $!;
> +			$msgbuf .= "\n\nFrom: $author";
>  		}
> +		print $log_fh $msgbuf or croak $!;
>  		command_close_pipe($msg_fh, $ctx);
>  	}
>  	close $log_fh or croak $!;
> -- 
> 1.5.4.3

Re: [PATCH 1/2] git-svn: don't append extra newlines at the end of commit messages.

[Karl Hasselström]

On 2008-06-12 22:41:46 -0700, Junio C Hamano wrote:

> Avery Pennarun <apenwarr@gmail.com> writes:
>
> > In git, all commits end in exactly one newline character. In svn,
> > commits end in zero or more newlines. Thus, when importing commits
> > from svn into git, git-svn always appends two extra newlines to
> > ensure that the git-svn-id: line is separated from the main commit
> > message by at least one blank line.
> >
> > Combined with the terminating newline that's always present in svn
> > commits produced by git, you usually end up with two blank lines
> > instead of one between the commit message and git-svn-id: line,
> > which is undesirable.
> >
> > Instead, let's remove all trailing whitespace from the git commit
> > on the way through to svn.
>
> Perl part of the code looks fine but I am unsure if we like the
> ramifications of this patch on existing git-svn managed
> repositories. Doesn't this change the commit object name on our end
> for almost all of them?

You're correct that this will change the commit id of imported svn
revisions -- the new verson of git-svn will give other ids than the
old version. However, the only thing that's going to "break" is that
two separate imports of the same repository with the two different
versions of git-svn will not give the exact same result on the git
side.

I think this is a good change, and it would be a pity if this and
other changes like it had to be dropped out of a desire to keep the
svn -> git transformation function forever fixed. (IIRC, such changes
have been allowed in the past -- e.g. 0bed5eaa: "enable follow-parent
functionality by default".)

One could imagine a policy of not introducing this kind of change
during a stable version, or some such, but historically that's not
been the case as far as I can remember.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

Re: [PATCH 1/2] git-svn: don't append extra newlines at the end of commit messages.

[Avery Pennarun]

On 6/13/08, Junio C Hamano <gitster@pobox.com> wrote:
> Avery Pennarun <apenwarr@gmail.com> writes:
>  > Instead, let's remove all trailing whitespace from the git commit on the way
>  > through to svn.
>
> Perl part of the code looks fine but I am unsure if we like the
>  ramifications of this patch on existing git-svn managed repositories.
>  Doesn't this change the commit object name on our end for almost all of
>  them?

Unless I got confused while coding this (I don't think I did), this
should *not* affect existing or re-imported svn or git-svn
repositories.  It only removes trailing whitespace the first time a
git commit is sent into svn, which should happen only once for a brand
new commit by someone who has made it in git and is now dcommiting it
to svn.

Naturally, the dcommit round-trip *always* produces a new sha1 hash
for that commit anyhow because of the added git-svn-id: line.  After
this change, the sha1 will be different than it would have been
before, but it will still be the same for anyone who checks out from
svn again with git-svn.

Thanks,

Avery

Re: [PATCH 1/2] git-svn: don't append extra newlines at the end of commit messages.

[Andreas Ericsson]

Avery Pennarun wrote:
> In git, all commits end in exactly one newline character.  In svn, commits
> end in zero or more newlines.  Thus, when importing commits from svn into
> git, git-svn always appends two extra newlines to ensure that the
> git-svn-id: line is separated from the main commit message by at least one
> blank line.
> 
> Combined with the terminating newline that's always present in svn commits
> produced by git, you usually end up with two blank lines instead of one
> between the commit message and git-svn-id: line, which is undesirable.
> 
> Instead, let's remove all trailing whitespace from the git commit on the way
> through to svn.
> 

I'm not familiar with git-svn, and my perl is pretty weak as it is.
Are you proposing to remove extra whitespace from git commits when they are
sent back to svn via dcommit? If so, wouldn't it be better to always strip
extra newlines when importing from svn so they're never there in the first
place?

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [PATCH 1/2] git-svn: don't append extra newlines at the end of commit messages.

[Karl Hasselström]

On 2008-06-13 09:23:01 +0200, Andreas Ericsson wrote:

> Are you proposing to remove extra whitespace from git commits when
> they are sent back to svn via dcommit? If so, wouldn't it be better
> to always strip extra newlines when importing from svn so they're
> never there in the first place?

That's what the patch does, unless I'm misreading it.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

Re: [PATCH 1/2] git-svn: don't append extra newlines at the end of commit messages.

[Karl Hasselström]

On 2008-06-13 10:09:49 +0200, Karl Hasselström wrote:

> On 2008-06-13 09:23:01 +0200, Andreas Ericsson wrote:
>
> > Are you proposing to remove extra whitespace from git commits when
> > they are sent back to svn via dcommit? If so, wouldn't it be
> > better to always strip extra newlines when importing from svn so
> > they're never there in the first place?
>
> That's what the patch does, unless I'm misreading it.

As has been realized elsewhere in this thread, I was misreading it.
:-P

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

Re: [PATCH 1/2] git-svn: don't append extra newlines at the end of commit messages.

[Avery Pennarun]

On 6/13/08, Andreas Ericsson <ae@op5.se> wrote:
>  I'm not familiar with git-svn, and my perl is pretty weak as it is.
>  Are you proposing to remove extra whitespace from git commits when they are
>  sent back to svn via dcommit? If so, wouldn't it be better to always strip
>  extra newlines when importing from svn so they're never there in the first
>  place?

I thought of doing it that way (in fact, I *did* do it that way the
first time).  But I figured it would be too bad if you couldn't always
reconstruct the precise svn commit message from the git commit
message.  Currently you can, even though you sometimes have to remove
multiple newlines.  Interestingly, the addition of the git-svn-id:
line seems to be the only reason the commit messages aren't corrupted,
since git normally trims whitespace from the end of its own commits.

So for the record, whether the sha1's would be affected after my patch
didn't even occur to me.  But it's very convenient that they're not.

Avery