plack / Plack

PSGI toolkit and server adapters
http://plackperl.org/
Other
486 stars 214 forks source link

Maybe bump minimum Perl version #691

Closed oalders closed 1 year ago

oalders commented 1 year ago

MetaCPAN lists 5.8.1 as the minimum version, but I can't install Plack::Test on 5.10. Note that this actually doesn't bother me, but I mention it because I came across this in some Docker builds.

--> Working on HTTP::Entity::Parser
Fetching http://www.cpan.org/authors/id/K/KA/KAZEBURO/HTTP-Entity-Parser-0.25.tar.gz ... OK
Configuring HTTP-Entity-Parser-0.25 ... OK
==> Found dependencies: HTTP::MultiPartParser, JSON::MaybeXS
--> Working on HTTP::MultiPartParser
Fetching http://www.cpan.org/authors/id/C/CH/CHANSEN/HTTP-MultiPartParser-0.02.tar.gz ... OK
Configuring HTTP-MultiPartParser-0.02 ... OK
==> Found dependencies: Test::Deep
--> Working on Test::Deep
Fetching http://www.cpan.org/authors/id/R/RJ/RJBS/Test-Deep-1.204.tar.gz ... OK
Configuring Test-Deep-1.204 ... N/A
! Configure failed for Test-Deep-1.204. See /root/.cpanm/work/1673300825.7/build.log for details.
! Installing the dependencies failed: Module 'Test::Deep' is not installed
! Bailing out the installation for HTTP-MultiPartParser-0.02.

I believe docker run --rm -it perl:5.10-buster bash -c "cpanm Plack::Test" should replicate this.

I've worked around this via requires 'Test::Deep', '==1.130'; in my cpanfile.

miyagawa commented 1 year ago

I guess that's because @rjbs has been updating the minimum for his modules https://rjbs.cloud/blog/2023/01/leaving-perl-v5.8-behind/ and Test::Deep is in the transit dependencies.

I don't mind bumping Plack's minimum to 5.10, but also don't mind if whoever needs to run production box on 5.8 need to deal with this by themselves by pinning the module to a specific version. Though the CI on this repo probably falls into that category, ironically.

Because Plack by itself doesn't depend on Test::Deep, it doesn't feel right to upgrade the minimum because at some point Test::Deep could support 5.8 again, or HTTP::MultiPartParser can remove that dependency, and none of those decisions should affect whether Plack should support 5.8.

By the way, if HTTP::MultiPartParser correctly identifies Test::Deep as test dependency, cpanm --notest would skip installing the module since it's not required when --notest is specified, but it's not done that way.

haarg commented 1 year ago

I've already filed chansen/p5-http-multipartparser#3, although chansen doesn't seem to be around anymore.

oalders commented 1 year ago

none of those decisions should affect whether Plack should support 5.8.

That's certainly fair and in that case, I think we can close this. I just wanted to note the issue. If anyone needs a solution for 5.8 in the meantime, it's documented here.