Teach Makefile.PL to find .pm files on its own

Teach Makefile.PL to find .pm files on its own

[Michael G. Schwern]

This makes it so you no longer must edit the Makefile.PL every time you
add, rename or delete a Perl module.  This is convenient, and I'm about
to extract a bunch of .pm files out of git-svn.

You still have to edit the Makefile. That parallel build system should be
able to be removed at a later date and replaced with the right Makefile.PL
flags.

Patch 1 and 2 are just things I noticed in the Makefile.PL along the way.
Patch 3 is the meat.  It doesn't depend on 1 & 2 but I figured it would
be silly to send them separately.

[PATCH 1/3] Quiet warning if Makefile.PL is run with -w and no --localedir

[Michael G. Schwern]

From: "Michael G. Schwern" <schwern@pobox.com>

Usually it isn't, but its nice if it can be run with warnings on.

Signed-off-by: Michael G Schwern <schwern@pobox.com>
---
 perl/Makefile.PL | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/perl/Makefile.PL b/perl/Makefile.PL
index b54b04a..87e1f62 100644
--- a/perl/Makefile.PL
+++ b/perl/Makefile.PL
@@ -6,7 +6,8 @@ use Getopt::Long;
 # Sanity: die at first unknown option
 Getopt::Long::Configure qw/ pass_through /;
 
-GetOptions("localedir=s" => \my $localedir);
+my $localedir = '';
+GetOptions("localedir=s" => \$localedir);
 
 sub MY::postamble {
 	return <<'MAKE_FRAG';
-- 
1.7.11.1

[PATCH 2/3] Don't lose Error.pm if $@ gets clobbered.

[Michael G. Schwern]

From: "Michael G. Schwern" <schwern@pobox.com>

In older Perls, sometimes $@ can become unset between the eval and
checking $@.  Its safer to check the eval directly.

Signed-off-by: Michael G Schwern <schwern@pobox.com>
---
 perl/Makefile.PL | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/perl/Makefile.PL b/perl/Makefile.PL
index 87e1f62..887fa1b 100644
--- a/perl/Makefile.PL
+++ b/perl/Makefile.PL
@@ -41,8 +41,7 @@ my %pm = (
 
 # We come with our own bundled Error.pm. It's not in the set of default
 # Perl modules so install it if it's not available on the system yet.
-eval { require Error };
-if ($@ || $Error::VERSION < 0.15009) {
+if ( !eval { require Error } || $Error::VERSION < 0.15009) {
 	$pm{'private-Error.pm'} = '$(INST_LIBDIR)/Error.pm';
 }
 
-- 
1.7.11.1

[PATCH 3/3] The Makefile.PL will now find .pm files itself.

[Michael G. Schwern]

From: "Michael G. Schwern" <schwern@pobox.com>

It is no longer necessary to manually add new .pm files to the
Makefile.PL.  This makes it easier to add modules.

It is still necessary to add them to the Makefile, but that extra work
should be removed at a future date.

Signed-off-by: Michael G Schwern <schwern@pobox.com>
---
 perl/Makefile.PL | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/perl/Makefile.PL b/perl/Makefile.PL
index 887fa1b..3f29ba9 100644
--- a/perl/Makefile.PL
+++ b/perl/Makefile.PL
@@ -2,6 +2,10 @@ use strict;
 use warnings;
 use ExtUtils::MakeMaker;
 use Getopt::Long;
+use File::Find;
+
+# Don't forget to update the perl/Makefile, too.
+# Don't forget to test with NO_PERL_MAKEMAKER=YesPlease
 
 # Sanity: die at first unknown option
 Getopt::Long::Configure qw/ pass_through /;
@@ -25,19 +29,18 @@ endif
 MAKE_FRAG
 }
 
-# XXX. When editing this list:
-#
-# * Please update perl/Makefile, too.
-# * Don't forget to test with NO_PERL_MAKEMAKER=YesPlease
-my %pm = (
-	'Git.pm' => '$(INST_LIBDIR)/Git.pm',
-	'Git/I18N.pm' => '$(INST_LIBDIR)/Git/I18N.pm',
-	'Git/SVN/Memoize/YAML.pm' => '$(INST_LIBDIR)/Git/SVN/Memoize/YAML.pm',
-	'Git/SVN/Fetcher.pm' => '$(INST_LIBDIR)/Git/SVN/Fetcher.pm',
-	'Git/SVN/Editor.pm' => '$(INST_LIBDIR)/Git/SVN/Editor.pm',
-	'Git/SVN/Prompt.pm' => '$(INST_LIBDIR)/Git/SVN/Prompt.pm',
-	'Git/SVN/Ra.pm' => '$(INST_LIBDIR)/Git/SVN/Ra.pm',
-);
+# Find all the .pm files in "Git/" and Git.pm
+my %pm;
+find sub {
+	return unless /\.pm$/;
+
+	# sometimes File::Find prepends a ./  Strip it.
+	my $pm_path = $File::Find::name;
+	$pm_path =~ s{^\./}{};
+
+	$pm{$pm_path} = '$(INST_LIBDIR)/'.$pm_path;
+}, "Git", "Git.pm";
+
 
 # We come with our own bundled Error.pm. It's not in the set of default
 # Perl modules so install it if it's not available on the system yet.
-- 
1.7.11.1

Re: [PATCH 3/3] The Makefile.PL will now find .pm files itself.

[Jonathan Nieder]

Hi,

Michael G. Schwern wrote:

> It is no longer necessary to manually add new .pm files to the
> Makefile.PL.  This makes it easier to add modules.

Thanks!  Sorry I missed this.

[...]
> --- a/perl/Makefile.PL
> +++ b/perl/Makefile.PL
> @@ -2,6 +2,10 @@ use strict;
>  use warnings;
>  use ExtUtils::MakeMaker;
>  use Getopt::Long;
> +use File::Find;
> +
> +# Don't forget to update the perl/Makefile, too.
> +# Don't forget to test with NO_PERL_MAKEMAKER=YesPlease

In a previous apartment I lived in, there was a note taped to the
lightswitch reminding us to turn off the heat, take keys with us, and
lock the door.  The note was useful because by force of habit we would
be turning off the light, and as a result see the note, on the way
out.

Who are these comments in perl/Makefile.PL addressed to?  Why would
such a person be looking at perl/Makefile.PL?  Sorry to sound like a
broken record, but I don't think these questions were answered yet.

How about this patch for squashing in, which would avoid the question
and save me from having to worry that my words are going to stay in
this file after the no-makemaker option no longer exists because
nobody looks at them here?

diff --git i/perl/Makefile.PL w/perl/Makefile.PL
index 3d88a6b9..377fd042 100644
--- i/perl/Makefile.PL
+++ w/perl/Makefile.PL
@@ -4,9 +4,6 @@ use ExtUtils::MakeMaker;
 use Getopt::Long;
 use File::Find;
 
-# Don't forget to update the perl/Makefile, too.
-# Don't forget to test with NO_PERL_MAKEMAKER=YesPlease
-
 # Sanity: die at first unknown option
 Getopt::Long::Configure qw/ pass_through /;
 

Re: [PATCH 3/3] The Makefile.PL will now find .pm files itself.

[Jonathan Nieder]

Jonathan Nieder wrote:
> Michael G. Schwern wrote:

>> --- a/perl/Makefile.PL
>> +++ b/perl/Makefile.PL
>> @@ -2,6 +2,10 @@ use strict;
>>  use warnings;
>>  use ExtUtils::MakeMaker;
>>  use Getopt::Long;
>> +use File::Find;
>> +
>> +# Don't forget to update the perl/Makefile, too.
>> +# Don't forget to test with NO_PERL_MAKEMAKER=YesPlease
[...]
> Who are these comments in perl/Makefile.PL addressed to?  Why would
> such a person be looking at perl/Makefile.PL?  Sorry to sound like a
> broken record, but I don't think these questions were answered yet.

To maybe answer my own question: are these comments addressed to
people making other changes to perl/Makefile.PL, rather than people
adding new modules?

That could make sense --- it would just be a change in purpose from
the original comments.  It also means there's no reminder when adding
new modules to list them in perl/Makefile any more, but that's
probably inevitable as long as we don't have a perl coding style
document.

Hoping that clarifies,
Jonathan

Re: [PATCH 3/3] The Makefile.PL will now find .pm files itself.

[Michael G Schwern]

On 2012.7.25 2:11 PM, Jonathan Nieder wrote:
>> --- a/perl/Makefile.PL
>> +++ b/perl/Makefile.PL
>> @@ -2,6 +2,10 @@ use strict;
>>  use warnings;
>>  use ExtUtils::MakeMaker;
>>  use Getopt::Long;
>> +use File::Find;
>> +
>> +# Don't forget to update the perl/Makefile, too.
>> +# Don't forget to test with NO_PERL_MAKEMAKER=YesPlease
> 
> In a previous apartment I lived in, there was a note taped to the
> lightswitch reminding us to turn off the heat, take keys with us, and
> lock the door.  The note was useful because by force of habit we would
> be turning off the light, and as a result see the note, on the way
> out.
> 
> Who are these comments in perl/Makefile.PL addressed to?

Somebody adding, renaming or deleting a .pm file.

> Why would such a person be looking at perl/Makefile.PL?

Because sometimes they do wacky things, especially in non-Perl projects, its
good to check.

> Sorry to sound like a broken record, but I don't think these questions
> were answered yet.

The instructions are still necessary and I don't know where to put them so
they have a better chance to be seen.  At least somebody adding a .pm file
might glance inside the Makefile.PL.


> How about this patch for squashing in, which would avoid the question
> and save me from having to worry that my words are going to stay in
> this file after the no-makemaker option no longer exists because
> nobody looks at them here?

If somebody eliminates NO_PERL_MAKEMAKER they'd grep the tree for all its
occurrences.  I'd rather keep the instructions in there, because having two
build systems is downright wacky.  In fact, I'd go on to say that an
explanation should be added to the Makefile as well.

This is out of scope for what I wanted this patch to do, and I really don't
have a horse in this race.  For my purposes, I just preserved the comment.  If
it goes away that's ok, too.

Later on I can help getting rid of the second build system.


> diff --git i/perl/Makefile.PL w/perl/Makefile.PL
> index 3d88a6b9..377fd042 100644
> --- i/perl/Makefile.PL
> +++ w/perl/Makefile.PL
> @@ -4,9 +4,6 @@ use ExtUtils::MakeMaker;
>  use Getopt::Long;
>  use File::Find;
>  
> -# Don't forget to update the perl/Makefile, too.
> -# Don't forget to test with NO_PERL_MAKEMAKER=YesPlease
> -
>  # Sanity: die at first unknown option
>  Getopt::Long::Configure qw/ pass_through /;



-- 
31. Not allowed to let sock puppets take responsibility for any of my
    actions.
    -- The 213 Things Skippy Is No Longer Allowed To Do In The U.S. Army
           http://skippyslist.com/list/

Re: [PATCH 3/3] The Makefile.PL will now find .pm files itself.

[Jonathan Nieder]

Michael G Schwern wrote:
> On 2012.7.25 2:11 PM, Jonathan Nieder wrote:

>> Who are these comments in perl/Makefile.PL addressed to?
>
> Somebody adding, renaming or deleting a .pm file.
>
>> Why would such a person be looking at perl/Makefile.PL?
>
> Because sometimes they do wacky things

Not convincing at all. ;-)

But my made-up justification about people making other changes to
perl/Makefile.PL convinced me, so keeping the comments seems fine to
me now.

[...]
>                             For my purposes, I just preserved the comment.

That's what I feared and how cruft collects.  Sorry for the lack of
clarity.

Thanks,
Jonathan

Re: Teach Makefile.PL to find .pm files on its own

[Junio C Hamano]

Looks sensible.  Will queue.

Thanks.

Re: Teach Makefile.PL to find .pm files on its own

[Michael G Schwern]

On 2012.7.25 9:56 AM, Junio C Hamano wrote:
> Looks sensible.  Will queue.
> 
> Thanks.

Thanks!

What's the lag time on it showing up in the repo, and which branch will it
appear in?

Also I just realized I've been basing my work on master.  Should I move to maint?


-- 
If you want the truth to stand clear before you, never be for or against.
The struggle between "for" and "against" is the mind's worst disease.
    -- Sent-ts'an

Re: Teach Makefile.PL to find .pm files on its own

[Junio C Hamano]

Michael G Schwern <schwern@pobox.com> writes:

> What's the lag time on it showing up in the repo, and which branch will it
> appear in?

There is nothing special in this topic, so it is likely to start on
'pu', and unlikely to come to 'master' before 1.7.12 ships sometime
next month.

> Also I just realized I've been basing my work on master.  Should I move to maint?

I don't think so.  It is not fixing any urgent breakage (iow, by
being told about .pm explicitly, it knows about them just fine
without being taught how to find them).

Re: Teach Makefile.PL to find .pm files on its own

[Michael G Schwern]

On 2012.7.25 1:32 PM, Junio C Hamano wrote:
> Michael G Schwern <schwern@pobox.com> writes:
> 
>> What's the lag time on it showing up in the repo, and which branch will it
>> appear in?
> 
> There is nothing special in this topic, so it is likely to start on
> 'pu', and unlikely to come to 'master' before 1.7.12 ships sometime
> next month.

Ok.


>> Also I just realized I've been basing my work on master.  Should I move to maint?
> 
> I don't think so.  It is not fixing any urgent breakage (iow, by
> being told about .pm explicitly, it knows about them just fine
> without being taught how to find them).

How about the git-svn SVN 1.7 fix in general?  All of these patch sets I'm
sending build on one another, is that going to be a problem?  It's going to
come in about six parts.

1) Makefile.PL .pm auto-find
2) Extract Git::SVN from git-svn
3) Extract the other modules from git-svn
4) Create and use accessors for paths and urls
5) Make the accessors canonicalize
6) Fix misc canonicalization issues

This is #1.  #2 was submitted last night.  #3 will be coming today.  #4 should
probably wait until #3 is at least in pu and I stop moving around large chunks
of code.

#1 to #4 are all refactorings with no functional changes.  #5 and #6 will be
tricky to provide as small commits while keeping all tests passing in SVN 1.6.
 We'll see when I get there after doing the rebasing necessary to separate #4,
#5 and #6.


-- 
10. Not allowed to purchase anyone's soul on government time.
    -- The 213 Things Skippy Is No Longer Allowed To Do In The U.S. Army
           http://skippyslist.com/list/

Re: Teach Makefile.PL to find .pm files on its own

[Junio C Hamano]

Michael G Schwern <schwern@pobox.com> writes:

>>> Also I just realized I've been basing my work on master.  Should I move to maint?
>> 
>> I don't think so.  It is not fixing any urgent breakage (iow, by
>> being told about .pm explicitly, it knows about them just fine
>> without being taught how to find them).
>
> How about the git-svn SVN 1.7 fix in general?  All of these patch sets I'm
> sending build on one another, is that going to be a problem?  It's going to
> come in about six parts.

Judging from the rate of the discussion this is progressing, I was
imagining that this series would be ready by 1.7.13 at the earliest,
possibly back-merged to 1.7.12.X maintenance series, and 1.7.11.X
maintenance series is no longer relevant by then.

But I certainly do not mind seeing the series based on earlier
maintenance releases, e.g. maint-1.7.9.  There however are tons of
other git-svn.perl and perl/ updates since then, so basing the
series on the current maint branch to abandon 1.7.10.X and earlier
but still leaving the door open to downmerge to 1.7.11.X may be a
good trade-off.

Re: Teach Makefile.PL to find .pm files on its own

[Michael G Schwern]

On 2012.7.25 3:19 PM, Junio C Hamano wrote:
> Michael G Schwern <schwern@pobox.com> writes:
>> How about the git-svn SVN 1.7 fix in general?  All of these patch sets I'm
>> sending build on one another, is that going to be a problem?  It's going to
>> come in about six parts.
> 
> Judging from the rate of the discussion this is progressing, I was
> imagining that this series would be ready by 1.7.13 at the earliest,
> possibly back-merged to 1.7.12.X maintenance series, and 1.7.11.X
> maintenance series is no longer relevant by then.
> 
> But I certainly do not mind seeing the series based on earlier
> maintenance releases, e.g. maint-1.7.9.  There however are tons of
> other git-svn.perl and perl/ updates since then, so basing the
> series on the current maint branch to abandon 1.7.10.X and earlier
> but still leaving the door open to downmerge to 1.7.11.X may be a
> good trade-off.

So... is that master or maint?  Just let me know which one.


-- 
91. I am not authorized to initiate Jihad.
    -- The 213 Things Skippy Is No Longer Allowed To Do In The U.S. Army
           http://skippyslist.com/list/

Re: Teach Makefile.PL to find .pm files on its own

[Junio C Hamano]

Michael G Schwern <schwern@pobox.com> writes:

> On 2012.7.25 3:19 PM, Junio C Hamano wrote:
>> Michael G Schwern <schwern@pobox.com> writes:
>>> How about the git-svn SVN 1.7 fix in general?  All of these patch sets I'm
>>> sending build on one another, is that going to be a problem?  It's going to
>>> come in about six parts.
>> 
>> Judging from the rate of the discussion this is progressing, I was
>> imagining that this series would be ready by 1.7.13 at the earliest,
>> possibly back-merged to 1.7.12.X maintenance series, and 1.7.11.X
>> maintenance series is no longer relevant by then.
>> 
>> But I certainly do not mind seeing the series based on earlier
>> maintenance releases, e.g. maint-1.7.9.  There however are tons of
>> other git-svn.perl and perl/ updates since then, so basing the
>> series on the current maint branch to abandon 1.7.10.X and earlier
>> but still leaving the door open to downmerge to 1.7.11.X may be a
>> good trade-off.
>
> So... is that master or maint?  Just let me know which one.

I do not care too deeply either way, and in the end I think Eric
should have the final say.

Given that git://git.bogomips.org/git-svn.git/ has 'master' but
nothing to build on 'maint', I would imagine that basing on master
is just fine.

Thanks.

Re: Teach Makefile.PL to find .pm files on its own

[Eric Wong]

Junio C Hamano <gitster@pobox.com> wrote:
> Michael G Schwern <schwern@pobox.com> writes:
> > So... is that master or maint?  Just let me know which one.
> 
> I do not care too deeply either way, and in the end I think Eric
> should have the final say.
> 
> Given that git://git.bogomips.org/git-svn.git/ has 'master' but
> nothing to build on 'maint', I would imagine that basing on master
> is just fine.

Yes, "master" is fine.