Pin App::Cmd::Setup to 0.331 or less.

[cromedome]

rjbs informed me that the latest version of App::Cmd would be supporting only Perl 5.24 and beyond, but later backed off to 5.20. This still doesn't get us Perl 5.10 support though, so pinning Dancer2 to v0.331 at least gets us installable on older versions of Perl.

Going forward, we should move to something like MooX::Options or CLI::Osprey.

[veryrusty]

I think this is a slippery slope. If you start with a machine with Perl 5.24+, install App::Cmd from cpan, then try and install Dancer2 with this applied, one will also hit dependency version problems.

We almost want a dynamic prereq.., only for perl < 5.20 require App::Cmd 0.331 or less.

[cromedome]

I agree. I don't know how to write the cpanfile in such a way to do what you describe though. And if I have to pick a direction to go, I'd rather deal with the situation you describe than the one this PR addresses.

Ultimate solution is to replace App::Cmd. This is only intended to be a stop gap. But we should do something fast.

[cromedome]

I've confirmed we cannot do this in our cpanfile. We can either plugin to our build process to address this (so, hacking Dist::Zilla essentially) or deal with a less than ideal situation for a short time while we scramble to rewrite the CLI. The CLI is not very extensive, and personally, I'd rather spend my time improving the CLI than beating on dzil. Thoughts?

[racke]

As we only use dancer2 to create new applications I don't see the point to make provisions to support ancient Perl versions for this script.

So I disagree with pinning App::Cmd.

[cromedome]

While I am not a fan of supporting everything back to 5.10, I do not think this is the way to stop supporting old versions of Perl. With the App::Cmd release, we cannot install Dancer2 on Perls older than 5.20 without changing something. And I think that would be a giant 🖕 to some part of our user base.

If we want to stop supporting old versions of Perl, that's a separate discussion, and one I'd be excited to have. But this is not the way.

I am open to alternative solutions here. This is the best I have for now.

[bigpresh]

Whilst I like supporting older perls where it's practical to do and doesn't cost us much effort, it's not unreasonable to consider dropping support for 5.10 and going for more modern versions. As a data point, at work we have some CentOS 7 boxes, which have 5.16 - and that's an old CentOS version. 5.10 is far more ancient.

I agree, though, that we shouldn't be failing on older perls just because a dependency of ours has decided to pull support for older perls - our minimum supported version should be something we decide and communicate clearly to users, with a "Soon, Dancer2 will require a minimum perl version of 5.XX, we will no longer be able to support older perls in newer Dancer2 releases" etc.

Could be be hacky and just bundle App::Cmd 0.331 with Dancer2 for the time being (renamed to e.g. Dancer2::App::Cmd and excluded from CPAN indexing), until we decide on the better approach?

[cromedome]

I am not opposed to any of your points, @bigpresh. I would need some help and a little guidance in making it happen. What do the other Dancers think about this option?

I do think the best solution is to get off of App::Cmd as requested in #1319.

[racke]

A quick solution could be to demote the prerequisite from requires to recommends until we have a replacement for App::Cmd. This would fix the install, correct?

[SysPete]

I've played all sorts of games to try to pin versions based on $], but we also have to downgrade Getopt::Long::Descriptive to 0.105 (one of the deps of App::Cmd), and I just can't get both to kick in in the correct order. Time to kill App::Cmd::Setup.

[SysPete]

Whilst I like supporting older perls where it's practical to do and doesn't cost us much effort, it's not unreasonable to consider dropping support for 5.10 and going for more modern versions. As a data point, at work we have some CentOS 7 boxes, which have 5.16 - and that's an old CentOS version. 5.10 is far more ancient.

I agree, though, that we shouldn't be failing on older perls just because a dependency of ours has decided to pull support for older perls - our minimum supported version should be something we decide and communicate clearly to users, with a "Soon, Dancer2 will require a minimum perl version of 5.XX, we will no longer be able to support older perls in newer Dancer2 releases" etc.

Could be be hacky and just bundle App::Cmd 0.331 with Dancer2 for the time being (renamed to e.g. Dancer2::App::Cmd and excluded from CPAN indexing), until we decide on the better approach?

:+1:

We might also need to bundle Getopt::Long::Descriptive as per earlier comment.

[SysPete]

Or perhaps we should just go with @racke's suggestion and get to work on removing App::Cmd?

[bigpresh]

Or perhaps we should just go with @racke's suggestion and get to work on removing App::Cmd?

Oh, I agree that pulling out App::Cmd is the better long term solution, but we may need a quick-win band-aid solution so that Dancer is installable on older Perls again for the time being until we've had time to do that (unless it turns out to be nice and easy).

Another option could be to remove the dependency and have bin/dancer2 check for it at runtime, and if not found, show a "You need to install App::Cmd to use this script" - not ideal, but more ideal than Dancer2 not being installable at all.

If I get a chance tonight after the kids in bed I may find a moment to have a poke, unless anyone else has in the meantime.

[cromedome]

@bigpresh: @SysPete and I discussed that a little bit earlier but I hadn't got to documenting it yet. If you get to it tonight, ping me so I don't start on it too :)

[bigpresh]

PR #1603 raised, which appears to work, with some testing (using Devel::Hide to hide App::Cmd from it to make sure that it's using the bundled version). Please do review and make sure I've not missed anything!

[bigpresh]

And PR #1604 raised for just removing the dependency and having the dancer2 script check at runtime for App::Cmd and tell you to install it if you don't already have it. Now we have PRs for all three approaches we've discussed, we need to pick one and go with it :)

[cromedome]

Closing in favor of #1604.