[Catalyst-dev] Fwd: MRO::Compat / Class::C3 / Catalyst / Moose

[Guillermo Roditi]

This is a forward of the exchange I had with Brandon Black about this.
I think that we need to ditch Class::C3 and break back compat. Here's
a short rundown of why this has been such a nightmare:

The only thing affected in the core is Catalyst::Component::COMPONENT
COMPONENT attempts to call $self->next::method as follows:

return $self->next::can ? $self->next::method($c, $arguments) :
$self->new($c, $arguments);

This is necessary to support a case in which
 @MyApp::SomeComponent::ISA = ('MyApp::BaseComponent', 'Catalyst::Component');

This is by no means a long shot though. I would be very very surprised
if the change did not break some applications, however unless we want
to go back to NEXT this change is required.

Why this matters?
Well, simple really, because it means that, if I am not mistaken, we
would have to reinitialize a couple of times as things are loaded and
then components are instantiated (at &Catalyst::setup time). I think
if we do everything correctly we should be able to cut this down to
once before setup and once after, when everything is already loaded.
But the reinitialize operation is not quick for the core and I can
imagine it would be even less so for large applications.

All in all though, I think this problem sucks and I really see no
reason not to break this before it becomes too late.

Please speak up everyone. I really want to get the branch merged back
to trunk before it's too late. I will not work on this project again
if we fail to merge to trunk.

Please send failing test cases to the list! The more bugs you find the
quicker we can merge. (exchange between me and blblack below...)

-- 
Guillermo Roditi (groditi)

---------- Forwarded message ----------
From: Brandon Black <blblack at gmail.com>
Date: Wed, May 21, 2008 at 11:45 AM
Subject: Re: MRO::Compat / Class::C3 / Catalyst / Moose
To: Guillermo Roditi <groditi at gmail.com>


Guillermo Roditi wrote:
>
> As you may know, Catalyst is moving to Moose and we are also getting rid of NEXT and replacing it with Class::C3.
>
> I am in the process of introducing MRO::Compat into the codebase so that things JustWork in the future, but I do have one question for you. Most of my packages look like this
>
> package Catalyst::${something};
>
> use MRO::Compat;
> use mro 'c3';
> use Moose;
> extends 'Catalyst::${somethingElse}';
>
>
> My question is, when should I call reinitialize? Should I call it at the end of the file or after all the files are loaded or what? "extends" happens at runtime along with accessor creation etc, so I assume that I need to call it at some point or ther other for next::method to work correctly, but when is that time? Also, is this something that is called once per file or once for everything?
>
> Class::Data::Inheritable has a bad habit of writing to the symbol tables of packages that inherited the class accessor, so would I need to reinitialize after every time that happens?
>
> All I want is next::method next::can to work. If I can avoid the fancy method chaching stuff Class::C3 does I'd be perfectly content. Any words of advice? I have found no help on IRC and the core cat team is completely MIA
>

Sorry I've been completely MIA from perl lately too :)  The
reinitialize() debacle was one of the main reasons for moving C3 into
the perl core for 5.10.  There's simply no sane, universal way around
that problem.  One alternative is to ditch MRO::Compat, ignore
initialization issues, and add "require 5.010000" at the top :)

If you really just want "next::method" and "next::can", you don't have
to use reinitialize() at all.  Those always work and always use C3
order without any initialization.  But direct inheritance itself will
be broken until you do the intialization.

Generally speaking reinitialize() must be called after all c3-using
modules have been loaded and have done "use mro 'c3'", but before you
start making direct use of C3 inheritance, where direct use means
allowing methods to fall through the inheritance hierarchy in C3 order
when not defined locally in the class.  next::method (and presumably
before/after/around from Moose too) probably don't care about
reinitialize(), as they always pull the class precedence list anyways.
otoh, the automatic fallthrough inheritance set up by "extends" almost
certainly needs reinitialize() before calling the applicable methods.

The best place to put it for Catalyst would be once in Catalyst.pm,
after all plugins and components have already been loaded, but before
doing any real work.

The safest place to put reinitialize() is right after your "extends"
in your example above in every class, but that will be horribly slow
on 5.8.

But honestly, if Catalyst is really moving to Moose, it would be
better to just go all Moose and ditch C3.  With Moose one shouldn't
need C3 or next::method.  Don't ask me how you compatibly handle the
transition for all the existing plugins and components that use NEXT
and/or C3 though.  Seem like it's such a big change, if it were me I'd
branch Catalyst, convert wholesale to Moose w/o C3 in the branch, and
then announce a big version bump that makes all older Plugins /
Components obsolete and give people lots of warning and announcements
on the web, etc.  Then detect old components that haven't been
upgraded to Moose versions and die at startup in the Moosey-Catalyst
(should be easy, they won't have Moose metaclass info).

In the pure Moose sense, most of the plugins would just be handled as
roles rather than real classes with complex inheritance.

-- Brandon