Simplify loading madness back to what I was originally trying to do
Tomas Doran [Thu, 1 Mar 2012 08:17:27 +0000 (08:17 +0000)]
Changes
lib/Catalyst/ScriptRunner.pm
lib/Catalyst/Utils.pm
t/aggregate/unit_core_scriptrunner_home.t [deleted file]
t/aggregate/unit_utils_home.t
t/lib/Makefile.PL [deleted file]
t/lib/TestAppEncoding/Controller/Root.pm

diff --git a/Changes b/Changes
index 49b8481..6320309 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,10 +1,13 @@
 # This file documents the revision history for Perl extension Catalyst.
 
  Bug fixes:
-  - Yet another fix to the previous fix to Catalyst::ScriptRunner which
-    was resulting in the lib directory not being pushed onto @INC.
-    This bug occurred when you were in a directory in your checkout
-    below the root of the application.
+  - Simplification of the previous changes to Catalyst::ScriptRunner
+    We now just push $FindBin::Bin/../lib to the @INC path again, but
+    only if one of the dist indicator files (Makefile.PL Build.PL or
+    dist.ini) can be found in $FindBin::Bin/../$_
+    This avoids heuristics when the app is unloaded and therefore
+    works better for extensions which have entire applications in
+    their test suites.
   - Bug fix to again correctly detect checkouts in dist zilla using
     applications.
 
index 7b80377..32031ed 100644 (file)
@@ -35,8 +35,8 @@ sub subclass_with_traits {
 sub run {
     my ($self, $appclass, $scriptclass) = @_;
 
-    if (my $home = Catalyst::Utils::find_home_unloaded_in_checkout()) {
-        lib->import(File::Spec->catdir($home, 'lib'));
+    if (grep { -f File::Spec->catdir($FindBin::Bin, '..', $_) } Catalyst::Utils::dist_indicator_file_list()) {
+        lib->import(File::Spec->catdir($FindBin::Bin, '..', 'lib'));
     }
 
     my $class = $self->find_script_class($appclass, $scriptclass);
index 6ba6f86..440d9e9 100644 (file)
@@ -6,10 +6,9 @@ use HTTP::Request;
 use Path::Class;
 use URI;
 use Carp qw/croak/;
+use Cwd;
 use Class::MOP;
 use String::RewritePrefix;
-use List::MoreUtils qw/ any /;
-use Cwd qw/ cwd /;
 
 use namespace::clean;
 
@@ -155,24 +154,21 @@ sub class2tempdir {
     return $tmpdir->stringify;
 }
 
+=head2 home($class)
+
+Returns home directory for given class.
+
 =head2 dist_indicator_file_list
 
-Returns a list of files which can be tested to check if you're inside a checkout
+Returns a list of files which can be tested to check if you're inside
+a checkout
 
 =cut
 
 sub dist_indicator_file_list {
-    qw/ Makefile.PL Build.PL dist.ini /;
+    qw{Makefile.PL Build.PL dist.ini};
 }
 
-=head2 home($class)
-
-Returns home directory for given class.
-
-Note that the class must be loaded for the home directory to be found using this function.
-
-=cut
-
 sub home {
     my $class = shift;
 
@@ -185,8 +181,25 @@ sub home {
 
             # find the @INC entry in which $file was found
             (my $path = $inc_entry) =~ s/$file$//;
-            my $home = find_home_unloaded_in_checkout($path);
-            return $home if $home;
+            $path ||= cwd() if !defined $path || !length $path;
+            my $home = dir($path)->absolute->cleanup;
+
+            # pop off /lib and /blib if they're there
+            $home = $home->parent while $home =~ /b?lib$/;
+
+            # only return the dir if it has a Makefile.PL or Build.PL or dist.ini
+            if (grep { -f $home->file($_) } dist_indicator_file_list()) {
+                # clean up relative path:
+                # MyApp/script/.. -> MyApp
+
+                my $dir;
+                my @dir_list = $home->dir_list();
+                while (($dir = pop(@dir_list)) && $dir eq '..') {
+                    $home = dir($home)->parent->parent;
+                }
+
+                return $home->stringify;
+            }
         }
 
         {
@@ -202,45 +215,7 @@ sub home {
     }
 
     # we found nothing
-    return;
-}
-
-=head2 find_home_unloaded_in_checkout ($path)
-
-Tries to determine if C<$path> (or the current working directory if not supplied)
-looks like a checkout. Any leading lib, script or blib components
-will be removed, then the directory produced will be checked
-for the existence of a C<< dist_indicator_file_list() >>.
-
-If one is found, the directory will be returned, otherwise false.
-
-=cut
-
-# XXX - Is this actually sane - should we just split into two simpler routines
-#       one for when we do have an @INC entry and one for when we don't?
-sub find_home_unloaded_in_checkout {
-    my ($path) = @_;
-    $path ||= cwd() if !defined $path || !length $path;
-    my $home = dir($path)->absolute->cleanup;
-    my $last_home;
-    do {
-        # only return the dir if it has a Makefile.PL or Build.PL or dist.ini
-        if (any { $_ } map { -f $home->file($_) } dist_indicator_file_list()) {
-            # clean up relative path:
-            # MyApp/script/.. -> MyApp
-
-            my $dir;
-            my @dir_list = $home->dir_list();
-            while (($dir = pop(@dir_list)) && $dir eq '..') {
-                $home = dir($home)->parent->parent;
-            }
-            return $home->stringify;
-        }
-        $last_home = $home;
-        $home = $home->parent;
-    }
-    while # pop off /lib and /blib or /script or /t/ if they're there
-        ($last_home =~ /b?lib$/ || $last_home =~ /script$/ || $last_home =~ /\/t(\/|$)/);
+    return 0;
 }
 
 =head2 prefix($class, $name);
diff --git a/t/aggregate/unit_core_scriptrunner_home.t b/t/aggregate/unit_core_scriptrunner_home.t
deleted file mode 100644 (file)
index 0816d25..0000000
+++ /dev/null
@@ -1,42 +0,0 @@
-use strict;
-use warnings;
-use Test::More;
-use FindBin qw/$Bin/;
-use Test::Exception;
-use lib "$Bin/../lib";
-use File::Temp qw/ tempdir /;
-use Cwd;
-
-use_ok('Catalyst::ScriptRunner');
-
-my $cwd = cwd();
-
-my $d = tempdir(); #CLEANUP => 1);
-chdir($d) or die;
-mkdir("lib") or die;
-mkdir(File::Spec->catdir("lib", "MyApp")) or die;
-mkdir(File::Spec->catdir("lib", "MyApp", "Script")) or die;
-
-open(my $fh, '>', 'Makefile.PL') or die;
-close($fh) or die;
-
-open($fh, '>', File::Spec->catdir("lib", "MyApp", "Script", "Foo.pm")) or die;
-print $fh q{package MyApp::Script::Foo;
-use Moose;
-use namespace::autoclean;
-
-with 'Catalyst::ScriptRole';
-
-sub run { __PACKAGE__ }
-
-1;
-};
-close($fh) or die;
-
-use_ok 'Catalyst::ScriptRunner';
-
-is Catalyst::ScriptRunner->run('MyApp', 'Foo'), 'MyApp::Script::Foo';
-
-chdir($cwd) or die;
-
-done_testing;
index 587f618..7e499ee 100644 (file)
@@ -30,15 +30,6 @@ foreach my $inc ('', 'lib', 'blib'){
     is Catalyst::Utils::home('MyApp'), dir($d, 'MyApp')->absolute->cleanup;
 }
 
-{
-    my $d = tempdir(CLEANUP => 1);
-    chdir($d);
-    ok !Catalyst::Utils::find_home_unloaded_in_checkout();
-    open(my $fh, '>', "Makefile.PL");
-    close($fh);
-    is Catalyst::Utils::find_home_unloaded_in_checkout(), cwd(), "Did find home_unloaded_in_checkout"
-}
-
 chdir($cwd);
 
 done_testing;
diff --git a/t/lib/Makefile.PL b/t/lib/Makefile.PL
deleted file mode 100644 (file)
index e69de29..0000000
index 64caa28..b82e1bf 100644 (file)
@@ -9,7 +9,7 @@ __PACKAGE__->config->{namespace} = '';
 sub binary : Local {
     my ($self, $c) = @_;
     $c->res->body(do {
-        open(my $fh, '<', $c->path_to('..', 'catalyst_130pix.gif')) or die $!; 
+        open(my $fh, '<', $c->path_to('..', '..', 'catalyst_130pix.gif')) or die $!; 
         binmode($fh); 
         local $/ = undef; <$fh>;
     });