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

If Text::CSV_XS installed, require compatible version #49

Merged
merged 1 commit into from
Jun 19, 2021
Merged

If Text::CSV_XS installed, require compatible version #49

merged 1 commit into from
Jun 19, 2021

Conversation

mohawk2
Copy link
Contributor

@mohawk2 mohawk2 commented Feb 1, 2021

@charsbar
Copy link
Collaborator

charsbar commented Feb 1, 2021

I'd rather just bump up the $XS_Version in Text::CSV as the XS module itself may have been installed via an OS packager (and in that case essential files to build (gcc, make etc) may not have been installed), but I'm not yet sure to which version. According to the Changes file, 1.39 may be a good candidate, but it may be too high for the OS package users.

By the way, putting BEGIN { $ENV{PERL_TEXT_CSV} = 0 } at the top of your test (to force Text::CSV to use the bundled Text::CSV_PP) should solve your problem.

@mohawk2
Copy link
Contributor Author

mohawk2 commented Feb 2, 2021

My problem is that my module relies on your module providing a csv function, which it is documented as providing (it's making a promise). Your module is currently failing to deliver on that promise if an old version of CSV_XS is there. If you want to increase the $XS_Version, and your module then successfully delivers on its promise, that will be fine. I don't see it as valid to ask your module's users to set environment variables so that it will work correctly.

@charsbar charsbar merged commit 6d888b0 into makamaka:master Jun 19, 2021
@charsbar
Copy link
Collaborator

Sorry for being late. Now Text::CSV_PP passes all the tests of Text::CSV_XS 1.46, so it should be safe to update required XS version to 1.45 (actually 1.46). Merged. Thank you.

@mohawk2
Copy link
Contributor Author

mohawk2 commented Jun 25, 2021

Thanks!

@mohawk2 mohawk2 deleted the patch-1 branch June 25, 2021 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants