kazeburo / WWW-Form-UrlEncoded

parser for application/x-www-form-urlencoded
Other
0 stars 6 forks source link

Should not warn if XS is not new enough #3

Open eserte opened 7 years ago

eserte commented 7 years ago

Currently there's a warning if WWW::Form::UrlEncoded::XS is older than WWW::Form::UrlEncoded:

$ perl5.20.2 -MWWW::Form::UrlEncoded -e1            
WWW::Form::UrlEncoded::XS 0.23 is require. fallback to PP version at /usr/perl5.20.2p/lib/site_perl/5.20.2/amd64-freebsd/WWW/Form/UrlEncoded.pm line 17.

However, if WWW::Form::UrlEncoded::XS is not installed at all, then there's no warning. So it seems that the XS is not really needed to work properly, and so there should be no warning if it's too old. Or the warning could be optionally triggered, maybe using a new environment variable.

The current behavior may cause failures in other modules' test suites, especially if something like Test::NoWarnings is used there. Currently I see such test failures for HORNBURG/Dancer2-Plugin-Auth-Extensible-0.621.tar.gz (@racke: FYI)

kazeburo commented 7 years ago

That warning is required to prevent from unexpected performance degradation I think.

How about

warn "..failback to PP version" if ! exists $INC{"Test/NoWarnings.pm"}
eserte commented 7 years ago

A possible workaround, it could work in the particular case. However, there may be other modules which do the same like Test::NoWarnings, or users may simply build a custom check with $SIG{__WARN__} in their test suites.

I wonder if there are other possibilities to make sure that the XS in the current version is there. E.g. WWW::Form::UrlEncode's Makefile.PL could check if the XS is already is installed, and if it is, then add the XS dynamically to the PREREQ_PM list with the appropriate version. So it's made sure that the XS is uptodate after a WWW::Form::UrlEncode upgrade.