Skip Menu |

This queue is for tickets about the App-CSV CPAN distribution.

Report information
The Basics
Id: 95861
Status: resolved
Priority: 0/
Queue: App-CSV

People
Owner: GAAL [...] cpan.org
Requestors: SREZIC [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: 0.07
Fixed in: (no value)



Subject: Tests are using Text::CSV, but only Text::CSV_XS is a prereq
See http://matrix.cpantesters.org/?dist=App-CSV+0.07 . The fails in the darwin + freebsd columns seem to happen because only Text::CSV_XS is declared as a prereq, but tests are using Text::CSV. Probably it's best to rewrite the tests to use Text::CSV_XS, too. Regards, Slaven
On 2014-05-22 06:04:05, SREZIC wrote: Show quoted text
> See http://matrix.cpantesters.org/?dist=App-CSV+0.07 . The fails in > the darwin + freebsd columns seem to happen because only Text::CSV_XS > is declared as a prereq, but tests are using Text::CSV. Probably it's > best to rewrite the tests to use Text::CSV_XS, too. > > Regards, > Slaven
Or add Text::CSV as a required prereq, and Text::CSV_XS as a recommended prereq that changes to required if a compiler is present?
RT-Send-CC: ether [...] cpan.org
On Thu May 22 11:29:04 2014, ETHER wrote: Show quoted text
> On 2014-05-22 06:04:05, SREZIC wrote:
> > See http://matrix.cpantesters.org/?dist=App-CSV+0.07 . The fails in > > the darwin + freebsd columns seem to happen because only Text::CSV_XS > > is declared as a prereq, but tests are using Text::CSV. Probably it's > > best to rewrite the tests to use Text::CSV_XS, too. > > > > Regards, > > Slaven
> > > Or add Text::CSV as a required prereq, and Text::CSV_XS as a > recommended prereq that changes to required if a compiler is present?
These two packages have compatible functionality and Text::CSV can use either. I'm not really sure how to formally express what I want in the meta files, which is to depend on at least one and at runtime load the best(*) one of them. Is there a clear way of doing it you can suggest? (*) https://metacpan.org/release/Best
RT-Send-CC: ether [...] cpan.org
On Thu May 22 11:37:48 2014, GAAL wrote: Show quoted text
> On Thu May 22 11:29:04 2014, ETHER wrote:
> > On 2014-05-22 06:04:05, SREZIC wrote:
> > > See http://matrix.cpantesters.org/?dist=App-CSV+0.07 . The fails in > > > the darwin + freebsd columns seem to happen because only > > > Text::CSV_XS > > > is declared as a prereq, but tests are using Text::CSV. Probably > > > it's > > > best to rewrite the tests to use Text::CSV_XS, too. > > > > > > Regards, > > > Slaven
> > > > > > Or add Text::CSV as a required prereq, and Text::CSV_XS as a > > recommended prereq that changes to required if a compiler is present?
> > These two packages have compatible functionality and Text::CSV can use > either. I'm not really sure how to formally express what I want in the > meta files, which is to depend on at least one and at runtime load the > best(*) one of them. Is there a clear way of doing it you can suggest? > > (*) https://metacpan.org/release/Best
I meant, of course, that App::CSV can use either module. The Makefile.PL is somewhat buggy; it means that when the maintainer creates meta files one dependency will be recorded depending on what's installed in the maintainer's system at the time. I'm not sure how to address that. Now I'm looking more carefully and I see that the code itself doesn't actually use Best.pm, it just uses Text::CSV. That would explain the bug reports. Is it reasonable to assume pretty much everybody has Text::CSV_XS these days? Stopping to try to be clever might be the best way to fix this. It takes care of the metadata problem too.
Subject: Re: [rt.cpan.org #95861] Tests are using Text::CSV, but only Text::CSV_XS is a prereq
Date: Thu, 22 May 2014 09:12:42 -0700
To: GAAL via RT <bug-App-CSV [...] rt.cpan.org>
From: Karen Etheridge <ether [...] cpan.org>
On Thu, May 22, 2014 at 11:55:35AM -0400, GAAL via RT wrote: Show quoted text
> The Makefile.PL is somewhat buggy; it means that when the maintainer creates meta files one dependency will be recorded depending on what's installed in the maintainer's system at the time. I'm not sure how to address that.
Generate the META.* files separately, instead of letting ExtUtils::MakeMaker do it. One common way of doing that is with Dist::Zilla. Show quoted text
> Is it reasonable to assume pretty much everybody has Text::CSV_XS these days? Stopping to try to be clever might be the best way to fix this. It takes care of the metadata problem too.
You can add it as a required prereq, which will work for everyone except those with no compiler, or who want to fatpack your module. One option is to do this and wait for the bug report from someone who cares about this (it may never happen). Or, do this in Makefile.PL: $WriteMakefileArgs{PREREQ_PM}{'Text::CSV_XS'} = '0' if can_xs(); sub can_xs { return eval 'require ExtUtils::CBuilder; 1' && ExtUtils::CBuilder->new( quiet => 1 )->have_compiler; } ..and also add ExtUtils::CBuilder to {prereqs}{configure}{requires} metadata. Note you CANNOT use META_MERGE in WriteMakefile() for this to work. You need to either use META_ADD, or generate your meta files independently..
On 2014-05-22 11:37:48, GAAL wrote: Show quoted text
> On Thu May 22 11:29:04 2014, ETHER wrote:
> > On 2014-05-22 06:04:05, SREZIC wrote:
> > > See http://matrix.cpantesters.org/?dist=App-CSV+0.07 . The fails in > > > the darwin + freebsd columns seem to happen because only > > > Text::CSV_XS > > > is declared as a prereq, but tests are using Text::CSV. Probably > > > it's > > > best to rewrite the tests to use Text::CSV_XS, too. > > > > > > Regards, > > > Slaven
> > > > > > Or add Text::CSV as a required prereq, and Text::CSV_XS as a > > recommended prereq that changes to required if a compiler is present?
> > These two packages have compatible functionality and Text::CSV can use > either. I'm not really sure how to formally express what I want in the > meta files, which is to depend on at least one and at runtime load the > best(*) one of them. Is there a clear way of doing it you can suggest? > > (*) https://metacpan.org/release/Best
My recommendation: if the user has any of the Text::CSV* modules already installed, then stick to this, don't install another one, and use the available one for testing. For instance you can do something like this in the test: my $Text_CSV_module; BEGIN { if (eval { require Text::CSV_XS; 1 }) { $Text_CSV_module = 'Text::CSV_XS'; } else { require Text::CSV; $Text_CSV_module = 'Text::CSV'; } } # later: my $csv = $Text_CSV_module->new(...); In the Makefile.PL do something similar to determine whether to include Text::CSV_XS or Text::CSV in the PREREQ_PM, and fallback to one of both if neither is installed (maybe using the use_xs() recommendation by ETHER...) Regards, Slaven
RT-Send-CC: ether [...] cpan.org
0.08 uploaded to PAUSE. Thanks for the report.