Multiple -M options for git-cvsimport

Multiple -M options for git-cvsimport

[Philippe Bruhat (BooK)]

Sending again my series of patches to git-cvsimport, which allow to
use several -M options for giving the regular expressions capturing the
source branch name when merging.

Signed-off-by: Philippe Bruhat (BooK) <book@cpan.org>

[PATCH] cvsimport: have default merge regex allow for dashes in the branch name

[Philippe Bruhat (BooK)]

From: Philippe Bruhat (BooK <book@cpan.org>

The default value of @mergerx uses \w, which matches word
character; a branch name like policy-20050608-br will not be
matched.
---
 git-cvsimport.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 9516242..3d013a7 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -164,7 +164,7 @@ if ($#ARGV == 0) {
 
 our @mergerx = ();
 if ($opt_m) {
-	@mergerx = ( qr/\b(?:from|of|merge|merging|merged) (\w+)/i );
+	@mergerx = ( qr/\b(?:from|of|merge|merging|merged) ([-\w]+)/i );
 }
 if ($opt_M) {
 	push (@mergerx, qr/$opt_M/);
-- 
1.5.4.2.187.gfc276

[PATCH] cvsimport: allow for multiple -M options

[Philippe Bruhat (BooK)]

From: Philippe Bruhat (BooK <book@cpan.org>

Use Getopt::Long instead of Getopt::Std to handle multiple -M options,
for all the cases when having a single custom regex is not enough.

For example, "merged (\w+)" and "(\w+) merged" can't be easily turned
into a single regular expression capturing the branch name in $1.
---
 git-cvsimport.perl |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 3d013a7..f138a01 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -15,7 +15,7 @@
 
 use strict;
 use warnings;
-use Getopt::Std;
+use Getopt::Long;
 use File::Spec;
 use File::Temp qw(tempfile tmpnam);
 use File::Path qw(mkpath);
@@ -29,7 +29,7 @@ use IPC::Open2;
 $SIG{'PIPE'}="IGNORE";
 $ENV{'TZ'}="UTC";
 
-our ($opt_h,$opt_o,$opt_v,$opt_k,$opt_u,$opt_d,$opt_p,$opt_C,$opt_z,$opt_i,$opt_P, $opt_s,$opt_m,$opt_M,$opt_A,$opt_S,$opt_L, $opt_a, $opt_r);
+our ($opt_h,$opt_o,$opt_v,$opt_k,$opt_u,$opt_d,$opt_p,$opt_C,$opt_z,$opt_i,$opt_P, $opt_s,$opt_m,@opt_M,$opt_A,$opt_S,$opt_L, $opt_a, $opt_r);
 my (%conv_author_name, %conv_author_email);
 
 sub usage(;$) {
@@ -112,7 +112,12 @@ sub read_repo_config {
 
 my $opts = "haivmkuo:d:p:r:C:z:s:M:P:A:S:L:";
 read_repo_config($opts);
-getopts($opts) or usage();
+Getopt::Long::Configure( 'no_ignore_case' );
+
+# turn the Getopt::Std specification in a Getopt::Long one,
+# with support for multiple -M options
+GetOptions( map { s/:/=s/; /M/ ? "$_\@" : $_ } split( /(?!:)/, $opts ) )
+    or usage();
 usage if $opt_h;
 
 if (@ARGV == 0) {
@@ -166,8 +171,8 @@ our @mergerx = ();
 if ($opt_m) {
 	@mergerx = ( qr/\b(?:from|of|merge|merging|merged) ([-\w]+)/i );
 }
-if ($opt_M) {
-	push (@mergerx, qr/$opt_M/);
+if (@opt_M) {
+	push (@mergerx, map { qr/$_/ } @opt_M);
 }
 
 # Remember UTC of our starting time
-- 
1.5.4.2.187.gfc276

[PATCH] cvsimport: document that -M can be used multiple times

[Philippe Bruhat (BooK)]

From: Philippe Bruhat (BooK <book@cpan.org>

Also document the capture behaviour (source branch name in $1)

Signed-off-by: Philippe Bruhat (BooK) <book@cpan.org>
---
 Documentation/git-cvsimport.txt |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-cvsimport.txt b/Documentation/git-cvsimport.txt
index 6f91b9e..58eefd4 100644
--- a/Documentation/git-cvsimport.txt
+++ b/Documentation/git-cvsimport.txt
@@ -102,13 +102,17 @@ If you need to pass multiple options, separate them with a comma.
 
 -m::
 	Attempt to detect merges based on the commit message. This option
-	will enable default regexes that try to capture the name source
+	will enable default regexes that try to capture the source
 	branch name from the commit message.
 
 -M <regex>::
 	Attempt to detect merges based on the commit message with a custom
 	regex. It can be used with '-m' to enable the default regexes
 	as well. You must escape forward slashes.
++
+The regex must capture the source branch name in $1.
++
+This option can be used several times to provide several detection regexes.
 
 -S <regex>::
 	Skip paths matching the regex.
-- 
1.5.4.2.187.gfc276

[PATCH] cvsimport: configure Getopt::Long to bundle options

[Philippe Bruhat (BooK)]

---
 git-cvsimport.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index f138a01..47f116f 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -112,7 +112,7 @@ sub read_repo_config {
 
 my $opts = "haivmkuo:d:p:r:C:z:s:M:P:A:S:L:";
 read_repo_config($opts);
-Getopt::Long::Configure( 'no_ignore_case' );
+Getopt::Long::Configure( 'no_ignore_case', 'bundling' );
 
 # turn the Getopt::Std specification in a Getopt::Long one,
 # with support for multiple -M options
-- 
1.5.4.2.187.gfc276

Re: Multiple -M options for git-cvsimport

[Junio C Hamano]

"Philippe Bruhat (BooK)" <book@cpan.org> writes:

> Sending again my series of patches to git-cvsimport, which allow to
> use several -M options for giving the regular expressions capturing the
> source branch name when merging.

Could you be a bit more explicit than "Sending again", describe
if it is just a straight resend, or what problems were pointed
out in the earlier round (if any) and how they were addressed
(or ignored, if any)?

Also please Sign-off all your patches.  Cover letters do not
need one.

I'll take a look at them later, when I have enough time to fish
for messages and discussions from earlier round in the list
archive in order to process this.

Thanks.

Re: Multiple -M options for git-cvsimport

[Philippe Bruhat (BooK)]

On Thu, Feb 28, 2008 at 12:07:46PM -0800, Junio C Hamano wrote:
> "Philippe Bruhat (BooK)" <book@cpan.org> writes:
> 
> > Sending again my series of patches to git-cvsimport, which allow to
> > use several -M options for giving the regular expressions capturing the
> > source branch name when merging.
> 
> Could you be a bit more explicit than "Sending again", describe
> if it is just a straight resend, or what problems were pointed
> out in the earlier round (if any) and how they were addressed
> (or ignored, if any)?

Sorry.

In the previous round, an asciidoc formatting error was pointed in my
doc patch, and you also noted that the t/t9600-cvsimport.sh test script
failed after my Getopt::Std -> Getopt::Long patch.

I corrected all those problems (checked the asciidoc HTML output, and
made sure the test script passed again).

> Also please Sign-off all your patches.  Cover letters do not
> need one.

It noticed that I forgot to -s my commits, so I thought that signing off
the cover letter would be equivalent. I tried to follow the guidelines
for sending patches, using format-patch and send-email, but I'm still a
newbie both to git and it's development model.

> I'll take a look at them later, when I have enough time to fish
> for messages and discussions from earlier round in the list
> archive in order to process this.

You can ignore my previous messages, then. These four patches were
rebased on the top of master, and correct the problems that were
previously pointed to me.

    Thanks,

-- 
 Philippe Bruhat (BooK)

 The shortest distance between two points is not always the safest.
                                    (Moral from Groo The Wanderer #69 (Epic))

Re: Multiple -M options for git-cvsimport

[Junio C Hamano]

"Philippe Bruhat (BooK)" <book@cpan.org> writes:

> On Thu, Feb 28, 2008 at 12:07:46PM -0800, Junio C Hamano wrote:
>
>> I'll take a look at them later, when I have enough time to fish
>> for messages and discussions from earlier round in the list
>> archive in order to process this.
>
> You can ignore my previous messages, then. These four patches were
> rebased on the top of master, and correct the problems that were
> previously pointed to me.

I do not work that way.  I am a trust-but-verify kind of person.

So I dug up the old ones and the discussion.  The series looks
fine.

cvsimport: have default merge regex allow for dashes in the branch name

  You can extend the default with -M anyway, but I guess the
  default pattern can be loosened like this without increasing
  the risk of false hits, so probably it is Ok.

cvsimport: allow for multiple -M options

  Ok.

cvsimport: document that -M can be used multiple times

  Ok, except that "can be used seveval times" should probably be
  "can be used more than once".

cvsimport: configure Getopt::Long to bundle options

  Ok.  Why isn't bundling the default, I have to wonder...

Re: Multiple -M options for git-cvsimport

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> "Philippe Bruhat (BooK)" <book@cpan.org> writes:
>
>> On Thu, Feb 28, 2008 at 12:07:46PM -0800, Junio C Hamano wrote:
>>
>>> I'll take a look at them later, when I have enough time to fish
>>> for messages and discussions from earlier round in the list
>>> archive in order to process this.
>>
>> You can ignore my previous messages, then. These four patches were
>> rebased on the top of master, and correct the problems that were
>> previously pointed to me.
>
> I do not work that way.  I am a trust-but-verify kind of person.
>
> So I dug up the old ones and the discussion.  The series looks
> fine.
> ...
> cvsimport: have default merge regex allow for dashes in the branch name
> ...
> cvsimport: allow for multiple -M options
>
>   Ok.
> ...
> cvsimport: document that -M can be used multiple times
> cvsimport: configure Getopt::Long to bundle options
>
>   Ok.  Why isn't bundling the default, I have to wonder...

I take these back.  

The last one is an "Oops, the second one was an utter crap, it does not
even pass the testsuite, well let's patch it up with an extra commit at
the end."

Please don't do this.  We do not have to record earlier mistakes in the
public history.

I've squashed 4/4 into 2/4 and made it a 3-patch series.