[PATCH] svnimport add support for parsing From lines for author

[PATCH] svnimport add support for parsing From lines for author

[Andy Whitcroft]

svnimport: add support for parsing From: lines for author

When commiting a non-signed off contribution you cannot just add
a Signed-off-by: from the author as they did not sign it off.
But if you then commit it, and necessarily sign it off yourself,
the change appears to be yours.  In this case it is common to use
the following form:

	Commentry

	From: originator <email>
	Signed-of-by: me <my email>

Now that we have support for parsing Signed-off-by: for author
information it makes sense to handle From: as well.  This patch
adds a new -F which will handle From: lines in the comments.  It
may be used in combination with -S.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
diff --git a/git-svnimport.perl b/git-svnimport.perl
index ed62897..988514e 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_S);
+    $opt_b,$opt_r,$opt_I,$opt_A,$opt_s,$opt_l,$opt_d,$opt_D,$opt_S,$opt_F);
 
 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] [-S] [SVN_URL]
+       [-m] [-M regex] [-A author_file] [-S] [-F] [SVN_URL]
 END
 	exit(1);
 }
 
-getopts("A:b:C:dDhiI:l:mM:o:rs:t:T:Suv") or usage();
+getopts("A:b:C:dDFhiI:l:mM:o:rs:t:T:Suv") or usage();
 usage if $opt_h;
 
 my $tag_name = $opt_t || "tags";
@@ -548,8 +548,12 @@ sub commit {
 		$committer_name = $committer_email = $author;
 	}
 
-	if ($opt_S && $message =~ /Signed-off-by:\s+(.*?)\s+<(.*)>\s*\n/) {
+	if ($opt_F && $message =~ /From:\s+(.*?)\s+<(.*)>\s*\n/) {
 		($author_name, $author_email) = ($1, $2);
+		print "Author from From: $1 <$2>\n" if ($opt_v);;
+	} elsif ($opt_S && $message =~ /Signed-off-by:\s+(.*?)\s+<(.*)>\s*\n/) {
+		($author_name, $author_email) = ($1, $2);
+		print "Author from Signed-off-by: $1 <$2>\n" if ($opt_v);;
 	} else {
 		$author_name = $committer_name;
 		$author_email = $committer_email;

Re: [PATCH] svnimport add support for parsing From lines for author

[Junio C Hamano]

Andy Whitcroft <apw@shadowen.org> writes:

> svnimport: add support for parsing From: lines for author

Please do _not_ repeat the summary line in the message body.  It
is on the Subject: already.

> Now that we have support for parsing Signed-off-by: for author
> information it makes sense to handle From: as well.

I take that you are referring to Sasha's change in ae35b304; I
asked for actual svn users for ACK/NACK but I did not hear any.
Can I understand that you use svnimport for real projects and
are happy with Sasha's change? --- that would be an ack that
would help me sleep better ;-).

> adds a new -F which will handle From: lines in the comments.  It
> may be used in combination with -S.

This sort of makes sense but how common is this?

I also wonder instead of piling up custom flags if it is better
to let match-and-extract pattern be specified from the command
line.

Re: [PATCH] svnimport add support for parsing From lines for author

[Andy Whitcroft]

Junio C Hamano wrote:
> Andy Whitcroft <apw@shadowen.org> writes:
> 
>> svnimport: add support for parsing From: lines for author
> 
> Please do _not_ repeat the summary line in the message body.  It
> is on the Subject: already.

Sorry ... different projects different rules.

>> Now that we have support for parsing Signed-off-by: for author
>> information it makes sense to handle From: as well.
> 
> I take that you are referring to Sasha's change in ae35b304; I
> asked for actual svn users for ACK/NACK but I did not hear any.
> Can I understand that you use svnimport for real projects and
> are happy with Sasha's change? --- that would be an ack that
> would help me sleep better ;-).

Heh.  Yeah I am tracking a small SVN repository which is using the
kernel DCO.  we have From:/S-o-b: much as akpm uses in -mm.  This was
the result of seeing that change and wanting to see if it would pick up
our sign-offs.  It only seemed deficient in From: handling :).  It seems
to work well in practice for me.

> 
>> adds a new -F which will handle From: lines in the comments.  It
>> may be used in combination with -S.
> 
> This sort of makes sense but how common is this?

I think the kernel DCO has started a trend which will expand to other
projects.  The problem it is intended to fix is a general one which
needs addressing in all such projects.

> I also wonder instead of piling up custom flags if it is better
> to let match-and-extract pattern be specified from the command
> line.

I did look at reusing the -S flag, such that -S would be S-o-b: handling
and -SS would be S-o-b: and From:, but this script is currently using
the old getopt implementation which doesn't record repeats.

So you're proposing something more like:

    git svn-import -S "Signed-off-by:" -S "From:" ...

Again, we'll have to update the options handling to get that kind of
behaviour.  How would you feel about -SS in this context.

-apw

Re: [PATCH] svnimport add support for parsing From lines for author

[Junio C Hamano]

Andy Whitcroft <apw@shadowen.org> writes:

>>> Now that we have support for parsing Signed-off-by: for author
>>> information it makes sense to handle From: as well.
>> 
>> I take that you are referring to Sasha's change in ae35b304; I
>> asked for actual svn users for ACK/NACK but I did not hear any.
>> Can I understand that you use svnimport for real projects and
>> are happy with Sasha's change? --- that would be an ack that
>> would help me sleep better ;-).
>
> Heh.  Yeah I am tracking a small SVN repository which is using the
> kernel DCO.  we have From:/S-o-b: much as akpm uses in -mm.  This was
> the result of seeing that change and wanting to see if it would pick up
> our sign-offs.  It only seemed deficient in From: handling :).  It seems
> to work well in practice for me.

Thanks.

>> I also wonder instead of piling up custom flags if it is better
>> to let match-and-extract pattern be specified from the command
>> line.
>
> I did look at reusing the -S flag, such that -S would be S-o-b: handling
> and -SS would be S-o-b: and From:, but this script is currently using
> the old getopt implementation which doesn't record repeats.
>
> So you're proposing something more like:
>
>     git svn-import -S "Signed-off-by:" -S "From:" ...
>
> Again, we'll have to update the options handling to get that kind of
> behaviour.  How would you feel about -SS in this context.

It was more of an idle speculation than a serious proposal.  I
do not think there are too many different ways to record the
authorship information, so having just two hardwired patterns -F
and -S would be sufficient.  If there were, then string of -S
options that specify the header-looking strings or match
patterns would have made more sense.

Re: [PATCH] svnimport add support for parsing From lines for author

[Andy Whitcroft]

Junio C Hamano wrote:
> Andy Whitcroft <apw@shadowen.org> writes:
> 
>>>> Now that we have support for parsing Signed-off-by: for author
>>>> information it makes sense to handle From: as well.
>>> I take that you are referring to Sasha's change in ae35b304; I
>>> asked for actual svn users for ACK/NACK but I did not hear any.
>>> Can I understand that you use svnimport for real projects and
>>> are happy with Sasha's change? --- that would be an ack that
>>> would help me sleep better ;-).
>> Heh.  Yeah I am tracking a small SVN repository which is using the
>> kernel DCO.  we have From:/S-o-b: much as akpm uses in -mm.  This was
>> the result of seeing that change and wanting to see if it would pick up
>> our sign-offs.  It only seemed deficient in From: handling :).  It seems
>> to work well in practice for me.
> 
> Thanks.
> 
>>> I also wonder instead of piling up custom flags if it is better
>>> to let match-and-extract pattern be specified from the command
>>> line.
>> I did look at reusing the -S flag, such that -S would be S-o-b: handling
>> and -SS would be S-o-b: and From:, but this script is currently using
>> the old getopt implementation which doesn't record repeats.
>>
>> So you're proposing something more like:
>>
>>     git svn-import -S "Signed-off-by:" -S "From:" ...
>>
>> Again, we'll have to update the options handling to get that kind of
>> behaviour.  How would you feel about -SS in this context.
> 
> It was more of an idle speculation than a serious proposal.  I
> do not think there are too many different ways to record the
> authorship information, so having just two hardwired patterns -F
> and -S would be sufficient.  If there were, then string of -S
> options that specify the header-looking strings or match
> patterns would have made more sense.

This one didn't make it onto the git.git whats next page.  Not sure of
protocol in these matters, so I'll just ask.  Has this been rejected or
forgotten?

:)

-apw

Re: [PATCH] svnimport add support for parsing From lines for author

[Junio C Hamano]

Andy Whitcroft <apw@shadowen.org> writes:

> This one didn't make it onto the git.git whats next page.  Not sure of
> protocol in these matters, so I'll just ask.  Has this been rejected or
> forgotten?

Asking is always good.  It was forgotten.