[PATCH] git-svn.perl: Strip ChangeLog bits.

[PATCH] git-svn.perl: Strip ChangeLog bits.

[Jan Nieuwenhuizen]

Some projects write ChangeLog entries and use those for svn
commit messages.  Converting such a repository makes for
a most uninteresting git log --pretty=oneline output.

    482cfc6... 2008-08-01  Kohei Yoshida  <kyoshida@novell.com>
    90c16d7... 2008-08-01  Fong Lin  <pflin@novell.com> 	* patches/dev300/cws-npow
    505fc23... 2008-08-01  Kohei Yoshida  <kyoshida@novell.com>
    4729379... 2008-08-01  Kohei Yoshida  <kyoshida@novell.com>
    9e34b85... 2008-07-31  Kohei Yoshida  <kyoshida@novell.com>
    ee1855a... 2008-07-31  Jan Nieuwenhuizen  <janneke@gnu.org>
    518b921... 2008-07-31  Miklos Vajna  <vmiklos@frugalware.org>

The --cut-changelog-bits fixes this

    44e7784... patches/dev300/apply: added upstream issue numbers to some of my  p
    1a19026... patches/dev300/cws-npower11.diff: Add vba events stuff.
    7422531... patches/test/calc-external-defined-names.diff: more progress; make 
    66c5975... scratch/sc-xlsutil/src/formula.py:
    307fa53... patches/dev300/chart-subtitle-xls-export.diff: preserve the subtitl
    0d127af... patches/dev300/layout-simple-dialogs-toolkit.diff: Remove duplicate
    843d41e... patches/dev300/apply: Remove gcc401-stlport45-include.diff from Fru

partly.  I tried removing the file name bits too, but that makeslog
entries that touch multiple files almost useless.


Signed-off-by: Jan Nieuwenhuizen <Jan Nieuwenhuizen janneke@gnu.org>
---
 git-svn.perl |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index cf6dbbc..651739a 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -66,7 +66,7 @@ my ($_stdin, $_help, $_edit,
 	$_version, $_fetch_all, $_no_rebase,
 	$_merge, $_strategy, $_dry_run, $_local,
 	$_prefix, $_no_checkout, $_url, $_verbose,
-	$_git_format);
+	$_git_format, $_cut_changelog_bits);
 $Git::SVN::_follow_parent = 1;
 my %remote_opts = ( 'username=s' => \$Git::SVN::Prompt::_username,
                     'config-dir=s' => \$Git::SVN::Ra::config_dir,
@@ -109,9 +109,11 @@ my %cmd = (
 	fetch => [ \&cmd_fetch, "Download new revisions from SVN",
 			{ 'revision|r=s' => \$_revision,
 			  'fetch-all|all' => \$_fetch_all,
+			  'cut-changelog-bits' => \$_cut_changelog_bits,
 			   %fc_opts } ],
 	clone => [ \&cmd_clone, "Initialize and fetch revisions",
 			{ 'revision|r=s' => \$_revision,
+			  'cut-changelog-bits' => \$_cut_changelog_bits,
 			   %fc_opts, %init_opts } ],
 	init => [ \&cmd_init, "Initialize a repo for tracking" .
 			  " (requires URL argument)",
@@ -2463,6 +2465,13 @@ sub make_log_entry {
 	close $un or croak $!;
 
 	$log_entry{date} = parse_svn_date($log_entry{date});
+	if (defined $_cut_changelog_bits) {
+	    $log_entry{log} =~ s/(^|\n)\s*((\n|\s)*(199[0-9]|
20[0-1][0-9])(-[0-9]{2}){2}\s+.*<.*>\s*\n\s+)?/$1/g;
+	    $log_entry{log} =~ s/(^|\n)\* /\n$1/g;
+	    $log_entry{log} =~ s/^[\n\s]*//;
+	    $log_entry{log} =~ s/\n\s*/ /g if length ($log_entry{log}) < 81;
+	    $log_entry{log} .= "\n";
+	}
 	$log_entry{log} .= "\n";
 	my $author = $log_entry{author} = check_author($log_entry{author});
 	my ($name, $email) = defined $::users{$author} ? @{$::users{$author}}
-- 
1.6.0.rc0.44.g67270


-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond - The music typesetter
http://www.xs4all.nl/~jantien       | http://www.lilypond.org

Re: [PATCH] git-svn.perl: Strip ChangeLog bits.

[Petr Baudis]

  Hi!

On Sat, Aug 02, 2008 at 03:42:29PM +0200, Jan Nieuwenhuizen wrote:
> Signed-off-by: Jan Nieuwenhuizen <Jan Nieuwenhuizen janneke@gnu.org>

  Just your email should be within the <> brackets.

> ---
>  git-svn.perl |   11 ++++++++++-
>  1 files changed, 10 insertions(+), 1 deletions(-)

  You forgot to document your option. (And possibly write a testcase.)

				Petr "Pasky" Baudis

Re: [PATCH] git-svn.perl: Strip ChangeLog bits.

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

>   Hi!
>
> On Sat, Aug 02, 2008 at 03:42:29PM +0200, Jan Nieuwenhuizen wrote:
>> Signed-off-by: Jan Nieuwenhuizen <Jan Nieuwenhuizen janneke@gnu.org>
>
>   Just your email should be within the <> brackets.
>
>> ---
>>  git-svn.perl |   11 ++++++++++-
>>  1 files changed, 10 insertions(+), 1 deletions(-)
>
>   You forgot to document your option. (And possibly write a testcase.)

I am not sure if this is generic enough to be in git-svn.perl itself, or
perhaps there should be a hook make_log_entry() would call in the form of
some Perl scriptlet given by the user to munge $log_entry{log}, which
would be very specific to each project.

Re: [PATCH] git-svn.perl: Strip ChangeLog bits.

[Jan Nieuwenhuizen]

On za, 2008-08-02 at 10:36 -0700, Junio C Hamano wrote:

> >   You forgot to document your option. (And possibly write a testcase.)
> 
> I am not sure if this is generic enough to be in git-svn.perl itself, or
> perhaps there should be a hook make_log_entry() would call in the form of
> some Perl scriptlet given by the user to munge $log_entry{log}, which
> would be very specific to each project.

If you're not sure, please make up your mind.  I agree it's quite a hack
but now it's in the archives for others to find.  Doing this in a single
regexp is a bit tricky and asking a user to write a perl snippet is even
worse, imho.  Especially if would turn out that stripping changelog bits
is the only thing that the hook is getting used for, in the end.  I have
gotten more careful to provide generic solutions to specific problems in
anticipation of possible future desires.

I could imagine that leaving git-svn alone and adding a hook to git-log
would be more useful, though.

Jan.

-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond - The music typesetter
http://www.xs4all.nl/~jantien       | http://www.lilypond.org

Re: [PATCH] git-svn.perl: Strip ChangeLog bits.

[Jan Nieuwenhuizen]

On za, 2008-08-02 at 19:27 +0200, Petr Baudis wrote:

> On Sat, Aug 02, 2008 at 03:42:29PM +0200, Jan Nieuwenhuizen wrote:
> > Signed-off-by: Jan Nieuwenhuizen <Jan Nieuwenhuizen janneke@gnu.org>
> 
>   Just your email should be within the <> brackets.

Oops :-)  I have

    EMAIL='Jan Nieuwenhuizen <janneke@gnu.org>'

so I should probably use GIT_EMAIL='janneke@gnu.org' or something.  I'll
have a look.

Jan.

-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond - The music typesetter
http://www.xs4all.nl/~jantien       | http://www.lilypond.org

Re: [PATCH] git-svn.perl: Strip ChangeLog bits.

[Junio C Hamano]

Jan Nieuwenhuizen <janneke-list@xs4all.nl> writes:

> On za, 2008-08-02 at 10:36 -0700, Junio C Hamano wrote:
>
>> >   You forgot to document your option. (And possibly write a testcase.)
>> 
>> I am not sure if this is generic enough to be in git-svn.perl itself, or
>> perhaps there should be a hook make_log_entry() would call in the form of
>> some Perl scriptlet given by the user to munge $log_entry{log}, which
>> would be very specific to each project.
>
> If you're not sure, please make up your mind.

That's something you would say when I cannot decide the color of
bikeshed.  I do not think your change falls into that category.

We could add an ad-hoc preprocessing option like this, and keep adding
more for different patterns, and at certain point we may be fed up with
millions of such options and try to introduce a more generic mechanism.
While doing so, the resulting code needs to support the ad-hoc ones that
are added earlier, forever.

We've done that in the past with other commands (cc-suppression scheme in
send-email comes to mind).  It was very unpleasant.

> ...  Doing this in a single
> regexp is a bit tricky and asking a user to write a perl snippet is even
> worse, imho.

What you are saying is that a built-in one, no matter what, won't be
sufficient for many projects.  Unless a user writes Perl snippet to match
his project's needs, the noise at the beginning of the log won't be
stripped for him.

That's fine.  I do not expect a single built-in transformation would fit
everybody's needs.  I am not asking for miracles.

But you could at least keep the door open for people who are _willing_ to
write such transformation for their projects, right?

For one thing, your --cut-changelog-bits has one fixed pattern.  Later
people either have to come up with different option, or modify your
pattern (potentially breaking your project).  Neither is good.

Perhaps doing something like this a (admittedly slightly) better option?
It allows you to choose from a canned set, or give a series of s///
rewriting rules (or whatever you would want to have in the custom function)..

---

 git-svn.perl |   34 +++++++++++++++++++++++++++++++++-
 1 files changed, 33 insertions(+), 1 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index cf6dbbc..eaf6a56 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -66,7 +66,7 @@ my ($_stdin, $_help, $_edit,
 	$_version, $_fetch_all, $_no_rebase,
 	$_merge, $_strategy, $_dry_run, $_local,
 	$_prefix, $_no_checkout, $_url, $_verbose,
-	$_git_format);
+	$_git_format, $_clean_changelog, $_clean_log_message);
 $Git::SVN::_follow_parent = 1;
 my %remote_opts = ( 'username=s' => \$Git::SVN::Prompt::_username,
                     'config-dir=s' => \$Git::SVN::Ra::config_dir,
@@ -109,9 +109,11 @@ my %cmd = (
 	fetch => [ \&cmd_fetch, "Download new revisions from SVN",
 			{ 'revision|r=s' => \$_revision,
 			  'fetch-all|all' => \$_fetch_all,
+			  'clean-changelog=s' => \$_clean_changelog,
 			   %fc_opts } ],
 	clone => [ \&cmd_clone, "Initialize and fetch revisions",
 			{ 'revision|r=s' => \$_revision,
+			  'clean-changelog=s' => \$_clean_changelog,
 			   %fc_opts, %init_opts } ],
 	init => [ \&cmd_init, "Initialize a repo for tracking" .
 			  " (requires URL argument)",
@@ -238,6 +240,33 @@ my $rv = GetOptions(%opts, 'help|H|h' => \$_help, 'version|V' => \$_version,
                        $Git::SVN::default_repo_id = $_[1] });
 exit 1 if (!$rv && $cmd && $cmd ne 'log');
 
+my %canned_changelog_cleaner =
+(
+ 'ooo' => sub {
+	 local ($_) = @_;
+	 s/(^|\n)\s*((\n|\s)*(199[0-9]|20[0-1][0-9])(-[0-9]{2}){2}\s+.*<.*>\s*\n\s+)?/$1/g;
+	 s/(^|\n)\* /\n$1/g;
+	 s/^[\n\s]*//;
+	 s/\n\s*/ /g if length ($_) < 81;
+	 "\n";
+ }
+);
+
+if (defined $_clean_changelog) {
+	if (exists $canned_changelog_cleaner{$_clean_changelog}) {
+		$_clean_log_message = $canned_changelog_cleaner{$_clean_changelog};
+	} elsif ($_clean_changelog ne '') {
+		$_clean_log_message = eval "
+			sub { local(\$_) = \@_; $_clean_changelog; return \$_; }
+		";
+		if ($@) {
+			die "$!: $_clean_changelog";
+		}
+	} else {
+		die "$_clean_changelog: unknown way to clean log message";
+	}
+}
+
 usage(0) if $_help;
 version() if $_version;
 usage(1) unless defined $cmd;
@@ -2463,6 +2492,9 @@ sub make_log_entry {
 	close $un or croak $!;
 
 	$log_entry{date} = parse_svn_date($log_entry{date});
+	if ($_clean_log_message) {
+		$log_entry{log} = $_clean_log_message->($log_entry{log});
+	}
 	$log_entry{log} .= "\n";
 	my $author = $log_entry{author} = check_author($log_entry{author});
 	my ($name, $email) = defined $::users{$author} ? @{$::users{$author}}

Re: [PATCH] git-svn.perl: Strip ChangeLog bits.

[Jan Nieuwenhuizen]

On za, 2008-08-02 at 14:13 -0700, Junio C Hamano wrote:

> Perhaps doing something like this a (admittedly slightly) better option?
> It allows you to choose from a canned set, or give a series of s///
> rewriting rules (or whatever you would want to have in the custom function)..

Yes, this is (a bit) much nicer, thanks.

I fixed the function to actually return the log entry, renamed it to
'gnu', as it--although this is ooo's silly convention--is a GNU style
ChangeLog that is being stripped, added it to the 'rebase' command,
added some doco and a test.

Jan.


Signed-off-by: Jan Nieuwenhuizen <janneke@gnu.org>
---
 Documentation/RelNotes-1.6.0.txt   |    3 +
 Documentation/git-svn.txt          |    4 +
 git-svn.perl                       |   36 ++++++++-
 t/t9125-git-svn-clean-changelog.sh |  171 ++++++++++++++++++++++++++++++++++++
 4 files changed, 213 insertions(+), 1 deletions(-)
 create mode 100644 t/t9125-git-svn-clean-changelog.sh

diff --git a/Documentation/RelNotes-1.6.0.txt b/Documentation/RelNotes-1.6.0.txt
index 2542cf5..917b05a 100644
--- a/Documentation/RelNotes-1.6.0.txt
+++ b/Documentation/RelNotes-1.6.0.txt
@@ -206,6 +206,9 @@ Updates since v1.5.6
 * "git-svn dcommit" is now aware of auto-props setting the subversion user
   has.
 
+* git-svn can process changelog entries in order to produce nicer
+  commit messages with the new option --clean-changelog=gnu.
+
 * You can tell "git status -u" to even more aggressively omit checking
   untracked files with --untracked-files=no.
 
diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index f230125..653d1fc 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -118,6 +118,10 @@ accept.  However, '--fetch-all' only fetches from the current
 Like 'git-rebase'; this requires that the working tree be clean
 and have no uncommitted changes.
 
+--clean-changelog=gnu;;
+	Manipulate SVN commit log messages to produce clean commit
+	messages.  Supported formats are: 'gnu', which strips
+	GNU ChangeLog bits.
 -l;;
 --local;;
 	Do not fetch remotely; only run 'git-rebase' against the
diff --git a/git-svn.perl b/git-svn.perl
index cf6dbbc..fe4d147 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -66,7 +66,7 @@ my ($_stdin, $_help, $_edit,
 	$_version, $_fetch_all, $_no_rebase,
 	$_merge, $_strategy, $_dry_run, $_local,
 	$_prefix, $_no_checkout, $_url, $_verbose,
-	$_git_format);
+	$_git_format, $_clean_changelog, $_clean_log_message);
 $Git::SVN::_follow_parent = 1;
 my %remote_opts = ( 'username=s' => \$Git::SVN::Prompt::_username,
                     'config-dir=s' => \$Git::SVN::Ra::config_dir,
@@ -109,9 +109,11 @@ my %cmd = (
 	fetch => [ \&cmd_fetch, "Download new revisions from SVN",
 			{ 'revision|r=s' => \$_revision,
 			  'fetch-all|all' => \$_fetch_all,
+			  'clean-changelog=s' => \$_clean_changelog,
 			   %fc_opts } ],
 	clone => [ \&cmd_clone, "Initialize and fetch revisions",
 			{ 'revision|r=s' => \$_revision,
+			  'clean-changelog=s' => \$_clean_changelog,
 			   %fc_opts, %init_opts } ],
 	init => [ \&cmd_init, "Initialize a repo for tracking" .
 			  " (requires URL argument)",
@@ -178,6 +180,7 @@ my %cmd = (
 			  'local|l' => \$_local,
 			  'fetch-all|all' => \$_fetch_all,
 			  'dry-run|n' => \$_dry_run,
+			  'clean-changelog=s' => \$_clean_changelog,
 			  %fc_opts } ],
 	'commit-diff' => [ \&cmd_commit_diff,
 	                   'Commit a diff between two trees',
@@ -238,6 +241,34 @@ my $rv = GetOptions(%opts, 'help|H|h' => \$_help, 'version|V' => \$_version,
                        $Git::SVN::default_repo_id = $_[1] });
 exit 1 if (!$rv && $cmd && $cmd ne 'log');
 
+my %canned_changelog_cleaner =
+(
+ 'gnu' => sub {
+	 local ($_) = @_;
+	 s/(^|\n)\s*((\n|\s)*(199[0-9]|20[0-1][0-9])(-[0-9]{2}){2}\s+.*<.*>\s*\n\s+)?/$1/g;
+	 s/(^|\n)\* /\n$1/g;
+	 s/^[\n\s]*//;
+	 s/[\n\s]*$//;
+	 s/\n\s*/ /g if length ($_) < 81;
+	 $_ . "\n";
+ }
+);
+
+if (defined $_clean_changelog) {
+	if (exists $canned_changelog_cleaner{$_clean_changelog}) {
+		$_clean_log_message = $canned_changelog_cleaner{$_clean_changelog};
+	} elsif ($_clean_changelog ne '') {
+		$_clean_log_message = eval "
+			sub { local(\$_) = \@_; $_clean_changelog; return \$_; }
+		";
+		if ($@) {
+			die "$!: $_clean_changelog";
+		}
+	} else {
+		die "$_clean_changelog: unknown way to clean log message";
+	}
+}
+
 usage(0) if $_help;
 version() if $_version;
 usage(1) unless defined $cmd;
@@ -2463,6 +2494,9 @@ sub make_log_entry {
 	close $un or croak $!;
 
 	$log_entry{date} = parse_svn_date($log_entry{date});
+	if ($_clean_log_message) {
+		$log_entry{log} = $_clean_log_message->($log_entry{log});
+	}
 	$log_entry{log} .= "\n";
 	my $author = $log_entry{author} = check_author($log_entry{author});
 	my ($name, $email) = defined $::users{$author} ? @{$::users{$author}}
diff --git a/t/t9125-git-svn-clean-changelog.sh b/t/t9125-git-svn-clean-changelog.sh
new file mode 100644
index 0000000..1a7c5e1
--- /dev/null
+++ b/t/t9125-git-svn-clean-changelog.sh
@@ -0,0 +1,171 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Jan Nieuwenhuizen <janneke@gnu.org>
+
+test_description='git-svn fetch --clean-changelog'
+
+. ./lib-git-svn.sh
+
+cat > dumpfile.svn <<EOF
+SVN-fs-dump-format-version: 2
+
+UUID: 3e54420c-6148-11dd-bf80-b901c556354a
+
+Revision-number: 0
+Prop-content-length: 56
+Content-length: 56
+
+K 8
+svn:date
+V 27
+2008-08-03T10:38:00.279529Z
+PROPS-END
+
+Revision-number: 1
+Prop-content-length: 231
+Content-length: 231
+
+K 7
+svn:log
+V 128
+2008-08-03  Jan Nieuwenhuizen  <janneke@gnu.org>
+
+	* one: New file.  This adds the \`one' file to the svn
+          repository.
+
+
+K 10
+svn:author
+V 7
+janneke
+K 8
+svn:date
+V 27
+2008-08-03T10:42:21.211246Z
+PROPS-END
+
+Node-path: one
+Node-kind: file
+Node-action: add
+Prop-content-length: 10
+Text-content-length: 4
+Text-content-md5: 5bbf5a52328e7439ae6e719dfe712200
+Content-length: 14
+
+PROPS-END
+one
+
+
+Revision-number: 2
+Prop-content-length: 217
+Content-length: 217
+
+K 7
+svn:log
+V 114
+2008-08-03  Jan Nieuwenhuizen  <janneke@gnu.org>
+
+	* one: Change to numeral.
+	* /path/to/two (foo, bar): Update.
+
+
+K 10
+svn:author
+V 7
+janneke
+K 8
+svn:date
+V 27
+2008-08-03T10:47:11.023749Z
+PROPS-END
+
+Node-path: one
+Node-kind: file
+Node-action: change
+Text-content-length: 2
+Text-content-md5: b026324c6904b2a9cb4b88d6d61c81d1
+Content-length: 2
+
+1
+
+
+Node-path: two
+Node-kind: file
+Node-action: add
+Prop-content-length: 10
+Text-content-length: 4
+Text-content-md5: c193497a1a06b2c72230e6146ff47080
+Content-length: 14
+
+PROPS-END
+two
+
+
+Revision-number: 3
+Prop-content-length: 192
+Content-length: 192
+
+K 7
+svn:log
+V 90
+    * three: Add yet another commit that is too long to fit on one
+line, i.e., 81 chars.
+
+
+K 10
+svn:author
+V 7
+janneke
+K 8
+svn:date
+V 27
+2008-08-03T10:51:21.214317Z
+PROPS-END
+
+Node-path: three
+Node-kind: file
+Node-action: add
+Prop-content-length: 10
+Text-content-length: 6
+Text-content-md5: febe6995bad457991331348f7b9c85fa
+Content-length: 16
+
+PROPS-END
+three
+
+
+EOF
+
+test_expect_success 'load svn dumpfile' 'svnadmin load "$PWD/svnrepo" < dumpfile.svn'
+test_expect_success 'clone revision from svn' 'git-svn clone --clean-changelog=gnu -r0 "file://$PWD/svnrepo" test-git'
+test_expect_success 'fetch revision from svn' '(cd test-git && git-svn fetch --clean-changelog=gnu)'
+test_expect_success 'rebase revision from svn' '(cd test-git && git-svn rebase --clean-changelog=gnu)'
+test_expect_success 'get log' '(cd test-git && git log | grep -Ev "^(commit|Author)" > git-log.output)'
+
+cat > git-log.expect << EOF
+Date:   Sun Aug 3 10:51:21 2008 +0000
+
+    three: Add yet another commit that is too long to fit on one
+    line, i.e., 81 chars.
+    
+    
+    git-svn-id: file://$PWD/svnrepo@3 3e54420c-6148-11dd-bf80-b901c556354a
+
+Date:   Sun Aug 3 10:47:11 2008 +0000
+
+    one: Change to numeral. /path/to/two (foo, bar): Update.
+    
+    
+    git-svn-id: file://$PWD/svnrepo@2 3e54420c-6148-11dd-bf80-b901c556354a
+
+Date:   Sun Aug 3 10:42:21 2008 +0000
+
+    one: New file.  This adds the \`one' file to the svn repository.
+    
+    
+    git-svn-id: file://$PWD/svnrepo@1 3e54420c-6148-11dd-bf80-b901c556354a
+EOF
+
+test_expect_success 'git svn clone/fetch --changelog=gnu' 'cmp git-log.expect test-git/git-log.output'
+
+test_done
-- 
1.6.0.rc0.44.g67270




-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond - The music typesetter
http://www.xs4all.nl/~jantien       | http://www.lilypond.org

Re: [PATCH] git-svn.perl: Strip ChangeLog bits.

[Junio C Hamano]

Jan Nieuwenhuizen <janneke-list@xs4all.nl> writes:

>  Documentation/RelNotes-1.6.0.txt   |    3 +

Nice try, but after -rc1 we won't take feature enhancements on the
'master' branch.  The earliest this will appear is in 1.6.1.

Thanks for clean-up, and starting tests, but I have a few more comments:

 * Documentation; introduce this with heading --clean-changelog=<style>; I
   kept the list of supported styles in-line, but we may later want to
   have other supported styles at which time it might become easier to
   read if it were a separate itemized list.

   You seem to have taken the "arbitrary Perl snippet" part of my patch as
   well, but it is not described here...

 * Script; two separate _clean_changelog and _clean_log_message variables
   are not necessary (I removed the extra variable in the patch below).

 * Tests.

   Your new tests do not seem to check these, but I think you should:

   - what should happen without --clean-changelog=gnu?  (iow, additional
     code does not regress the behaviour when this shiny new toy is not
     used).

   - what should happen when an unknown style is given e.g. --clean-changelog=yak?

   We prefer to use "test_cmp" for comparing expected and actual result,
   not bare "cmp".

Here is what I tested and based the above comments on after minor fixes to
ask comments from Eric.

-- >8 --
From: Jan Nieuwenhuizen <janneke@gnu.org>
Subject: git-svn: --clean-changelog=<style> to sanitize messages

Some projects write ChangeLog entries and use those for Subverison commit
messages.  Converting such a repository makes for a most uninteresting git
log --pretty=oneline output:

    482cfc6... 2008-08-01  Kohei Yoshida  <kyoshida@novell.com>
    90c16d7... 2008-08-01  Fong Lin  <pflin@novell.com> 	* patches/dev300/cws-npow
    ...

The --clean-changelog=<style> option can be used to remove useless bits
when reading log messages from Subversion side, to produce more readable
result:

    44e7784... patches/dev300/apply: added upstream issue numbers to some of my  p
    1a19026... patches/dev300/cws-npower11.diff: Add vba events stuff.
    ...

Signed-off-by: Jan Nieuwenhuizen <janneke@gnu.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-svn.txt          |    4 +
 git-svn.perl                       |   37 +++++++-
 t/t9125-git-svn-clean-changelog.sh |  187 ++++++++++++++++++++++++++++++++++++
 3 files changed, 227 insertions(+), 1 deletions(-)
 create mode 100755 t/t9125-git-svn-clean-changelog.sh

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index f230125..04b44c6 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -118,6 +118,10 @@ accept.  However, '--fetch-all' only fetches from the current
 Like 'git-rebase'; this requires that the working tree be clean
 and have no uncommitted changes.
 
+--clean-changelog=<style>;;
+	Manipulate SVN commit log messages to produce clean commit
+	messages.  Supported styles are: 'gnu', which strips
+	GNU ChangeLog bits.
 -l;;
 --local;;
 	Do not fetch remotely; only run 'git-rebase' against the
diff --git a/git-svn.perl b/git-svn.perl
index cf6dbbc..ff17457 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -66,7 +66,7 @@ my ($_stdin, $_help, $_edit,
 	$_version, $_fetch_all, $_no_rebase,
 	$_merge, $_strategy, $_dry_run, $_local,
 	$_prefix, $_no_checkout, $_url, $_verbose,
-	$_git_format);
+	$_git_format, $_clean_changelog);
 $Git::SVN::_follow_parent = 1;
 my %remote_opts = ( 'username=s' => \$Git::SVN::Prompt::_username,
                     'config-dir=s' => \$Git::SVN::Ra::config_dir,
@@ -109,9 +109,11 @@ my %cmd = (
 	fetch => [ \&cmd_fetch, "Download new revisions from SVN",
 			{ 'revision|r=s' => \$_revision,
 			  'fetch-all|all' => \$_fetch_all,
+			  'clean-changelog=s' => \$_clean_changelog,
 			   %fc_opts } ],
 	clone => [ \&cmd_clone, "Initialize and fetch revisions",
 			{ 'revision|r=s' => \$_revision,
+			  'clean-changelog=s' => \$_clean_changelog,
 			   %fc_opts, %init_opts } ],
 	init => [ \&cmd_init, "Initialize a repo for tracking" .
 			  " (requires URL argument)",
@@ -178,6 +180,7 @@ my %cmd = (
 			  'local|l' => \$_local,
 			  'fetch-all|all' => \$_fetch_all,
 			  'dry-run|n' => \$_dry_run,
+			  'clean-changelog=s' => \$_clean_changelog,
 			  %fc_opts } ],
 	'commit-diff' => [ \&cmd_commit_diff,
 	                   'Commit a diff between two trees',
@@ -238,6 +241,35 @@ my $rv = GetOptions(%opts, 'help|H|h' => \$_help, 'version|V' => \$_version,
                        $Git::SVN::default_repo_id = $_[1] });
 exit 1 if (!$rv && $cmd && $cmd ne 'log');
 
+my %canned_changelog_cleaner =
+(
+ 'gnu' => sub {
+	 local ($_) = @_;
+	 s/(^|\n)\s*((\n|\s)*(199[0-9]|20[0-1][0-9])(-[0-9]{2}){2}\s+.*<.*>\s*\n\s+)?/$1/g;
+	 s/(^|\n)\* /\n$1/g;
+	 s/^[\n\s]*//;
+	 s/[\n\s]*$//;
+	 s/\n\s*/ /g if length ($_) < 81;
+	 $_ . "\n";
+ }
+);
+
+if (defined $_clean_changelog) {
+	my $ccl = $_clean_changelog;
+	if (exists $canned_changelog_cleaner{$_clean_changelog}) {
+		$_clean_changelog = $canned_changelog_cleaner{$_clean_changelog};
+	} elsif ($_clean_changelog ne '') {
+		$_clean_changelog = eval "
+			sub { local(\$_) = \@_; $_clean_changelog; return \$_; }
+		";
+		if ($@) {
+			die "$!: $ccl";
+		}
+	} else {
+		die "$ccl: unknown way to clean log message";
+	}
+}
+
 usage(0) if $_help;
 version() if $_version;
 usage(1) unless defined $cmd;
@@ -2463,6 +2495,9 @@ sub make_log_entry {
 	close $un or croak $!;
 
 	$log_entry{date} = parse_svn_date($log_entry{date});
+	if ($_clean_changelog) {
+		$log_entry{log} = $_clean_changelog->($log_entry{log});
+	}
 	$log_entry{log} .= "\n";
 	my $author = $log_entry{author} = check_author($log_entry{author});
 	my ($name, $email) = defined $::users{$author} ? @{$::users{$author}}
diff --git a/t/t9125-git-svn-clean-changelog.sh b/t/t9125-git-svn-clean-changelog.sh
new file mode 100755
index 0000000..f6a02b8
--- /dev/null
+++ b/t/t9125-git-svn-clean-changelog.sh
@@ -0,0 +1,187 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Jan Nieuwenhuizen <janneke@gnu.org>
+
+test_description='git-svn fetch --clean-changelog'
+
+. ./lib-git-svn.sh
+
+cat >dumpfile.svn <<\EOF
+SVN-fs-dump-format-version: 2
+
+UUID: 3e54420c-6148-11dd-bf80-b901c556354a
+
+Revision-number: 0
+Prop-content-length: 56
+Content-length: 56
+
+K 8
+svn:date
+V 27
+2008-08-03T10:38:00.279529Z
+PROPS-END
+
+Revision-number: 1
+Prop-content-length: 231
+Content-length: 231
+
+K 7
+svn:log
+V 128
+2008-08-03  Jan Nieuwenhuizen  <janneke@gnu.org>
+
+	* one: New file.  This adds the `one' file to the svn
+          repository.
+
+
+K 10
+svn:author
+V 7
+janneke
+K 8
+svn:date
+V 27
+2008-08-03T10:42:21.211246Z
+PROPS-END
+
+Node-path: one
+Node-kind: file
+Node-action: add
+Prop-content-length: 10
+Text-content-length: 4
+Text-content-md5: 5bbf5a52328e7439ae6e719dfe712200
+Content-length: 14
+
+PROPS-END
+one
+
+
+Revision-number: 2
+Prop-content-length: 217
+Content-length: 217
+
+K 7
+svn:log
+V 114
+2008-08-03  Jan Nieuwenhuizen  <janneke@gnu.org>
+
+	* one: Change to numeral.
+	* /path/to/two (foo, bar): Update.
+
+
+K 10
+svn:author
+V 7
+janneke
+K 8
+svn:date
+V 27
+2008-08-03T10:47:11.023749Z
+PROPS-END
+
+Node-path: one
+Node-kind: file
+Node-action: change
+Text-content-length: 2
+Text-content-md5: b026324c6904b2a9cb4b88d6d61c81d1
+Content-length: 2
+
+1
+
+
+Node-path: two
+Node-kind: file
+Node-action: add
+Prop-content-length: 10
+Text-content-length: 4
+Text-content-md5: c193497a1a06b2c72230e6146ff47080
+Content-length: 14
+
+PROPS-END
+two
+
+
+Revision-number: 3
+Prop-content-length: 192
+Content-length: 192
+
+K 7
+svn:log
+V 90
+    * three: Add yet another commit that is too long to fit on one
+line, i.e., 81 chars.
+
+
+K 10
+svn:author
+V 7
+janneke
+K 8
+svn:date
+V 27
+2008-08-03T10:51:21.214317Z
+PROPS-END
+
+Node-path: three
+Node-kind: file
+Node-action: add
+Prop-content-length: 10
+Text-content-length: 6
+Text-content-md5: febe6995bad457991331348f7b9c85fa
+Content-length: 16
+
+PROPS-END
+three
+
+
+EOF
+
+test_expect_success 'load svn dumpfile' '
+	svnadmin load "$PWD/svnrepo" <dumpfile.svn
+'
+
+test_expect_success 'clone revision from svn' '
+	git-svn clone --clean-changelog=gnu -r0 "file://$PWD/svnrepo" test-git
+'
+
+test_expect_success 'fetch revision from svn' '
+	(cd test-git && git-svn fetch --clean-changelog=gnu)
+'
+
+test_expect_success 'rebase revision from svn' '
+	(cd test-git && git-svn rebase --clean-changelog=gnu)
+'
+
+test_expect_success 'get log' '
+	(cd test-git && git log | grep -Ev "^(commit|Author)" >git-log.output)
+'
+
+sed -e 's/|$//' >git-log.expect <<EOF
+Date:   Sun Aug 3 10:51:21 2008 +0000
+
+    three: Add yet another commit that is too long to fit on one
+    line, i.e., 81 chars.
+    |
+    |
+    git-svn-id: file://$PWD/svnrepo@3 3e54420c-6148-11dd-bf80-b901c556354a
+
+Date:   Sun Aug 3 10:47:11 2008 +0000
+
+    one: Change to numeral. /path/to/two (foo, bar): Update.
+    |
+    |
+    git-svn-id: file://$PWD/svnrepo@2 3e54420c-6148-11dd-bf80-b901c556354a
+
+Date:   Sun Aug 3 10:42:21 2008 +0000
+
+    one: New file.  This adds the \`one' file to the svn repository.
+    |
+    |
+    git-svn-id: file://$PWD/svnrepo@1 3e54420c-6148-11dd-bf80-b901c556354a
+EOF
+
+test_expect_success 'git svn clone/fetch --changelog=gnu' '
+	test_cmp git-log.expect test-git/git-log.output
+'
+
+test_done
-- 
1.6.0.rc1.58.g807d8

Re: [PATCH] git-svn.perl: Strip ChangeLog bits.

[Eric Wong]

Jan Nieuwenhuizen <janneke-list@xs4all.nl> wrote:
> On za, 2008-08-02 at 10:36 -0700, Junio C Hamano wrote:
> 
> > >   You forgot to document your option. (And possibly write a testcase.)
> > 
> > I am not sure if this is generic enough to be in git-svn.perl itself, or
> > perhaps there should be a hook make_log_entry() would call in the form of
> > some Perl scriptlet given by the user to munge $log_entry{log}, which
> > would be very specific to each project.
> 
> If you're not sure, please make up your mind.  I agree it's quite a hack
> but now it's in the archives for others to find.  Doing this in a single
> regexp is a bit tricky and asking a user to write a perl snippet is even
> worse, imho.  Especially if would turn out that stripping changelog bits
> is the only thing that the hook is getting used for, in the end.  I have
> gotten more careful to provide generic solutions to specific problems in
> anticipation of possible future desires.
> 
> I could imagine that leaving git-svn alone and adding a hook to git-log
> would be more useful, though.

NACK on modifying git-svn to support more changelog formats.

A better idea would be to write a generic script that takes "git log",
"git svn log" or even plain "svn log" output and filters it
independently.

This way existing projects don't have to be re-imported (a bad idea to
modify things SVN feeds us anyways), and plain svn users can benefit,
too.

This filter should be reusable for both plain svn and git-svn:

    svn log | changelog-filter --input=svn --style=gnu
    git svn log | changelog-filter --input=svn --style=gnu
    git log --pretty=raw | changelog-filter --input=git-raw --style=gnu


However, I would support a generic --log-filter parameter in git-svn
that would have git-svn filter its output through any given command
before piping it to less.

    git config svn.logFilter "changelog-filter --input=svn --style=gnu"
    git svn log
      or...
    git svn log --log-filter="changelog-filter --input=svn --style=yak"
    git svn log --log-filter="svn-log-to-LaTeX"
    git svn log --log-filter="svn-log-to-HTML"
    git svn log --log-filter="svn-log-to-XML"
    git svn log --log-filter="svn-log-to-JSON"
    git svn log --log-filter="svn-log-to-PNG" > log.png
    git svn log --log-filter="svn-log-to-theora" > log.ogg

    The possibilities are endless :)

-- 
Eric Wong

Re: [PATCH] git-svn.perl: Strip ChangeLog bits.

[Junio C Hamano]

Eric Wong <normalperson@yhbt.net> writes:

>> I could imagine that leaving git-svn alone and adding a hook to git-log
>> would be more useful, though.
>
> NACK on modifying git-svn to support more changelog formats.

Honestly, I am neutral about this one.  I think it all depends on the
motivation behind the desire to rewrite the log.

If the project that was hosted in Subversion wants to switch (perhaps
gradually) to git, _and_ if the project also wants to adopt a workflow
that does not do the GNU style changelog in the commit log but use a
one-line summary friendly format, then sanitizing the commit log while
importing via git-svn would make sense.  Even though filter-branch after
conversion would be another possibility, that option is only available if
you are converting away from Subversion, never to return.

On the other extreme, if the Subversion side will always be the canonical
one, _or_ if the project does not want to change its commit log format,
then I think it makes perfect sense to limit the commit log munging to the
absolute minimum (but even in such a case, the user can just run git-svn
without activating the log munging option --- so I do not think it is such
a big deal to add or reject an option like this).

> A better idea would be to write a generic script that takes "git log",
> "git svn log" or even plain "svn log" output and filters it
> independently.

That's an independent topic.  If such a filter supports a feature like our
shortlog command has, people whose history is still trapped in Subversion
would benefit from it.

Re: [PATCH] git-svn.perl: Strip ChangeLog bits.

[Jan Nieuwenhuizen]

On zo, 2008-08-03 at 19:09 -0700, Eric Wong wrote:

> Jan Nieuwenhuizen <janneke-list@xs4all.nl> wrote:
> > I could imagine that leaving git-svn alone and adding a hook to git-log
> > would be more useful, though.
> 
> NACK on modifying git-svn to support more changelog formats.
> 
> A better idea would be to write a generic script that takes "git log",
> "git svn log" or even plain "svn log" output and filters it
> independently.

> This filter should be reusable for both plain svn and git-svn:

>     The possibilities are endless :)

Yes, but we'll most probably drop SVN rsn, possibly after a short period
of supporting both; and not stripping the cruft in the conversion means
we carry this with us until eternity.  Besides, who is going to
distribute the script, inform users about its availability?

What we have now suffices for our ooo-build conversion.  If you do not
find it useful, more power/less code to you.  Let others search the
archives and/or patch git-svn themselves, if indeed there are any.

Jan.

-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond - The music typesetter
http://www.xs4all.nl/~jantien       | http://www.lilypond.org

Re: [PATCH] git-svn.perl: Strip ChangeLog bits.

[Jan Nieuwenhuizen]

On zo, 2008-08-03 at 13:45 -0700, Junio C Hamano wrote:

> Nice try, but after -rc1 we won't take feature enhancements on the
> 'master' branch.  The earliest this will appear is in 1.6.1.

Ok, I'm not that familiar with git development and I did not find any
newer/UNRELEASED list of features?

>  * Documentation; introduce this with heading --clean-changelog=<style>; I

Ok.  I tried ={gnu} first, which seems to be the style for multiple
choice arguments, but the document parser does not grok that.  {gnu|foo}
or {gnu|no-other-yet} did not really please me.

>    You seem to have taken the "arbitrary Perl snippet" part of my patch as
>    well, but it is not described here...

It all depends upon how you read the future.  I would most have chosen
to postpone that work until the second (or third) request for different
munging came in, but now that the code is already written...

>  * Script; two separate _clean_changelog and _clean_log_message variables
>    are not necessary (I removed the extra variable in the patch below).

Good, I didn't really look at that.

>    Your new tests do not seem to check these, but I think you should:

>    - what should happen without --clean-changelog=gnu?  (iow, additional
>      code does not regress the behaviour when this shiny new toy is not
>      used).

We could add a test to make sure that git-svn does not alter commit
messages, but it has little to do with this patch.

If this is not being tested atm, it is probably not deemed important
enough to test.  This could have regressed at any time.  

I would add a test for existing working code only if experience tells
you it is fragile and it (often) regresses, ie, when you fix a bug:
new/revised code.

>    - what should happen when an unknown style is given e.g. --clean-changelog=yak?

It would be nice if the script failed with an error message, telling
what the options are, but I do not really care that much about wrong
use.  You have that automatically if you use a sensible option parser,
this is where such a feature should be implemented, imho.

>    We prefer to use "test_cmp" for comparing expected and actual result,
>    not bare "cmp".

Ok.

> Here is what I tested and based the above comments on after minor fixes to
> ask comments from Eric.

Great, thanks.  We'll see what happens then.

Jan.

-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond - The music typesetter
http://www.xs4all.nl/~jantien       | http://www.lilypond.org

Re: [PATCH] git-svn.perl: Strip ChangeLog bits.

[Eric Wong]

Jan Nieuwenhuizen <janneke-list@xs4all.nl> wrote:
> On zo, 2008-08-03 at 19:09 -0700, Eric Wong wrote:
> 
> > Jan Nieuwenhuizen <janneke-list@xs4all.nl> wrote:
> > > I could imagine that leaving git-svn alone and adding a hook to git-log
> > > would be more useful, though.
> > 
> > NACK on modifying git-svn to support more changelog formats.
> > 
> > A better idea would be to write a generic script that takes "git log",
> > "git svn log" or even plain "svn log" output and filters it
> > independently.
> 
> > This filter should be reusable for both plain svn and git-svn:
> 
> >     The possibilities are endless :)
> 
> Yes, but we'll most probably drop SVN rsn, possibly after a short period
> of supporting both; and not stripping the cruft in the conversion means
> we carry this with us until eternity.  Besides, who is going to
> distribute the script, inform users about its availability?
> 
> What we have now suffices for our ooo-build conversion.  If you do not
> find it useful, more power/less code to you.  Let others search the
> archives and/or patch git-svn themselves, if indeed there are any.

In the one-shot case, a git filter-branch script would probably be ideal
and reusable for other projects.

Anyways, I strongly believe changelog modification/reformatting should
be done in a more generic way that can benefit users of other tools
(archimport/cvsimport etc...), too.

-- 
Eric Wong

Re: [PATCH] git-svn.perl: Strip ChangeLog bits.

[Jan Nieuwenhuizen]

On ma, 2008-08-04 at 02:03 -0700, Eric Wong wrote:

> In the one-shot case, a git filter-branch script would probably be ideal
> and reusable for other projects.

That's a nice idea!

Jan.

-- 
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond - The music typesetter
http://www.xs4all.nl/~jantien       | http://www.lilypond.org