Fix a 5.70/5.80 behavior change in Catalyst::Utils::ensure_class_loaded, pointed...
Tomas Doran [Sun, 14 Dec 2008 11:04:33 +0000 (11:04 +0000)]
Changes
TODO
lib/Catalyst/Utils.pm
t/lib/NullPackage.pm [new file with mode: 0644]
t/unit_utils_load_class.t

diff --git a/Changes b/Changes
index ecf47cf..1882335 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,5 +1,8 @@
 # This file documents the revision history for Perl extension Catalyst.
 
+        - Fix loading of classes which do not define any symbols to not
+          die, as it didn't in 5.70 (t0m)
+          - Test for this (t0m)
         - Bump MooseX::Emulate::Class::Accessor::Fast dependency
           to force new version which fixes a lot of plugins (t0m)
         - Make log levels additive, and add documentation and tests
diff --git a/TODO b/TODO
index 50bcd78..75c3176 100644 (file)
--- a/TODO
+++ b/TODO
@@ -5,6 +5,9 @@
     http://code2.0beta.co.uk/moose/svnweb/index.cgi/moose/revision?rev=7036
     (groditi)
 
+  - Fix 'as soon as Class::MOP 0.67 + 1 is released 
+    Class::MOP::load_class($class) can be used instead' in Catalyst::Utils
+
   - Common engine test failures, look into and get tests into core.
 
   - Catalyst-Plugin-Authorization-ACL, Can't locate object method "tree" 
index 9fd9c32..5d5cfc3 100644 (file)
@@ -259,10 +259,13 @@ sub ensure_class_loaded {
     croak "ensure_class_loaded should be given a classname, not a filename ($class)"
         if $class =~ m/\.pm$/;
 
+    # $opts->{ignore_loaded} can be set to true, and this causes the class to be required, even
+    # if it already has symbol table entries. This is to support things like Schema::Loader, which
+    # part-generate classes in memory, but then also load some of their contents from disk.
     return if !$opts->{ ignore_loaded }
         && Class::MOP::is_class_loaded($class); # if a symbol entry exists we don't load again
 
-    # as soon as Class::MOP 0.67 + 1 is released Class::MOP::load_class($class) can be used instead
+    # FIXME - as soon as Class::MOP 0.67 + 1 is released Class::MOP::load_class($class) can be used instead
 
     # this hack is so we don't overwrite $@ if the load did not generate an error
     my $error;
@@ -276,7 +279,7 @@ sub ensure_class_loaded {
 
     die $error if $error;
 
-    die "require $class was successful but the package is not defined"
+    warn "require $class was successful but the package is not defined."
         unless Class::MOP::is_class_loaded($class);
 
     return 1;
diff --git a/t/lib/NullPackage.pm b/t/lib/NullPackage.pm
new file mode 100644 (file)
index 0000000..47dcfda
--- /dev/null
@@ -0,0 +1,7 @@
+package NullPackage;
+# Do nothing class, there should be no code or symbols defined here..
+# Loading this works fine in 5.70, but a die was introduced in 5.80 which caused
+# it to fail. This has been changed to a warning to maintain back-compat.
+# See Catalyst::Utils::ensure_class_loaded() for more info.
+1;
+
index 4231ba7..881b1ff 100644 (file)
@@ -3,7 +3,7 @@
 use strict;
 use warnings;
 
-use Test::More tests => 16;
+use Test::More tests => 18;
 use Class::MOP;
 
 use lib "t/lib";
@@ -66,3 +66,9 @@ undef $@;
 eval { Catalyst::Utils::ensure_class_loaded('Silly::File::Name.pm') };
 like($@, qr/Malformed class Name/, 'errored sanely when given a classname ending in .pm');
 
+undef $@;
+$warnings = 0;
+Catalyst::Utils::ensure_class_loaded("NullPackage");
+is( $warnings, 1, 'Loading a package which defines no symbols warns');
+is( $@, undef, '$@ still undef' );
+