Skip Menu |
 

This queue is for tickets about the Catalyst-View-TT CPAN distribution.

Report information
The Basics
Id: 44717
Status: rejected
Priority: 0/
Queue: Catalyst-View-TT

People
Owner: Nobody in particular
Requestors: daniel [...] rimspace.net
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)



Subject: The use of local in Catalyst::View::TT->render is unreliable.
Date: Wed, 01 Apr 2009 17:27:30 +1100
To: bug-Catalyst-View-TT [...] rt.cpan.org
From: Daniel Pittman <daniel [...] rimspace.net>
Download (untitled) / with headers
text/plain 2.6k
G'day. I was trying to use the documented 'additional_template_paths' feature of a view derived from Catalyst::View::TT, and the additional paths were not being used. After quite some debugging I discovered that this is because the code in render is broken: local $self->{include_path} = [ @{ $vars->{additional_template_paths} }, @{ $self->{include_path} } ] if ref $vars->{additional_template_paths}; The value '$self->{include_path}' is an ARRAYREF that was passed to the Template::Toolkit object as the search path. The code above tries to modify that using 'local' to preserve changes. Unfortunately, that doesn't actually work: local creates a new ARRAYREF, containing the alternate values, and restores that — rather than modify the content of the array and restore it. This is reproducible with both: This is perl, v5.10.0 built for x86_64-linux-gnu-thread-multi This is perl, v5.8.8 built for i486-linux-gnu-thread-multi Both are from Debian, but I don't imagine that makes a great deal of difference. I have attached a little test script that demonstrates this, although you can easily reproduce the same results yourself. To remedy this I did a little poking, discovered (as you probably did) that the interface to modifying the include path for a Template::Toolkit instance is insanely horrible, and went back to mutating the array. This code works: sub render { my $self = shift; # for calling next... my $c = $_[0]; # because I need to access it. my @old; # This is very, very ugly. Not as ugly as using the API, and at least it # works, eh, without depending on quirks of the Perl implementation. if (exists $c->stash->{additional_template_paths}) { # Preserve the old content. @old = @{ $self->{include_path} || [] }; # Set the new content without changing the array location. @{ $self->{include_path} } = (@{ $c->stash->{additional_template_paths} }, @{ $self->{include_path} }); } my $rval = $self->NEXT::render(@_); if (exists $c->stash->{additional_template_paths}) { # Yay, restore the previous path value. @{ $self->{include_path} } = @old; } return $rval; } Clearly, not as nice as your code, but it has the benefit of actually working robustly.[1] Regards, Daniel Footnotes: [1] This will fail if we have multiple threads accessing the same template object instance, but I think other parts of Catalyst would fail there first, *AND* the existing code should suffer the same weakness. If one the Template::Toolkit API wasn't so awful in the area, eh?
Download bug.t
text/x-perl 379b

Message body is not shown because sender requested not to inline it.

Download (untitled) / with headers
text/plain 839b
I am marking this bug as rejected for the following reasons: 1) Your bug.t file doesn't test what you think it tests. To quote perlsub: "A local modifies its listed variables to be "local" to the enclosing block, eval, or do FILE --and to any subroutine called from within that block." Your test checks the memory address in the same scope, so of course it will be different. I've attached a test which more closely matches how local is being used in our context. 2) There are existing tests for this feature which pass. c.f. t/06includepath.t -- it seems to check the initial include path and then the result after a request that modifies it. You may wish to examine that part of the test suite closer in case we've missed something. Thank you for your time. Feel free to re-open this ticket if you have any new information to add.
Download local.t
text/x-perl 626b
use Test::More tests => 4; use strict; use warnings; my @first = ( 1, 2 ); my @second = ( 6, 7, 1, 2 ); { my $t = { arrayref => [1, 2] }; is_deeply( $t->{arrayref}, \@first, 'array content: 1, 2' ); my $first = "$t->{arrayref}"; # stringify to get the address { local $t->{arrayref} = [ 6, 7, @{ $t->{arrayref} } ]; is_deeply( $t->{arrayref}, \@second, 'array content: 6, 7, 1, 2' ); } my $second = "$t->{arrayref}"; # stringify to get the address is_deeply( $t->{arrayref}, \@first, 'array content: 1, 2' ); is($first, $second, "Both arrays were at the same address"); }


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.