Spec URL: http://people.redhat.com/mgoodwin/cpan/perl-Spreadsheet-read/perl-Spreadsheet-Read.spec SRPM URL: http://people.redhat.com/mgoodwin/cpan/perl-Spreadsheet-read/perl-Spreadsheet-Read-0.45-1.fc15.src.rpm Description: Spreadsheet::Read tries to transparently read *any* spreadsheet and return its content in a universal manner independent of the parsing module that does the actual spreadsheet scanning. It supports spreadsheets handled by Text::CSV_XS, Spreadsheet::ParseExcel, Spreadsheet::XLSX and Spreadsheet::ReadSXC (note: XLSX and ReadSXC are not yet in Fedora. Depending how this review request goes, I can add them too). The cpanspec utility was used to download the tarball from cpan and generate the spec. This request to add Spreadsheet:read is to satisfy a missing dependency for pcp-import-sheet2pcp, the new version of which is currently in updates-testing (see BZ 754678). Have built this for {el4,el5,el6,f15,f16,f17}-candidate and tested on f15.
[Although I know a bit of Perl, I'm not a Perl packager, so I'm not up-to-date on Fedora Perl Packaging]. * A plain src.rpm rebuild is interactive here: $ rpmbuild --rebuild perl-Spreadsheet-Read-0.45-1.fc15.src.rpm [...] + cd Spreadsheet-Read-0.45 + LANG=C + export LANG + unset DISPLAY + /usr/bin/perl Makefile.PL INSTALLDIRS=vendor Do you want to install 'xlscat' (Convert Spreadsheet to plain text or CSV) ? [n] That's a blocker. Several more setup questions are asked. This ought to be automatic with a default build config being applied. * There's a test-suite included, suitable for a %check section. "make test" does something, but requires _at least_ perl-Test-Simple perl-Test-NoWarnings and then still warns about further build requirements as well as skipped tests.
(In reply to comment #1) thanks for looking at this > [Although I know a bit of Perl, I'm not a Perl packager, so I'm not up-to-date > on Fedora Perl Packaging]. I'm not either - this probably should be reviewed by someone who is. > * A plain src.rpm rebuild is interactive here: > .. > + /usr/bin/perl Makefile.PL INSTALLDIRS=vendor > Do you want to install 'xlscat' (Convert Spreadsheet to plain text or CSV) ? > [n] > > That's a blocker. Several more setup questions are asked. This ought to be > automatic with a default build config being applied. My koji builds didn't stall or ask any questions - they just worked, presumably using default answers. Reading Makefile.PL, it will not ask questions if AUTOMATED_TESTING=1 in the environment, so I've made that change in the spec (see diff below). > * There's a test-suite included, suitable for a %check section. "make test" > does something, but requires _at least_ perl-Test-Simple perl-Test-NoWarnings > and then still warns about further build requirements as well as skipped tests. ok, I added a few more build dependencies for the testing and added a %check section. Seems to work, and tolerate any missing spreadsheet parsers. I've copied the new spec and srpm up to the same location. Here's the diff : --- old/perl-Spreadsheet-Read.spec 2012-01-11 19:47:36.884021000 -0500 +++ perl-Spreadsheet-Read.spec 2012-01-12 16:16:58.166961000 -0500 @@ -11,8 +11,11 @@ BuildRequires: perl >= 0:5.006 BuildRequires: perl(ExtUtils::MakeMaker) BuildRequires: perl(IO::Scalar) +BuildRequires: perl(Test::NoWarnings) +BuildRequires: perl(Test::More) >= 0.98 Requires: perl(IO::Scalar) +Requires: perl(Test::More) >= 0.98 Requires: perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version)) %description @@ -24,7 +27,7 @@ %setup -q -n Spreadsheet-Read-%{version} %build -%{__perl} Makefile.PL INSTALLDIRS=vendor +AUTOMATED_TESTING=1 %{__perl} Makefile.PL INSTALLDIRS=vendor make %{?_smp_mflags} %install @@ -37,6 +40,9 @@ %{_fixperms} $RPM_BUILD_ROOT/* +%check +make test + %clean rm -rf $RPM_BUILD_ROOT
> AUTOMATED_TESTING=1 Confirmed. No questions about the example scripts are asked anymore. > Requires: perl(IO::Scalar) > +Requires: perl(Test::More) >= 0.98 http://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires Even if it may seem that it is specific to libraries only, it covers explicit Requires in general. Comments in the spec file, which explain the explicit Requires, are good packaging practice. * Please keep the spec %changelog accurate. Your updates to the spec invalidated the only %changelog entry about cpanspec usage. * A few more packages for the tests are available in Fedora already, btw, just in case you consider the tests worthwhile: perl-Text-CSV perl-Text-CSV_XS perl-Test-Pod perl-Test-Pod-Coverage perl-Spreadsheet-ParseExcel t/00_pod.t ..... ok t/01_pod.t ..... ok t/10_basics.t .. ok t/11_call.t .... ok t/20_csv.t ..... # Parser: Text::CSV_XS-0.82 t/20_csv.t ..... ok t/21_csv.t ..... ok t/22_csv.t ..... ok t/23_csv.t ..... ok t/24_csv.t ..... ok t/30_xls.t ..... # Parser: Spreadsheet::ParseExcel-0.59 t/30_xls.t ..... ok t/31_clr.t ..... ok t/32_fmt.t ..... ok t/33_misc.t .... ok t/34_dates.t ... ok t/35_perc.t .... ok t/36_xls.t ..... ok t/40_sxc.t ..... skipped: No SXC parser found t/45_ods.t ..... skipped: No SXC parser found t/46_clr.t ..... skipped: No OpenOffice ODS parser found t/50_sc.t ...... # Parser: Spreadsheet::Read-0.45 t/50_sc.t ...... ok t/51_sc.t ...... ok t/60_xlsx.t .... skipped: No M$-Excel parser found t/61_clr.t ..... skipped: No M$-Excel parser found t/62_fmt.t ..... skipped: No M$-Excel parser found t/63_misc.t .... skipped: No M$-Excel parser found t/64_dates.t ... skipped: No M$-Excel parser found t/65_perc.t .... skipped: No M$-Excel parser found All tests successful. * You get an 'APPROVED' from me, provided that you update the %changelog within pkg git and add comments to the explicit Requires.
Requires: perl(Test::More) >= 0.98 This Requires: is wrong. There is no run-time dependency between this package and Test::More: # grep More Spreadsheet-Read-0.45/Read.pm Requires: perl(IO::Scalar) I am not sure about this one. Looks like this is implemented as optional run-time requirement, while it actually is a mandatory requirement. I.e. I am inclined to believe this Requires: might be right. ... t/40_sxc.t ..... skipped: No SXC parser found t/45_ods.t ..... skipped: No SXC parser found ... If I understand the code correctly, these warnings indicate that some optional modules, these tests are trying exercise are not available. I haven't checked all details, but in case of the 40_sxc.t, Spreadsheet::ReadSXC seems to be required to exercise this test. I'd recommend to also package such optional perl-modules and to BR: them. This would assure these packages are tested at build-time and are (optionally) available to users at run-time.
No response for years. Closing. Feel free to reopen if you want to continue.