[Dbix-class] Class::C3::Componentised bug (was deployment bug with DBIC running from PAR)

[Toby Corkindale]

Peter Rabbitson wrote:
> Toby Corkindale wrote:
>> Peter Rabbitson wrote:
>>> Toby Corkindale wrote:
>>>> I need to stop replying to myself :(
>>>> I'd say the problem lies this block of code:
>>>>
>>>> # Look through the @INC path to find the file
>>>>     foreach ( @try_first, @INC ) {
>>>>         my $full = "$_/$filename";
>>>>         next unless -e $full;
>>>>         return $UNIX ? $full : $class->_inc_to_local($full);
>>>> }
>>>>
>>>> since when using PAR, the first entry in @INC is a coderef, and thus
>>>> this routine tests for '-e "CODE(0xd34db33f)/Foo/Bar.pm"' which, not
>>>> unsurprisingly, fails.
>>>>
>>>> D'Oh.
>>>>
>>>> I'll raise some bugs on RT. Not sure where the blame lies now..
>>> Hi,
>>> The issue you describe was worked around in this commit.
>>> http://dev.catalyst.perl.org/svnweb/bast/revision/?rev=5355
>>>
>>> I would recommend against raising a bug against C3:: as it behaves
>>> correctly
>>> wrt availability of a module. Since there is no mechanism for a
>>> coderef in
>>> @INC to say "I didn't find" vs "I didn't load", ensure_class_found can
>>> not
>>> be expected to work correctly. A POD patch indicating the above would be
>>> most welcome.
>> I'm not entirely sure what that POD should say though..
>> "Warning - this feature will not work when used in one of the two ways
>> you can use PAR, and thus should not be used at all if you intend your
>> application, or any of its descendants, to work properly" ?
>>
>> Perhaps It would be better to submitt a patch against
>> DBIx::Class::Schema::Storage::DBI to remove the uses of
>> ensure_class_found instead?
> 
> You didn't check the link I sent did you.

Hi Peter,
I did check that link; however it didn't change my feeling on the 
matter, ie. that the mentioned function should always be avoided if it 
is (apparently) well known to cause issues downstream.

The thing is, a doc patch to C3::Componentised can warn developers who 
are using it immediately in their apps.. but it doesn't help developers 
who are working one step removed. Eg. I use DBIx::Class, but I haven't 
thoroughly read the documentation for every single one of its dependency 
tree.
I just think it's good practice to avoid using functions that have 
potential bad surprises..

But hey, look, I don't know the bigger picture, and don't want to start 
a storm in a teacup over this; so if the problem is known about and 
workarounds already being put in place, I'm happy that the right thing 
is being done.

>> I freely admit I don't understand why one would use the
>> "check-but-don't-load" functionality often, especially since the case in
>> point that breaks the deployment will always attempt to load the class
>> immediately if it is found:
>>
> 
> If someone put it there, someone does use it.

Although the existence, or popularity, of something doesn't make it 
correct.. Remember Matt's Script Archive? :)

> In short:
> 
> 1) DBIC was fixed last week - applied patch is here:
> http://dev.catalyst.perl.org/svnweb/bast/revision/?rev=5355
> 
> 2) Raising bugs against C3 is not productive. Much better to supply a POD
> patch against Class::C3::Componentised, explaining that ensure_class_found()
> will not work with PAR environments and anything that stuffs coderefs in @INC.

As above, I disagree that the bug is not productive. It's a valid issue, 
and you must agree that the module would be better if it did not have 
the issue?

However I'm writing up the POD patch as we speak, and will submit that 
as well. Thanks for the suggestion.

Cheers,
Toby