search

mschout / perl-text-template

Expand template text with embedded Perl
13 stars 6 forks source link

something broke badly between 1.47 and 1.50 #9

Closed karenetheridge closed 6 years ago

karenetheridge commented 6 years ago

Dist::Zilla code that uses templates is now broken with TT 1.50, but work with 1.47.

Here's an example, from travis: https://travis-ci.org/karenetheridge/Dist-Zilla-PluginBundle-Author-ETHER/jobs/348582003

I'm going to bisect to the exact version momentarily. The problem exists when upgrading from 1.47 to 1.48.

karenetheridge commented 6 years ago

results of git bisect (the tags weren't on the master branch, so I had to use commit comments):

c5bbd6df0f74398a4c584a0daaeb533f8d234574 is the first bad commit
commit c5bbd6df0f74398a4c584a0daaeb533f8d234574
Author: Mohammad S Anwar <mohammad.anwar@yahoo.com>
Date:   Sun Jul 30 10:05:42 2017 +0100

    - Added strict/warnings pragma as reported by CPANTS.

:040000 040000 be5637d6589324db6681ddbcbe0b6f1208e8f0cb a57e5162fad2b4d95f5538c78983eab763b239e5 M  lib

This is an important change to note for users of this module: their code is now being executed with strict+warnings turned on for the context of their code.

karenetheridge commented 6 years ago

I fixed my test failures in Dist-Zilla-PluginBundle-Author-ETHER-0.135 -- they were real problems with the template code I was executing, that went unnoticed because there were no visible warnings and I wasn't testing the bits of the template content that were failing.

I think the only thing to do in Text::Template is to document this change in the 1.48 release (and be aware that adding strict+warnings is not always a trivial change).

mschout commented 6 years ago

I'm going to turn off strict+warnings in the sections of Text::Template where it eval's the template code (except for the case where STRICT => 1 was passed to fill_in, because as you note, turning warnings on isn't trivial necesarily. The intent of the commit that caused this was to make Text::Template itself safe with strict+warnings. But the actual template code doesn't need to be (unless STRICT => 1 is passed to fill_in()). So this bug seems like an unintended change in behaviour IMO.

karenetheridge commented 6 years ago

I didn't realize STRICT was an option for fill_in. Was that recently added? (it's also not listed in Changes.)

mschout commented 6 years ago

It was added in v1.48. Its in the changes there:

- Add "strict" option to fill_in().  This adds "use strict" and "use vars
  (...)" to the prepend section, and only the keys of the HASH option are
  allowed in the template.  (Thanks Desmond Daignault, Kivanc Yazan, CJM)
  [55696]
karenetheridge commented 6 years ago

ah I see, I was searching for 'STRICT'.