threads / patch / 5690

patchsvnimport add support for parsing From lines for author

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

## tl;dr

6 messages between Sep 25, 2006 and Sep 28, 2006. Diffs are folded; open one to read it.

replies: 5people: 2as markdown or json

Andy Whitcroft· Sep 25, 2006, 11:08 UTC · lore
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>
---
Show changes to git-svnimport.perl +8 −4
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;
Junio C Hamano· Sep 26, 2006, 05:08 UTC · re: Andy Whitcroft · lore

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

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.

Andy Whitcroft· Sep 26, 2006, 07:51 UTC · re: Junio C Hamano · lore

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

Junio C Hamano wrote:
Show 6 quoted lines
> 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.
Show 8 quoted lines
>> 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.

Show 5 quoted lines
> 
>> 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
Junio C Hamano· Sep 27, 2006, 05:34 UTC · re: Andy Whitcroft · lore

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

Andy Whitcroft <apw@shadowen.org> writes:
Show 14 quoted lines
>>> 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.
Show 14 quoted lines
>> 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.

Andy Whitcroft· Sep 28, 2006, 10:58 UTC · re: Junio C Hamano · lore

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

Junio C Hamano wrote:
Show 37 quoted lines
> 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
Junio C Hamano· Sep 28, 2006, 16:10 UTC · re: Andy Whitcroft · lore

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

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.

← back to recent threads