[PATCH] git-svnimport: Parse log message for Signed-off-by: lines

[PATCH] git-svnimport: Parse log message for Signed-off-by: lines

[Sasha Khapyorsky]

Hi,

This feature was useful with importing https://openib.org/svn/gen2 .

Sasha

This add '-S' option. When specified svn-import will try to parse
commit message for 'Signed-off-by: ...' line, and if found will use
the name and email address extracted at first occurrence as this commit
author name and author email address. Committer name and email are
extracted in usual way.

Signed-off-by: Sasha Khapyorsky <sashak@voltaire.com>
---
 git-svnimport.perl |   31 ++++++++++++++++++++-----------
 1 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/git-svnimport.perl b/git-svnimport.perl
index 26dc454..7113cf5 100755
--- a/git-svnimport.perl
+++ b/git-svnimport.perl
@@ -31,7 +31,7 @@ die "Need SVN:Core 1.2.1 or better" if $
 $ENV{'TZ'}="UTC";
 
 our($opt_h,$opt_o,$opt_v,$opt_u,$opt_C,$opt_i,$opt_m,$opt_M,$opt_t,$opt_T,
-    $opt_b,$opt_r,$opt_I,$opt_A,$opt_s,$opt_l,$opt_d,$opt_D);
+    $opt_b,$opt_r,$opt_I,$opt_A,$opt_s,$opt_l,$opt_d,$opt_D,$opt_S);
 
 sub usage() {
 	print STDERR <<END;
@@ -39,12 +39,12 @@ Usage: ${\basename $0}     # fetch/updat
        [-o branch-for-HEAD] [-h] [-v] [-l max_rev]
        [-C GIT_repository] [-t tagname] [-T trunkname] [-b branchname]
        [-d|-D] [-i] [-u] [-r] [-I ignorefilename] [-s start_chg]
-       [-m] [-M regex] [-A author_file] [SVN_URL]
+       [-m] [-M regex] [-A author_file] [-S] [SVN_URL]
 END
 	exit(1);
 }
 
-getopts("A:b:C:dDhiI:l:mM:o:rs:t:T:uv") or usage();
+getopts("A:b:C:dDhiI:l:mM:o:rs:t:T:Suv") or usage();
 usage if $opt_h;
 
 my $tag_name = $opt_t || "tags";
@@ -531,21 +531,30 @@ sub copy_path($$$$$$$$) {
 
 sub commit {
 	my($branch, $changed_paths, $revision, $author, $date, $message) = @_;
-	my($author_name,$author_email,$dest);
+	my($committer_name,$committer_email,$dest);
+	my($author_name,$author_email);
 	my(@old,@new,@parents);
 
 	if (not defined $author or $author eq "") {
-		$author_name = $author_email = "unknown";
+		$committer_name = $committer_email = "unknown";
 	} elsif (defined $users_file) {
 		die "User $author is not listed in $users_file\n"
 		    unless exists $users{$author};
-		($author_name,$author_email) = @{$users{$author}};
+		($committer_name,$committer_email) = @{$users{$author}};
 	} elsif ($author =~ /^(.*?)\s+<(.*)>$/) {
-		($author_name, $author_email) = ($1, $2);
+		($committer_name, $committer_email) = ($1, $2);
 	} else {
 		$author =~ s/^<(.*)>$/$1/;
-		$author_name = $author_email = $author;
+		$committer_name = $committer_email = $author;
 	}
+
+	if ($opt_S && $message =~ /Signed-off-by:\s+(.*?)\s+<(.*)>\s*\n/) {
+        	($author_name, $author_email) = ($1, $2);
+	} else {
+		$author_name = $committer_name;
+		$author_email = $committer_email;
+	}
+
 	$date = pdate($date);
 
 	my $tag;
@@ -772,8 +781,8 @@ #	}
 				"GIT_AUTHOR_NAME=$author_name",
 				"GIT_AUTHOR_EMAIL=$author_email",
 				"GIT_AUTHOR_DATE=".strftime("+0000 %Y-%m-%d %H:%M:%S",gmtime($date)),
-				"GIT_COMMITTER_NAME=$author_name",
-				"GIT_COMMITTER_EMAIL=$author_email",
+				"GIT_COMMITTER_NAME=$committer_name",
+				"GIT_COMMITTER_EMAIL=$committer_email",
 				"GIT_COMMITTER_DATE=".strftime("+0000 %Y-%m-%d %H:%M:%S",gmtime($date)),
 				"git-commit-tree", $tree,@par);
 			die "Cannot exec git-commit-tree: $!\n";
@@ -825,7 +834,7 @@ #	}
 		print $out ("object $cid\n".
 		    "type commit\n".
 		    "tag $dest\n".
-		    "tagger $author_name <$author_email>\n") and
+		    "tagger $committer_name <$committer_email>\n") and
 		close($out)
 		    or die "Cannot create tag object $dest: $!\n";
 
-- 
1.4.2

Re: [PATCH] git-svnimport: Parse log message for Signed-off-by: lines

[Junio C Hamano]

Sasha Khapyorsky <sashak@voltaire.com> writes:

> Hi,
>
> This feature was useful with importing https://openib.org/svn/gen2 .
>
> Sasha
>
> This add '-S' option. When specified svn-import will try to parse
> commit message for 'Signed-off-by: ...' line, and if found will use
> the name and email address extracted at first occurrence as this commit
> author name and author email address. Committer name and email are
> extracted in usual way.
>
> Signed-off-by: Sasha Khapyorsky <sashak@voltaire.com>

Thanks.

I do not think the first signed-off-by is necessarily the author
of the change, so we are risking miscrediting (or misblaming) a
wrong person.  Having said that, using the committer information
has the same miscredit problem, so this change might be Ok, but
I am not sure if it adds much improvement.

I will wait for an ack/nack from somebody who use svnimport and
know it well.

BTW, I do not appreciate the first six lines of your message
being _before_ the proposed commit log message.  Please have it
between "---\n" (that comes immediately after your own
"Signed-off-by:") and the diffstat, like this:

        This add '-S' option. When specified svn-import will try to...
        ... in usual way.

        Signed-off-by: Sasha Khapyorsky <sashak@voltaire.com>
        ---

          Hi,

          This feature was useful with importing https://openib.org/svn/gen2 .

          Sasha

         git-svnimport.perl |   31 ++++++++++++++++++++-----------
         1 files changed, 20 insertions(+), 11 deletions(-)

         diff --git a/...

Re: [PATCH] git-svnimport: Parse log message for Signed-off-by: lines

[Sasha Khapyorsky]

On 14:26 Tue 05 Sep     , Junio C Hamano wrote:
> Sasha Khapyorsky <sashak@voltaire.com> writes:
> 
> > Hi,
> >
> > This feature was useful with importing https://openib.org/svn/gen2 .
> >
> > Sasha
> >
> > This add '-S' option. When specified svn-import will try to parse
> > commit message for 'Signed-off-by: ...' line, and if found will use
> > the name and email address extracted at first occurrence as this commit
> > author name and author email address. Committer name and email are
> > extracted in usual way.
> >
> > Signed-off-by: Sasha Khapyorsky <sashak@voltaire.com>
> 
> Thanks.
> 
> I do not think the first signed-off-by is necessarily the author
> of the change, so we are risking miscrediting (or misblaming) a
> wrong person. 

Right, there is such risk, so this feature is optional and by default is
"off".

> Having said that, using the committer information
> has the same miscredit problem, so this change might be Ok, but
> I am not sure if it adds much improvement.
> 
> I will wait for an ack/nack from somebody who use svnimport and
> know it well.

Sure.

BTW, what about to importing subdirectories, like this:

 <trunk>/path/to/subdir
 <branches>/path/to/subdir

Is this could be improvement?

> 
> BTW, I do not appreciate the first six lines of your message
> being _before_ the proposed commit log message.  Please have it
> between "---\n" (that comes immediately after your own
> "Signed-off-by:") and the diffstat, like this:
> 
>         This add '-S' option. When specified svn-import will try to...
>         ... in usual way.
> 
>         Signed-off-by: Sasha Khapyorsky <sashak@voltaire.com>
>         ---
> 
>           Hi,
> 
>           This feature was useful with importing https://openib.org/svn/gen2 .
> 
>           Sasha
> 
>          git-svnimport.perl |   31 ++++++++++++++++++++-----------
>          1 files changed, 20 insertions(+), 11 deletions(-)
> 
>          diff --git a/...

Ok.

Sasha

Re: [PATCH] git-svnimport: Parse log message for Signed-off-by: lines

[Junio C Hamano]

Sasha Khapyorsky <sashak@voltaire.com> writes:

> BTW, what about to importing subdirectories, like this:
>
>  <trunk>/path/to/subdir
>  <branches>/path/to/subdir
>
> Is this could be improvement?

I somehow had an impression that svnimport dealt with the
reversed layout already, although $project/{trunk,branches,tags}
layout is assumed by default; maybe I was mistaken.

If the tool can automatically detect the layout the remote
project employs, and adjust the default accordingly, I would
imagine that would be a useful addition.

Re: [PATCH] git-svnimport: Parse log message for Signed-off-by: lines

[Sasha Khapyorsky]

On 16:26 Tue 05 Sep     , Junio C Hamano wrote:
> Sasha Khapyorsky <sashak@voltaire.com> writes:
> 
> > BTW, what about to importing subdirectories, like this:
> >
> >  <trunk>/path/to/subdir
> >  <branches>/path/to/subdir
> >
> > Is this could be improvement?
> 
> I somehow had an impression that svnimport dealt with the
> reversed layout already, although $project/{trunk,branches,tags}
> layout is assumed by default; maybe I was mistaken.

At least I didn't succeed with reversed layout. With option
-T <trunk>/$project import works but only for trunk branch, attempts
to specify branch as -b <branches> or -b <branches>/$project don't help,
the same is with tags.

> If the tool can automatically detect the layout the remote
> project employs, and adjust the default accordingly, I would
> imagine that would be a useful addition.

Not really automatically, but $project may be passed as option argument
in command line (let's say -p or -P), then git-svnimport will attempt to
parse such SVN repo layout:

  <trunk>/$project
  <branches>/<names>/$project
  ...

I will verify the patch (it worked only once with our specific project -
OpenSM) and then will post.

Sasha

Re: [PATCH] git-svnimport: Parse log message for Signed-off-by: lines

[Matthias Urlichs]

Hi,

Sasha Khapyorsky:
> At least I didn't succeed with reversed layout. With option
> -T <trunk>/$project import works but only for trunk branch, attempts
> to specify branch as -b <branches> or -b <branches>/$project don't help,
> the same is with tags.
> 
That's true. The problem is that it wants the tag or branch name as the
last component of the path.

A more generic solution would be to use wildcards in the branch/tag
specification, to allow more than one wildcard, and to be able to
specify the exact form of the branch or tag name on the git side.

All of this should be specified in the repository's git config file,
not on the command line.


Somebody who wants to implement that is certainly invited to do so.
(I don't have time for that at the moment, unfortunately.)

-- 
Matthias Urlichs   |   {M:U} IT Design @ m-u-it.de   |  smurf@smurf.noris.de

partial svnimport [was: Re: [PATCH] git-svnimport: Parse log message for Signed-off-by: lines]

[Sasha Khapyorsky]

Hi,

On 17:33 Wed 06 Sep     , Matthias Urlichs wrote:
> Hi,
> 
> Sasha Khapyorsky:
> > At least I didn't succeed with reversed layout. With option
> > -T <trunk>/$project import works but only for trunk branch, attempts
> > to specify branch as -b <branches> or -b <branches>/$project don't help,
> > the same is with tags.
> > 
> That's true. The problem is that it wants the tag or branch name as the
> last component of the path.
> 
> A more generic solution would be to use wildcards in the branch/tag
> specification, to allow more than one wildcard, and to be able to
> specify the exact form of the branch or tag name on the git side.

Or perhaps just to specify path of the project (or sub-project) after
trunk/branches*/ . Like in this patch:


diff --git a/git-svnimport.perl b/git-svnimport.perl
index 988514e..86a6e8a 100755
--- a/git-svnimport.perl
+++ b/git-svnimport.perl
@@ -31,7 +31,8 @@ die "Need SVN:Core 1.2.1 or better" if $
 $ENV{'TZ'}="UTC";
 
 our($opt_h,$opt_o,$opt_v,$opt_u,$opt_C,$opt_i,$opt_m,$opt_M,$opt_t,$opt_T,
-    $opt_b,$opt_r,$opt_I,$opt_A,$opt_s,$opt_l,$opt_d,$opt_D,$opt_S,$opt_F);
+    $opt_b,$opt_r,$opt_I,$opt_A,$opt_s,$opt_l,$opt_d,$opt_D,$opt_S,$opt_F,
+    $opt_P);
 
 sub usage() {
 	print STDERR <<END;
@@ -39,17 +40,19 @@ Usage: ${\basename $0}     # fetch/updat
        [-o branch-for-HEAD] [-h] [-v] [-l max_rev]
        [-C GIT_repository] [-t tagname] [-T trunkname] [-b branchname]
        [-d|-D] [-i] [-u] [-r] [-I ignorefilename] [-s start_chg]
-       [-m] [-M regex] [-A author_file] [-S] [-F] [SVN_URL]
+       [-m] [-M regex] [-A author_file] [-S] [-F] [-P project] [SVN_URL]
 END
 	exit(1);
 }
 
-getopts("A:b:C:dDFhiI:l:mM:o:rs:t:T:Suv") or usage();
+getopts("A:b:C:dDFhiI:l:mM:o:rs:t:T:SP:uv") or usage();
 usage if $opt_h;
 
 my $tag_name = $opt_t || "tags";
 my $trunk_name = $opt_T || "trunk";
 my $branch_name = $opt_b || "branches";
+my $project_name = $opt_P || "";
+$project_name = "/" . $project_name if ($project_name);
 
 @ARGV == 1 or @ARGV == 2 or usage();
 
@@ -356,11 +359,11 @@ sub revert_split_path($$) {
 	my $svnpath;
 	$path = "" if $path eq "/"; # this should not happen, but ...
 	if($branch eq "/") {
-		$svnpath = "$trunk_name/$path";
+		$svnpath = "$trunk_name$project_name/$path";
 	} elsif($branch =~ m#^/#) {
-		$svnpath = "$tag_name$branch/$path";
+		$svnpath = "$tag_name$branch$project_name/$path";
 	} else {
-		$svnpath = "$branch_name/$branch/$path";
+		$svnpath = "$branch_name/$branch$project_name/$path";
 	}
 
 	$svnpath =~ s#/+$##;
@@ -864,6 +867,20 @@ #	}
 	print "DONE: $revision $dest $cid\n" if $opt_v;
 }
 
+sub project_path($$)
+{
+	my ($path, $project) = @_;
+
+	$path = "/".$path unless ($path =~ m#^\/#) ;
+	return $1 if ($path =~ m#^$project\/(.*)$#);
+
+	$path =~ s#\.#\\\.#g;
+	$path =~ s#\+#\\\+#g;
+	return "/" if ($project =~ m#^$path.*$#);
+
+	return undef;
+}
+
 sub commit_all {
 	# Recursive use of the SVN connection does not work
 	local $svn = $svn2;
@@ -883,6 +900,10 @@ sub commit_all {
 	while(my($path,$action) = each %$changed_paths) {
 		($branch,$path) = split_path($revision,$path);
 		next if not defined $branch;
+		if ($project_name) {
+			$path = project_path($path, $project_name);
+			next if not defined $path;
+		}
 		$done{$branch}{$path} = $action;
 	}
 	while(($branch,$changed_paths) = each %done) {


(will submit if above is acceptable)

> All of this should be specified in the repository's git config file,
> not on the command line.

It is for incremental imports? So we will not need to reproduce exact
command line for each git-svnimport run? Good idea.

Sasha