From: Peter Rabbitson <ribasushi@cpan.org>
Date: Wed, 13 Jul 2016 13:45:37 +0000 (+0200)
Subject: Revert C3-fication d009cb7d and fixups 7f068248 and 983f766d
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=7648acb5;p=dbsrgits%2FDBIx-Class.git

Revert C3-fication d009cb7d and fixups 7f068248 and 983f766d

While on its surface this was a good idea, it actually hides problems even
more: by the time we arrive at a useful hook-point to check the current MRO,
something likely already changed it from under us, and the old effects are
all masked away for good.

So instead scale back as much as possible, and set 'c3' where needed as
lazily as practical. In order to satisfy the mro requirements imposed by
5e0eea35 we do the "flip" during the ->source() stage.

Additionally we record the original setting any time we switch the mro on
foreign classes (two such spots in the codebase). A later commit will use
this information to add the final bit of sanity to this clusterfuck.
---

diff --git a/lib/DBIx/Class/AccessorGroup.pm b/lib/DBIx/Class/AccessorGroup.pm
index c76a456..77cf852 100644
--- a/lib/DBIx/Class/AccessorGroup.pm
+++ b/lib/DBIx/Class/AccessorGroup.pm
@@ -4,7 +4,6 @@ use strict;
 use warnings;
 
 use base qw( DBIx::Class::MethodAttributes Class::Accessor::Grouped );
-use mro 'c3';
 
 use Scalar::Util 'blessed';
 use DBIx::Class::_Util 'fail_on_internal_call';
@@ -41,8 +40,6 @@ sub get_component_class {
   ) {
     $_[0]->ensure_class_loaded($class);
 
-    mro::set_mro( $class, 'c3' );
-
     ${"${class}::__LOADED__BY__DBIC__CAG__COMPONENT_CLASS__"}
       = do { \(my $anon = 'loaded') };
   }
diff --git a/lib/DBIx/Class/Componentised.pm b/lib/DBIx/Class/Componentised.pm
index 3adea57..b417de6 100644
--- a/lib/DBIx/Class/Componentised.pm
+++ b/lib/DBIx/Class/Componentised.pm
@@ -13,9 +13,6 @@ use namespace::clean;
 
 # this warns of subtle bugs introduced by UTF8Columns hacky handling of store_column
 # if and only if it is placed before something overriding store_column
-#
-# and also enforces C3 mro on all components
-my $mro_already_set;
 sub inject_base {
   my $class = shift;
   my ($target, @complist) = @_;
@@ -75,12 +72,6 @@ sub inject_base {
     unshift @target_isa, $comp;
   }
 
-  # only examine from $_[2] onwards
-  # C::C3::C already sets c3 on $_[1]
-  mro::set_mro( $_ => 'c3' ) for grep {
-    $mro_already_set->{$_} ? 0 : ( $mro_already_set->{$_} = 1 )
-  } @_[1 .. $#_];
-
   $class->next::method(@_);
 }
 
diff --git a/lib/DBIx/Class/MethodAttributes.pm b/lib/DBIx/Class/MethodAttributes.pm
index 1b50ac9..7ffe560 100644
--- a/lib/DBIx/Class/MethodAttributes.pm
+++ b/lib/DBIx/Class/MethodAttributes.pm
@@ -6,7 +6,6 @@ use warnings;
 use DBIx::Class::_Util qw( uniq refdesc visit_namespaces );
 use Scalar::Util qw( weaken refaddr );
 
-use mro 'c3';
 use namespace::clean;
 
 my ( $attr_cref_registry, $attr_cache_active );
diff --git a/lib/DBIx/Class/ResultSet.pm b/lib/DBIx/Class/ResultSet.pm
index 6dbc7ca..2c5131d 100644
--- a/lib/DBIx/Class/ResultSet.pm
+++ b/lib/DBIx/Class/ResultSet.pm
@@ -4,7 +4,6 @@ use strict;
 use warnings;
 
 use base 'DBIx::Class';
-use mro 'c3';
 
 use DBIx::Class::Carp;
 use DBIx::Class::ResultSetColumn;
diff --git a/lib/DBIx/Class/ResultSource.pm b/lib/DBIx/Class/ResultSource.pm
index a7645ef..f6e3923 100644
--- a/lib/DBIx/Class/ResultSource.pm
+++ b/lib/DBIx/Class/ResultSource.pm
@@ -4,7 +4,6 @@ use strict;
 use warnings;
 
 use base 'DBIx::Class::ResultSource::RowParser';
-use mro 'c3';
 
 use DBIx::Class::Carp;
 use DBIx::Class::_Util qw( UNRESOLVABLE_CONDITION dbic_internal_try fail_on_internal_call );
diff --git a/lib/DBIx/Class/ResultSource/RowParser.pm b/lib/DBIx/Class/ResultSource/RowParser.pm
index aff2b81..676a548 100644
--- a/lib/DBIx/Class/ResultSource/RowParser.pm
+++ b/lib/DBIx/Class/ResultSource/RowParser.pm
@@ -5,7 +5,6 @@ use strict;
 use warnings;
 
 use base 'DBIx::Class';
-use mro 'c3';
 
 use Try::Tiny;
 
diff --git a/lib/DBIx/Class/ResultSource/Table.pm b/lib/DBIx/Class/ResultSource/Table.pm
index e1dcc03..450be9a 100644
--- a/lib/DBIx/Class/ResultSource/Table.pm
+++ b/lib/DBIx/Class/ResultSource/Table.pm
@@ -4,7 +4,6 @@ use strict;
 use warnings;
 
 use base 'DBIx::Class::ResultSource';
-use mro 'c3';
 
 =head1 NAME
 
diff --git a/lib/DBIx/Class/ResultSource/View.pm b/lib/DBIx/Class/ResultSource/View.pm
index 5995790..3339826 100644
--- a/lib/DBIx/Class/ResultSource/View.pm
+++ b/lib/DBIx/Class/ResultSource/View.pm
@@ -4,7 +4,6 @@ use strict;
 use warnings;
 
 use base 'DBIx::Class::ResultSource';
-use mro 'c3';
 
 __PACKAGE__->mk_group_accessors(
     'simple' => qw(is_virtual view_definition deploy_depends_on) );
diff --git a/lib/DBIx/Class/ResultSourceProxy.pm b/lib/DBIx/Class/ResultSourceProxy.pm
index 0f5e9d9..5f4bbe3 100644
--- a/lib/DBIx/Class/ResultSourceProxy.pm
+++ b/lib/DBIx/Class/ResultSourceProxy.pm
@@ -5,7 +5,6 @@ use strict;
 use warnings;
 
 use base 'DBIx::Class';
-use mro 'c3';
 
 use DBIx::Class::_Util qw( quote_sub fail_on_internal_call );
 use namespace::clean;
diff --git a/lib/DBIx/Class/Schema.pm b/lib/DBIx/Class/Schema.pm
index 5b9d07c..f19c7bc 100644
--- a/lib/DBIx/Class/Schema.pm
+++ b/lib/DBIx/Class/Schema.pm
@@ -4,7 +4,6 @@ use strict;
 use warnings;
 
 use base 'DBIx::Class';
-use mro 'c3';
 
 use DBIx::Class::Carp;
 use Try::Tiny;
@@ -631,6 +630,43 @@ sub source {
       ||
     $self->throw_exception( "Can't find source for ${source_name}" )
   ;
+
+  # DO NOT REMOVE:
+  # We need to prevent alterations of pre-existing $@ due to where this call
+  # sits in the overall stack ( *unless* of course there is an actual error
+  # to report ). set_mro does alter $@ (and yes - it *can* throw an exception)
+  # We do not use local because set_mro *can* throw an actual exception
+  # We do not use a try/catch either, as on one hand it would slow things
+  # down for no reason (we would always rethrow), but also because adding *any*
+  # try/catch block below will segfault various threading tests on older perls
+  # ( which in itself is a FIXME but ENOTIMETODIG )
+  my $old_dollarat = $@;
+
+  no strict 'refs';
+  mro::set_mro($_, 'c3') for
+    grep
+      {
+        # some pseudo-sources do not have a result/resultset yet
+        defined $_
+          and
+        (
+          (
+            ${"${_}::__INITIAL_MRO_UPON_DBIC_LOAD__"}
+              ||= mro::get_mro($_)
+          )
+            ne
+          'c3'
+        )
+      }
+      map
+        { length ref $_ ? ref $_ : $_ }
+        ( $rsrc, $rsrc->result_class, $rsrc->resultset_class )
+  ;
+
+  # DO NOT REMOVE - see comment above
+  $@ = $old_dollarat;
+
+  $rsrc;
 }
 
 =head2 class
diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm
index 2734385..9da3bd9 100644
--- a/lib/DBIx/Class/Storage/DBI.pm
+++ b/lib/DBIx/Class/Storage/DBI.pm
@@ -1321,7 +1321,17 @@ sub _determine_driver {
       if ($driver) {
         my $storage_class = "DBIx::Class::Storage::DBI::${driver}";
         if ($self->load_optional_class($storage_class)) {
-          mro::set_mro($storage_class, 'c3');
+
+          no strict 'refs';
+          mro::set_mro($storage_class, 'c3') if
+            (
+              ${"${storage_class}::__INITIAL_MRO_UPON_DBIC_LOAD__"}
+                ||= mro::get_mro($storage_class)
+            )
+              ne
+            'c3'
+          ;
+
           bless $self, $storage_class;
           $self->_rebless();
         }
diff --git a/xt/extra/c3_mro.t b/xt/extra/c3_mro.t
index 398f51e..fa63e0c 100644
--- a/xt/extra/c3_mro.t
+++ b/xt/extra/c3_mro.t
@@ -6,6 +6,7 @@ use strict;
 use Test::More;
 use DBICTest;
 use DBIx::Class::Optional::Dependencies;
+use DBIx::Class::_Util 'uniq';
 
 my @global_ISA_tail = qw(
   DBIx::Class
@@ -16,16 +17,11 @@ my @global_ISA_tail = qw(
   Class::Accessor::Grouped
 );
 
-is(
-  mro::get_mro($_),
-  'c3',
-  "Correct mro on base class '$_'",
-) for grep { $_ =~ /^DBIx::Class/ } @global_ISA_tail;
-
 {
   package AAA;
 
   use base "DBIx::Class::Core";
+  use mro 'c3';
 }
 
 {
@@ -55,23 +51,27 @@ ok (! $@, "Correctly skipped injecting an indirect parent of class BBB");
 
 my $art = DBICTest->init_schema->resultset("Artist")->next;
 
-check_ancestry($_) for (
-  ref( $art ),
-  ref( $art->result_source ),
-  ref( $art->result_source->resultset ),
-  ref( $art->result_source->schema ),
-  ( map
-    { ref $art->result_source->schema->source($_) }
-    $art->result_source->schema->sources
-  ),
-  qw( AAA BBB CCC ),
-  ((! DBIx::Class::Optional::Dependencies->req_ok_for('cdbicompat') ) ? () : do {
-    unshift @INC, 't/cdbi/testlib';
-    map { eval "require $_" or die $@; $_ } qw(
-      Film Lazy Actor ActorAlias ImplicitInflate
-    );
-  }),
-);
+check_ancestry($_) for uniq map
+  { length ref $_ ? ref $_ : $_ }
+  (
+    $art,
+    $art->result_source,
+    $art->result_source->resultset,
+    ( map
+      { $_, $_->result_class, $_->resultset_class }
+      map
+        { $art->result_source->schema->source($_) }
+        $art->result_source->schema->sources
+    ),
+    qw( AAA BBB CCC ),
+    ((! DBIx::Class::Optional::Dependencies->req_ok_for('cdbicompat') ) ? () : do {
+      unshift @INC, 't/cdbi/testlib';
+      map { eval "require $_" or die $@; $_ } qw(
+        Film Lazy Actor ActorAlias ImplicitInflate
+      );
+    }),
+  )
+;
 
 use DBIx::Class::Storage::DBI::Sybase::Microsoft_SQL_Server;
 
@@ -129,15 +129,11 @@ sub check_ancestry {
     "Correct end of \@ISA for '$class'"
   );
 
-  # check the remainder
-  for my $c (@linear_ISA) {
-    # nothing to see there
-    next if $c =~ /^DBICTest::/;
-
-    next if mro::get_mro($c) eq 'c3';
-
-    fail( "Incorrect mro '@{[ mro::get_mro($c) ]}' on '$c' (parent of '$class')" );
-  }
+  is(
+    mro::get_mro($class),
+    'c3',
+    "Expected mro on class '$class' automatically set",
+  );
 }
 
 done_testing;