[PATCH 0/5] git-svn: svk log message cleanup

[PATCH 0/5] git-svn: svk log message cleanup

[Alex Vandiver]

This patch series tries to clean up the cruft that svk leaves in log
messages, while optionally (using the existing --use-log-author
option) using the information therein to set author username and time.
 - Alex

[PATCH 1/5] git-svn: Allow setting the committer and author date separately

[Alex Vandiver]

Signed-off-by: Alex Vandiver <alex@chmrr.net>
---
 git-svn.perl |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 51f03ad..53bf20c 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2491,7 +2491,7 @@ sub set_commit_header_env {
 
 	$ENV{GIT_AUTHOR_NAME} = $log_entry->{name};
 	$ENV{GIT_AUTHOR_EMAIL} = $log_entry->{email};
-	$ENV{GIT_AUTHOR_DATE} = $ENV{GIT_COMMITTER_DATE} = $log_entry->{date};
+	$ENV{GIT_AUTHOR_DATE} = $log_entry->{date};
 
 	$ENV{GIT_COMMITTER_NAME} = (defined $log_entry->{commit_name})
 						? $log_entry->{commit_name}
@@ -2499,6 +2499,9 @@ sub set_commit_header_env {
 	$ENV{GIT_COMMITTER_EMAIL} = (defined $log_entry->{commit_email})
 						? $log_entry->{commit_email}
 						: $log_entry->{email};
+        $ENV{GIT_COMMITTER_DATE} =  (defined $log_entry->{commit_date})
+						? $log_entry->{commit_date}
+						: $log_entry->{date};
 	\%env;
 }
 
-- 
1.6.6.rc0.327.g032bc

[PATCH 2/5] git-svn: Make merge metadata accessible to make_log_entry

[Alex Vandiver]

Signed-off-by: Alex Vandiver <alex@chmrr.net>
---
 git-svn.perl |   22 ++++++++++++++--------
 1 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 53bf20c..5337326 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2924,7 +2924,7 @@ sub check_author {
 }
 
 sub find_extra_svk_parents {
-	my ($self, $ed, $tickets, $parents) = @_;
+	my ($self, $ed, $tickets, $parents, $merges) = @_;
 	# aha!  svk:merge property changed...
 	my @tickets = split "\n", $tickets;
 	my @known_parents;
@@ -2944,14 +2944,15 @@ sub find_extra_svk_parents {
 				# wahey!  we found it, but it might be
 				# an old one (!)
 				push @known_parents, [ $rev, $commit ];
+				push @known_parents, [ $rev, $path, $commit ];
 			}
 		}
 	}
 	# Ordering matters; highest-numbered commit merge tickets
 	# first, as they may account for later merge ticket additions
 	# or changes.
-	@known_parents = map {$_->[1]} sort {$b->[0] <=> $a->[0]} @known_parents;
-	for my $parent ( @known_parents ) {
+	for my $merge ( sort {$b->[0] <=> $a->[0]} @known_parents ) {
+		my ($rev, $path, $parent) = @{$merge};
 		my @cmd = ('rev-list', $parent, map { "^$_" } @$parents );
 		my ($msg_fh, $ctx) = command_output_pipe(@cmd);
 		my $new;
@@ -2963,6 +2964,7 @@ sub find_extra_svk_parents {
 			print STDERR
 			    "Found merge parent (svk:merge ticket): $parent\n";
 			push @$parents, $parent;
+			push @$merges, "$path:$rev";
 		}
 	}
 }
@@ -3061,27 +3063,31 @@ sub make_log_entry {
 	my ($self, $rev, $parents, $ed) = @_;
 	my $untracked = $self->get_untracked($ed);
 
-	my @parents = @$parents;
+	my %log_entry = ( parents => $parents,
+	                  merged_branches => [],
+	                  revision => $rev,
+	                  log => '');
 	my $ps = $ed->{path_strip} || "";
 	for my $path ( grep { m/$ps/ } %{$ed->{dir_prop}} ) {
 		my $props = $ed->{dir_prop}{$path};
 		if ( $props->{"svk:merge"} ) {
 			$self->find_extra_svk_parents
-				($ed, $props->{"svk:merge"}, \@parents);
+				($ed,
+				 $props->{"svk:merge"},
+				 $log_entry{parents},
+				 $log_entry{merged_branches});
 		}
 		if ( $props->{"svn:mergeinfo"} ) {
 			$self->find_extra_svn_parents
 				($ed,
 				 $props->{"svn:mergeinfo"},
-				 \@parents);
+				 $log_entry{parents});
 		}
 	}
 
 	open my $un, '>>', "$self->{dir}/unhandled.log" or croak $!;
 	print $un "r$rev\n" or croak $!;
 	print $un $_, "\n" foreach @$untracked;
-	my %log_entry = ( parents => \@parents, revision => $rev,
-	                  log => '');
 
 	my $headrev;
 	my $logged = delete $self->{logged_rev_props};
-- 
1.6.6.rc0.327.g032bc

Re: [PATCH 2/5] git-svn: Make merge metadata accessible to make_log_entry

[Alex Vandiver]

At Wed Dec 02 14:07:51 -0500 2009, Alex Vandiver wrote:
> @@ -2944,14 +2944,15 @@ sub find_extra_svk_parents {
>                  # wahey!  we found it, but it might be
>                  # an old one (!)
>                  push @known_parents, [ $rev, $commit ];
> +                push @known_parents, [ $rev, $path, $commit ];
>              }
>          }
>      }

This hunk is wrong due to a mis-merge on my part -- the first 'push'
line should have been removed, obviously.  I'll wait for other
comments before I push a v2, however.

 - Alex
-- 
Networking -- only one letter away from not working

Re: [PATCH 2/5] git-svn: Make merge metadata accessible to make_log_entry

[Eric Wong]

Alex Vandiver <alex@chmrr.net> wrote:
> This hunk is wrong due to a mis-merge on my part -- the first 'push'
> line should have been removed, obviously.  I'll wait for other
> comments before I push a v2, however.

Hi Alex,

I'll continue to defer to Sam for ack-ing SVK-related things.

One thing I am very picky about is wrapping lines at 80-columns or
less (hard tabs being 8 characters wide).

-- 
Eric Wong

Re: [PATCH 2/5] git-svn: Make merge metadata accessible to make_log_entry

[Alex Vandiver]

At Sat Dec 05 17:32:41 -0500 2009, Eric Wong wrote:
> I'll continue to defer to Sam for ack-ing SVK-related things.
> 
> One thing I am very picky about is wrapping lines at 80-columns or
> less (hard tabs being 8 characters wide).

*nod* I've fixed up my local copy for v2 to rewrap things at < 80
columns, and will keep that in mind for the future.
 - Alex
-- 
Networking -- only one letter away from not working

Re: [spf:guess] Re: [PATCH 2/5] git-svn: Make merge metadata accessible to make_log_entry

[Sam Vilain]

On Sat, 2009-12-05 at 17:51 -0500, Alex Vandiver wrote:
> At Sat Dec 05 17:32:41 -0500 2009, Eric Wong wrote:
> > I'll continue to defer to Sam for ack-ing SVK-related things.
> > 
> > One thing I am very picky about is wrapping lines at 80-columns or
> > less (hard tabs being 8 characters wide).
> 
> *nod* I've fixed up my local copy for v2 to rewrap things at < 80
> columns, and will keep that in mind for the future.

Hi, I've just seen the series, looks like a good idea.  Just a couple of
questions then I'll review the code;

 - when a change is merged upstream with svk, you will get multiple log
entries in a single commit message.  What do you do with commits like
that?

 - there are quite a few repos which will have empty commits (ie, no
changes), but with one or more of the above log entries, owing to a bug
which I can't seem to find the details of right now..  how might those
appear?

Thanks for your submission!
Sam

Re: [spf:guess] Re: [PATCH 2/5] git-svn: Make merge metadata accessible to make_log_entry

[Alex Vandiver]

At Sat Dec 05 17:59:32 -0500 2009, Sam Vilain wrote:
> Hi, I've just seen the series, looks like a good idea.  Just a couple of
> questions then I'll review the code;
> 
>  - when a change is merged upstream with svk, you will get multiple log
> entries in a single commit message.  What do you do with commits like
> that?

If the user edited the merge message and supplied their own message,
nothing.  If the first line is an SVK merge line, it inserts a message
"Merged from /svn/path/to/trunk:12345\n\n" at the top.

>  - there are quite a few repos which will have empty commits (ie, no
> changes), but with one or more of the above log entries, owing to a bug
> which I can't seem to find the details of right now..  how might those
> appear?

Without seeing an example, I'm not sure offhand -- but what you're
describing does ring a bell, so I'm sure I've seen the mismerges
you're talking about as well.
 - Alex
-- 
Networking -- only one letter away from not working

Re: [spf:guess] Re: [PATCH 2/5] git-svn: Make merge metadata accessible to make_log_entry

[Alex Vandiver]

At Sat Dec 05 17:59:32 -0500 2009, Sam Vilain wrote:
> Hi, I've just seen the series, looks like a good idea.  Just a couple of
> questions then I'll review the code;

Have you had time to take a look at these?
 - Alex
-- 
Networking -- only one letter away from not working

[PATCH 3/5] git-svn: Strip SVK headers, optionally parsing author information

[Alex Vandiver]

SVK adds additional headers (often nested arbitrarily) detailing
information on the local commit.  When possible, strip these headers
so that the first line of git's commit message is actually descriptive
of the commit.

Additionally, these headers contain information about the original
author's username, and their local commit time.  If the
--use-log-author flag is set, use this information to set the
information on the git commit.  Note that the username thus extracted
may be a _local_ username, and thus may require additional, somewhat
unexpected, entries in the authors file.

Signed-off-by: Alex Vandiver <alex@chmrr.net>
---
 git-svn.perl |   98 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 96 insertions(+), 2 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 5337326..0731425 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -3110,7 +3110,8 @@ sub make_log_entry {
 	close $un or croak $!;
 
 	$log_entry{date} = parse_svn_date($log_entry{date});
-	$log_entry{log} .= "\n";
+	parse_svk_log(\%log_entry) if $log_entry{log} =~ svk_header_regex( lenient => 1 );
+
 	my $author = $log_entry{author} = check_author($log_entry{author});
 	my ($name, $email) = defined $::users{$author} ? @{$::users{$author}}
 						       : ($author, undef);
@@ -3118,7 +3119,15 @@ sub make_log_entry {
 	my ($commit_name, $commit_email) = ($name, $email);
 	if ($_use_log_author) {
 		my $name_field;
-		if ($log_entry{log} =~ /From:\s+(.*\S)\s*\n/i) {
+		if ($log_entry{log_author}) {
+			$log_entry{commit_date} = $log_entry{date};
+			$log_entry{date} = $log_entry{log_author_date};
+			$log_entry{log_author} = check_author($log_entry{log_author});
+			my ($log_author_name, $log_author_email)
+			    = defined $::users{$log_entry{log_author}} ? @{$::users{$log_entry{log_author}}}
+			        : ($log_entry{log_author}, undef);
+			$name_field = "$log_author_name <$log_author_email>";
+		} elsif ($log_entry{log} =~ /From:\s+(.*\S)\s*\n/i) {
 			$name_field = $1;
 		} elsif ($log_entry{log} =~ /Signed-off-by:\s+(.*\S)\s*\n/i) {
 			$name_field = $1;
@@ -3182,6 +3191,91 @@ sub make_log_entry {
 	\%log_entry;
 }
 
+sub svk_header_regex {
+	my %args = ( lenient => 0, orig => 0, @_ );
+	my $orig = $args{orig} ? qr/ \(orig r\d+\)/ : "";
+	my $atstart = "";
+	if ($args{lenient}) {
+		$atstart = qr/\s*/;
+		$orig = qr/(?: \(orig r\d+\))?/;
+	}
+	return qr/^${atstart}r\d+\@\S+$orig:\s*(\S+)\s*\|\s*(.*?)\s*([+-]\d+)$/m;
+}
+
+sub parse_svk_log {
+	my $log_entry = shift;
+	my $log = $log_entry->{log};
+
+	# Strip off blank lines at the start and end
+	$log =~ s/^(\s*?\n)+//;
+	$log =~ s/\s*$//;
+
+	# If each line starts with a space, this might be an
+	# unmodified SVK log format.  As a side effect, this also
+	# trims the leading space off of the lines.
+	my $lines = $log =~ s/^//mg;
+	my $spaced = $log =~ s/^ //mg;
+	return unless $lines == $spaced;
+
+	my $regex = svk_header_regex( orig => 1 );
+	if ($log =~ /\A$regex/) {
+		# This is either a merge commit, or a base-less merge
+		# (replay from a different repository) The \A assures
+		# that this is an _unedited_ merge commit with no
+		# hand-supplied log message.
+		if (@{$log_entry->{merged_branches} || []}) {
+			# This is a merge with no description; provide
+			# one.
+			$log_entry->{log} = "Merge from @{$log_entry->{merged_branches}}\n\n$log";
+		} else {
+			my $commits = 0;
+			$commits++ while $log =~ /$regex/g;
+			if ($commits == 1) {
+				# This is a baseless merge of one
+				# commit; strip off the original
+				# commit info
+				$log_entry->{log_author} = $1;
+				$log_entry->{log_author_date} = "$3 $2";
+				$log =~ s/\A$regex\n*//;
+				$log_entry->{log} = $log;
+				parse_svk_log($log_entry);
+			} else {
+				# A lump baseless merge?  Remove all
+				# of the SVK headers on this level,
+				# and add a summary.  Trailing
+				# newlines on the svk header lines are
+				# left unmolested, so they become
+				# blank lines.
+				$log =~ s/$regex//g;
+				$log_entry->{log} = "Lump commit\n$log";
+			}
+		}
+	} else {
+		# Look for svk header lines without the (orig r12345),
+		# which were local commits.
+		$regex = svk_header_regex();
+		my $commits = 0;
+		$commits++ while $log =~ /$regex/g;
+		if ($commits == 0) {
+			# No more svk-like commits; don't change anything.
+		} elsif ($commits == 1) {
+			# Only one top-level commit-like object; strip
+			# it off, recurse down.
+			$log_entry->{log_author} = $1;
+			$log_entry->{log_author_date} = "$3 $2";
+			$log =~ s/$regex\n*//;
+			$log_entry->{log} = $log;
+			parse_svk_log($log_entry);
+		} else {
+			# This is a lump push of local commits.	 Strip
+			# off all of the svk headers in this level,
+			# and call it quits.
+			$log =~ s/$regex//g;
+			$log_entry->{log} = $log;
+		}
+	}
+}
+
 sub fetch {
 	my ($self, $min_rev, $max_rev, @parents) = @_;
 	my ($last_rev, $last_commit) = $self->last_rev_commit;
-- 
1.6.6.rc0.327.g032bc

[PATCH 4/5] git-svn: Provide a default "empty commit message" so the metadata is not the header

[Alex Vandiver]

git-svn adds a trailing line of metadata to the commit message.  If
the commit message would otherwise be empty, this can lead to
confusing display in `gitk` and `git log --oneline`.  Thus, provide a
no-op "(empty commit message)" message for the first line of such
messages.

Signed-off-by: Alex Vandiver <alex@chmrr.net>
---
 git-svn.perl |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 0731425..87462c9 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -3111,6 +3111,7 @@ sub make_log_entry {
 
 	$log_entry{date} = parse_svn_date($log_entry{date});
 	parse_svk_log(\%log_entry) if $log_entry{log} =~ svk_header_regex( lenient => 1 );
+	$log_entry{log} = "(empty commit message)\n" unless $log_entry{log} =~ /\S/;
 
 	my $author = $log_entry{author} = check_author($log_entry{author});
 	my ($name, $email) = defined $::users{$author} ? @{$::users{$author}}
-- 
1.6.6.rc0.327.g032bc

[PATCH 5/5] git-svn: Correct a copy-and-pasted misleading comment

[Alex Vandiver]

Signed-off-by: Alex Vandiver <alex@chmrr.net>
---
 git-svn.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 87462c9..804a26c 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2973,7 +2973,7 @@ sub find_extra_svk_parents {
 # have actually changed
 sub find_extra_svn_parents {
 	my ($self, $ed, $mergeinfo, $parents) = @_;
-	# aha!  svk:merge property changed...
+	# aha!  svn:mergeinfo property changed...
 
 	# We first search for merged tips which are not in our
 	# history.  Then, we figure out which git revisions are in
-- 
1.6.6.rc0.327.g032bc