Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
djerius opened this issue Jan 26, 2015 · 14 comments
Open

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

djerius opened this issue Jan 26, 2015 · 14 comments

Comments

@djerius
Copy link
Contributor

djerius commented Jan 26, 2015

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.
@xsawyerx
Copy link
Owner

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

Thank you for reporting the issue!

@xsawyerx
Copy link
Owner

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

@chrestomanci
Copy link
Contributor

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

chrestomanci pushed a commit to chrestomanci/module-starter that referenced this issue Jan 27, 2015
chrestomanci pushed a commit to chrestomanci/module-starter that referenced this issue Jan 27, 2015
NB: I cannot reproduce the bug report, so I don't know for sure if it fixes the bug.
@chrestomanci
Copy link
Contributor

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?

@djerius
Copy link
Contributor Author

djerius commented Jan 29, 2015

@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
Copy link
Contributor Author

djerius commented Jan 29, 2015

@chrestomanci,

I've tried your branch and it works.

Thanks,
Diab

@chrestomanci
Copy link
Contributor

@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?

@xsawyerx
Copy link
Owner

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

xsawyerx added a commit that referenced this issue Jan 30, 2015
Attempt to fix issue #47 with boilerplate creation
@xsawyerx
Copy link
Owner

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

@djerius
Copy link
Contributor Author

djerius commented Feb 3, 2015

Thanks!

@powerman
Copy link

powerman commented Feb 4, 2015

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

@xsawyerx
Copy link
Owner

xsawyerx commented Feb 6, 2015

ugh

@xsawyerx xsawyerx reopened this Feb 6, 2015
@chrestomanci
Copy link
Contributor

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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants