threads / patch / 12361
Subject: [PATCH] cvsimport: have default merge regex allow for dashes in the branch name
Philippe Bruhat (BooK)Multiple -M options for git-cvsimport
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>
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
Philippe Bruhat (BooK)[PATCH] cvsimport: allow for multiple -M options
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
Philippe Bruhat (BooK)[PATCH] cvsimport: document that -M can be used multiple times
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
Philippe Bruhat (BooK)[PATCH] cvsimport: configure Getopt::Long to bundle options
--- 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
Junio C HamanoRe: Multiple -M options for git-cvsimport
"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.
Philippe Bruhat (BooK)Re: Multiple -M options for git-cvsimport
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))Junio C HamanoRe: Multiple -M options for git-cvsimport
"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...
Junio C HamanoRe: Multiple -M options for git-cvsimport
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.