module-starter icon indicating copy to clipboard operation
module-starter copied to clipboard

Module::Starter::Simple::create_t in 1.70 breaks plugins

Open djerius opened this issue 10 years ago • 14 comments

Prior to 1.70, Module::Starter::Simple::create_t expected the t_guts method to return a hash (as a list). Now it expects that t_guts return two hashrefs. This breaks all plugins with a t_guts method (including Module::Starter's own Module::Starter::Plugin::Template).

For example, here's my plugins configuration:

plugins: Module::Starter::Simple Module::Starter::Plugin::Template Module::Starter::Plugin::DirStore Module::Starter::Plugin::TT2

I'm now seeing the following error:

% module-starter --module App::acisraw --mi
Can't use string ("perlcritic.t") as a HASH ref while "strict refs" in use at [...]/Module/Starter/Simple.pm line 1086.

djerius avatar Jan 26 '15 19:01 djerius

Oh my... we'll have to fix it right away.

Thank you for reporting the issue!

xsawyerx avatar Jan 26 '15 21:01 xsawyerx

@chrestomanci Do you happen to have time (and care) to take a look at it? You probably know that change better than anyone. :)

xsawyerx avatar Jan 26 '15 21:01 xsawyerx

I will take a look. That was my change, and I did not realise that any downstream modules where relying on that API.

chrestomanci avatar Jan 27 '15 08:01 chrestomanci

I can't reproduce the reported problem, as I don't have enough information to reproduce the reporter's setup.

I have pushed a fix which I think should fix things, but I can't be sure.

@djerius are you able to test my branch?

chrestomanci avatar Jan 27 '15 18:01 chrestomanci

@chrestomanci,

Sorry for not initially providing an example.

Since GitHub has no means of uploading files to issues, I stuck a shar file ( haven't used shar since usenet days!) of an example setup in a gist:

https://gist.github.com/djerius/95dd8fe5098d93a33029

It's a bash script which when run recreates the files.

The configuration requires Module::Starter::Plugin::DirStore and Module::Starter::Plugin::TT2. Here's the command line:

% MODULE_TEMPLATE_DIR=module-starter/MST MODULE_STARTER_DIR=module-starter module-starter --module Foo --mi Can't use string ("perlcritic.t") as a HASH ref while "strict refs" in use at [...]/lib/site_perl/5.16.3/Module/Starter/Simple.pm line 1086.

I'll try your branch

djerius avatar Jan 29 '15 16:01 djerius

@chrestomanci,

I've tried your branch and it works.

Thanks, Diab

djerius avatar Jan 29 '15 16:01 djerius

@djerius

Thanks for your code example. I can now see what is going on, so I am confident my fix is safe.

@xsawyerx are you able to merge the fix and do another release?

chrestomanci avatar Jan 29 '15 21:01 chrestomanci

Yup. Already on my task list for tomorrow. :)

xsawyerx avatar Jan 29 '15 21:01 xsawyerx

Released with fix! @djerius++ @chrestomanci++

xsawyerx avatar Jan 30 '15 12:01 xsawyerx

Thanks!

djerius avatar Feb 03 '15 15:02 djerius

Looks like this fix is incomplete - plugin Module::Starter::CSJEWELL is still broken (I'm trying to use it in default configuration):

Can't create Example-MBtiny/t: Is a directory
 at /usr/lib64/perl5/vendor_perl/5.20.1/Module/Starter/Simple.pm line 1581.
    Module::Starter::Simple::create_file(Module::Starter=HASH(0xfc403e68ad0), "Example-MBtiny/t", "xt/author/meta.t") called at /usr/lib64/perl5/vendor_perl/5.20.1/Module/Starter/CSJEWELL.pm line 134
    Module::Starter::CSJEWELL::_create_t(Module::Starter=HASH(0xfc403e68ad0), "t", "xt/author/meta.t", "#!perl\x{a}\x{a}# Test that our META.yml file matches the specificati"...) called at /usr/lib64/perl5/vendor_perl/5.20.1/Module/Starter/Simple.pm line 1087
    Module::Starter::Simple::create_t(Module::Starter=HASH(0xfc403e68ad0), "Example::MBtiny") called at /usr/lib64/perl5/vendor_perl/5.20.1/Module/Starter/Simple.pm line 127
    Module::Starter::Simple::create_distro(Module::Starter=HASH(0xfc403e68ad0)) called at /usr/lib64/perl5/vendor_perl/5.20.1/Module/Starter/App.pm line 134
    Module::Starter::App::run("Module::Starter::App") called at /usr/bin/module-starter line 17

powerman avatar Feb 04 '15 01:02 powerman

ugh

xsawyerx avatar Feb 06 '15 16:02 xsawyerx

ugh indeed.

At this point, I don't realy understand why these downstream modules are now broken. It looks like they are relying on undocumented behavor in Module::Starter::Simple, but I am not certain.

The overall change relative to the 1.62 release is to add an xt_guts() method to Module::Starter::Simple, and move the code that generates boilerplate.t from t_guts() to it, so it looks like a downsteam module was relying on t_guts() to return boilerplate.t.

At this point, the simplest fix to get the downsteam modules working is to just revert the offending change to how boilerplate.t is generated. This will re-open bug #28. I can create a pull request that does that if you like.

Alternativlely, if I am correct that these downsteam modules are relying on undocumented behavor, then we could just use this as an illustration as why it is a good idea to only use documented behavor. I would want to be 100% sure of my case before taking this approach.

In any case, you @xsawyerx are the maintaner, and I am just a guest here from the CPAN challenge, so what approach would you prefer?

chrestomanci avatar Feb 08 '15 18:02 chrestomanci

I have crated a PR to revert my boilerplate.t changes that cause all this trouble. It should fix this bug, but will of course re-open the bugs it was intended to fix.

chrestomanci avatar Feb 15 '15 17:02 chrestomanci