You are viewing a plain text version of this content. The canonical link for it is here.
Posted to modperl-cvs@perl.apache.org by au...@apache.org on 2006/07/05 17:06:30 UTC
svn commit: r419247 - /perl/Apache-SizeLimit/trunk/Makefile.PL
Author: autarch
Date: Wed Jul 5 08:06:29 2006
New Revision: 419247
URL: http://svn.apache.org/viewvc?rev=419247&view=rev
Log:
Cannot call sub to define a constant if the sub has not yet been
defined, so just use a variable instead.
More refactoring to make flow of execution simpler (IMHO).
The test target works fine on my box without explicitly setting the
apache binary or apxs binary. It would be very bad form for a CPAN
module to die in the Makefile.PL! That'd break any attempt to install
it via a CPAN{PLUS} shell. I'll figure out how to make this optional
in a future commit.
Modified:
perl/Apache-SizeLimit/trunk/Makefile.PL
Modified: perl/Apache-SizeLimit/trunk/Makefile.PL
URL: http://svn.apache.org/viewvc/perl/Apache-SizeLimit/trunk/Makefile.PL?rev=419247&r1=419246&r2=419247&view=diff
==============================================================================
--- perl/Apache-SizeLimit/trunk/Makefile.PL (original)
+++ perl/Apache-SizeLimit/trunk/Makefile.PL Wed Jul 5 08:06:29 2006
@@ -20,7 +20,7 @@
}
}
-use constant HAS_APACHE_TEST => check_for_apache_test();
+my $HAS_APACHE_TEST = check_for_apache_test();
WriteMakefile( VERSION_FROM => "lib/Apache/SizeLimit.pm",
NAME => "Apache::SizeLimit",
@@ -30,24 +30,21 @@
);
sub check_for_apache_test {
- return eval {
+ return unless eval {
require Apache::Test;
require Apache::TestMM;
require Apache::TestRunPerl;
+ 1;
+ };
- Apache::TestMM->import(qw(test clean));
- Apache::TestMM::filter_args();
-
- my %args = @Apache::TestMM::Argv;
+ Apache::TestMM->import(qw(test clean));
+ Apache::TestMM::filter_args();
- die 'suitable httpd required'
- unless ($args{apxs} or $args{httpd} or
- $ENV{APACHE_TEST_HTTPD} or $ENV{APACHE_TEST_APXS});
+ my %args = @Apache::TestMM::Argv;
- Apache::TestRunPerl->generate_script();
+ Apache::TestRunPerl->generate_script();
- return Apache::TestMM->test;
- };
+ return 1;
}
sub MY::postamble {
@@ -65,26 +62,22 @@
}
sub MY::test {
+ my $self = shift;
- my $test = shift->MM::test(@_);
-
- eval { require Test::More } or return <<EOF;
+ eval { require Test::More } or return <<EOF;
test::
\t\@echo sorry, cannot run tests without Test::More
EOF
- return HAS_APACHE_TEST if HAS_APACHE_TEST;
+ return Apache::TestMM::test(@_) if $HAS_APACHE_TEST;
- return $test;
+ return $self->MM::test(@_);
}
sub MY::clean {
+ my $self = shift;
- if (HAS_APACHE_TEST) {
- require Apache::TestMM;
-
- return Apache::TestMM::clean(@_);
- }
+ return Apache::TestMM::clean(@_) if $HAS_APACHE_TEST;
return shift->MM::clean(@_);
}
Re: svn commit: r419247 - /perl/Apache-SizeLimit/trunk/Makefile.PL
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> My main concern is getting people to run the tests for the RCs I post.
> Folks on this list will probably go the extra step, but how about people
> on the mp users list?
hmm, right.
ok, in the next release candidate announcement you can specify how
things should be tested, something akin to
"please test this with
$ perl Makefile.PL -httpd /path/to/your/httpd
..."
there's a big difference between people reporting back misconfiguration
"test failures" on a release candidate than once a release is in the wild.
>
> There's also the issue that I cannot test on all the platforms this
> module supports, which makes widespread running of the tests even more
> important, IMO.
>
>> but in the end, I leave the choice to you :)
>
>
> I think I found a reasonable way to do this:
>
> -f Apache::TestConfig->custom_config_path()
> or $args{apxs} or $args{httpd}
> or $ENV{APACHE_TEST_HTTPD} or $ENV{APACHE_TEST_APXS}
>
> That works on my system, at least.
sweet. go for it.
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: svn commit: r419247 - /perl/Apache-SizeLimit/trunk/Makefile.PL
Posted by Dave Rolsky <au...@urth.org>.
On Thu, 6 Jul 2006, Geoffrey Young wrote:
>> I'm with you on the goal, but I think it needs to check to for
>> TestConfigData.pm and ~/.apache-test as well to be useful.
>
> I guess that depends on your definition of useful. doing so would make
> it run tests for more users automatically, so in that sense it's good.
> but doing so would also mean more work for us
My main concern is getting people to run the tests for the RCs I post.
Folks on this list will probably go the extra step, but how about people
on the mp users list?
There's also the issue that I cannot test on all the platforms this module
supports, which makes widespread running of the tests even more important,
IMO.
> but in the end, I leave the choice to you :)
I think I found a reasonable way to do this:
-f Apache::TestConfig->custom_config_path()
or $args{apxs} or $args{httpd}
or $ENV{APACHE_TEST_HTTPD} or $ENV{APACHE_TEST_APXS}
That works on my system, at least.
-dave
/*===================================================
VegGuide.Org www.BookIRead.com
Your guide to all that's veg. My book blog
===================================================*/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: svn commit: r419247 - /perl/Apache-SizeLimit/trunk/Makefile.PL
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
>> my %args = @Apache::TestMM::Argv;
>>
>> die 'suitable httpd required'
>> unless ($args{apxs} or $args{httpd} or
>> $ENV{APACHE_TEST_HTTPD} or $ENV{APACHE_TEST_APXS});
>
>
> It's too bad this isn't an Apache::Test method/function. That'd be quite
> handy.
yeah, it would. unfortunately, even if it were it might be too late in
the game for it to be truly useful.
>
>> where the user is Apache-Test savvy. I _think_ it also covers when a
>> user has ~/.apache-test and/or TestConfigData.pm, but now I just can't
>> remember.
>
>
> Unfortunately it doesn't, which is kind of bad, since I have
> TestConfigData.pm and it's kind of important that I can run the tests.
Apache-Test savvy people can always run the tests
$ perl Makefile.PL -httpd /path/to/your/httpd
> That may be why I first looked at that code and wanted to remove it.
right. I was taking the position that it's better to not run the tests
(and risk an improper setup from the unaware) than to cause developers
or in-the-know people an extra step.
>> make sense?
>
>
> I'm with you on the goal, but I think it needs to check to for
> TestConfigData.pm and ~/.apache-test as well to be useful.
I guess that depends on your definition of useful. doing so would make
it run tests for more users automatically, so in that sense it's good.
but doing so would also mean more work for us
>
> Is there a separate check I can do for those? Looking at
> Apache::TestConfig I cannot figure out how to do it without invoking the
> prompts.
I don't think so. I think the process needs to be something like
creating the config then checking the test server object. but creating
the config is done with
t/TEST -config
during the test cycle. so what _really_ needs to happen in order to be
effective is creating the config during the 'perl Makefile.PL' phase
rather than the 'make test' phase. and I think I tried doing that a
while ago but couldn't make it work properly.
but at any rate, I don't think we need to decide this now and hold up
the release any longer. my thought is that we should allow this release
to focus its attention on the API changes and memory issues that
prompted it, rather than need to wade through bogus bug reports. so in
that sense, I'd rather users not run the tests if they don't know how to
properly configure their environment.
but in the end, I leave the choice to you :)
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: svn commit: r419247 - /perl/Apache-SizeLimit/trunk/Makefile.PL
Posted by Dave Rolsky <au...@urth.org>.
On Thu, 6 Jul 2006, Geoffrey Young wrote:
>
>> I don't suppose there's a simple way to ask Apache::Test if it can find
>> a server without prompting the user? That seems like the best way to
>> test for this.
>
> yeah, that's the code I had in there:
>
> my %args = @Apache::TestMM::Argv;
>
> die 'suitable httpd required'
> unless ($args{apxs} or $args{httpd} or
> $ENV{APACHE_TEST_HTTPD} or $ENV{APACHE_TEST_APXS});
It's too bad this isn't an Apache::Test method/function. That'd be quite
handy.
> where the user is Apache-Test savvy. I _think_ it also covers when a
> user has ~/.apache-test and/or TestConfigData.pm, but now I just can't
> remember.
Unfortunately it doesn't, which is kind of bad, since I have
TestConfigData.pm and it's kind of important that I can run the tests.
That may be why I first looked at that code and wanted to remove it.
> "the tests will fail unless the httpd environment isn't properly set up,
> but those test failures aren't really test failures they're environment
> failures - you don't know if the tests fail or not because you can't
> actually run them. so, do our best to set up the environment (without
> prompting the user), but if we can't don't run the tests."
>
> in my own modules, I really don't care about telling people "go setup
> Apache-Test, then your tests won't fail" but for our public
> distributions I think it looks bad to have failing cpan-tester reports
> and bug reports flooding the dev list due to a bad test environment
> (rather than actual failing tests).
>
> make sense?
I'm with you on the goal, but I think it needs to check to for
TestConfigData.pm and ~/.apache-test as well to be useful.
Is there a separate check I can do for those? Looking at
Apache::TestConfig I cannot figure out how to do it without invoking the
prompts.
-dave
/*===================================================
VegGuide.Org www.BookIRead.com
Your guide to all that's veg. My book blog
===================================================*/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: svn commit: r419247 - /perl/Apache-SizeLimit/trunk/Makefile.PL
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> I don't suppose there's a simple way to ask Apache::Test if it can find
> a server without prompting the user? That seems like the best way to
> test for this.
yeah, that's the code I had in there:
my %args = @Apache::TestMM::Argv;
die 'suitable httpd required'
unless ($args{apxs} or $args{httpd} or
$ENV{APACHE_TEST_HTTPD} or $ENV{APACHE_TEST_APXS});
basically, this checks standard Apache-Test %ENV variables as well as
cases like
$ perl Makefile.PL -apxs /usr/local/apache/bin/apxs
where the user is Apache-Test savvy. I _think_ it also covers when a
user has ~/.apache-test and/or TestConfigData.pm, but now I just can't
remember.
my goal for stuff like this has always been
"the tests will fail unless the httpd environment isn't properly set up,
but those test failures aren't really test failures they're environment
failures - you don't know if the tests fail or not because you can't
actually run them. so, do our best to set up the environment (without
prompting the user), but if we can't don't run the tests."
in my own modules, I really don't care about telling people "go setup
Apache-Test, then your tests won't fail" but for our public
distributions I think it looks bad to have failing cpan-tester reports
and bug reports flooding the dev list due to a bad test environment
(rather than actual failing tests).
make sense?
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: svn commit: r419247 - /perl/Apache-SizeLimit/trunk/Makefile.PL
Posted by "Philip M. Gollucci" <pg...@p6m7g8.com>.
Dave Rolsky wrote:
> Ah, good point. I didn't realize you were trying to avoid the prompts,
> as I've seen those many times in the past.
>
> I don't suppose there's a simple way to ask Apache::Test if it can find
> a server without prompting the user? That seems like the best way to
> test for this.
Yeah, but it will likely find the wrong one.
--
------------------------------------------------------------------------
Philip M. Gollucci (pgollucci@p6m7g8.com) 323.219.4708
Consultant / http://p6m7g8.net/Resume/resume.shtml
Senior Software Engineer - TicketMaster - http://ticketmaster.com
1024D/A79997FA F357 0FDD 2301 6296 690F 6A47 D55A 7172 A799 97F
"It takes a minute to have a crush on someone, an hour to like someone,
and a day to love someone, but it takes a lifetime to forget someone..."
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: svn commit: r419247 - /perl/Apache-SizeLimit/trunk/Makefile.PL
Posted by Dave Rolsky <au...@urth.org>.
On Thu, 6 Jul 2006, Geoffrey Young wrote:
> autarch@apache.org wrote:
>> Author: autarch
>> Date: Wed Jul 5 08:06:29 2006
>> New Revision: 419247
>>
>> URL: http://svn.apache.org/viewvc?rev=419247&view=rev
>> Log:
>> Cannot call sub to define a constant if the sub has not yet been
>> defined, so just use a variable instead.
>
> hmm, it worked ok for me (and has for quite a while)... but ok, whatever.
It had to do with the fact that I had moved the code out of the constant
definition and into its own sub, and the sub was defined _later_ in the
file. Since "use constant" happens at compile time right when Perl sees
it, the sub was not yet in place.
> it was in the eval that defined HAS_APACHE_TEST, so no it wouldn't have.
>
> I think that code needs to be re-added back in. maybe not exactly the
> way it was if you think it could be better, but the idea is so that no
> combination of Apache-Test, it's configuration files, or lack thereof
> causes a simple
>
> $ perl Makefile.PL && make && make test
>
> to either fail or enter into an interactive process (which would also
> cause the CPAN shell to timeout and not work). I worked pretty hard to
> cover all the cases, and I don't really code much without a real reason :)
Ah, good point. I didn't realize you were trying to avoid the prompts, as
I've seen those many times in the past.
I don't suppose there's a simple way to ask Apache::Test if it can find a
server without prompting the user? That seems like the best way to test
for this.
-dave
/*===================================================
VegGuide.Org www.BookIRead.com
Your guide to all that's veg. My book blog
===================================================*/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: svn commit: r419247 - /perl/Apache-SizeLimit/trunk/Makefile.PL
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
autarch@apache.org wrote:
> Author: autarch
> Date: Wed Jul 5 08:06:29 2006
> New Revision: 419247
>
> URL: http://svn.apache.org/viewvc?rev=419247&view=rev
> Log:
> Cannot call sub to define a constant if the sub has not yet been
> defined, so just use a variable instead.
hmm, it worked ok for me (and has for quite a while)... but ok, whatever.
>
> More refactoring to make flow of execution simpler (IMHO).
simpler, but not covering all the cases...
>
> The test target works fine on my box without explicitly setting the
> apache binary or apxs binary.
yes, because you have things like ~/.apache-test to tell it where they
are. for a user that doesn't have that or TestConfigData.pm but has
Apache-Test installed you'll get this:
$ perl Makefile.PL && make && make test
Writing Makefile for Apache::SizeLimit
...
We are now going to configure the Apache-Test framework.
This configuration process needs to be done only once.
...
Please provide a full path to 'httpd' executable:
[]
you won't have those files if you choose not to have them (by setting
$ENV{APACHE_TEST_NO_STICKY_PREFERENCES}) of if you've installed
Apache-Test from CPAN but haven't yet used it to run any tests. or a
bunch of other reasons.
> It would be very bad form for a CPAN
> module to die in the Makefile.PL! That'd break any attempt to install
> it via a CPAN{PLUS} shell. I'll figure out how to make this optional
> in a future commit.
it was in the eval that defined HAS_APACHE_TEST, so no it wouldn't have.
I think that code needs to be re-added back in. maybe not exactly the
way it was if you think it could be better, but the idea is so that no
combination of Apache-Test, it's configuration files, or lack thereof
causes a simple
$ perl Makefile.PL && make && make test
to either fail or enter into an interactive process (which would also
cause the CPAN shell to timeout and not work). I worked pretty hard to
cover all the cases, and I don't really code much without a real reason :)
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org