Spec URL: http://verdurin.org.uk/~verdurin/fedora/reviews/fastx_toolkit/fastx_toolkit.spec SRPM URL: http://verdurin.org.uk/~verdurin/fedora/reviews/fastx_toolkit/fastx_toolkit-0.0.13-1.fc12.src.rpm Description: The FASTX-Toolkit is a collection of command line tools for Short-Reads FASTA/FASTQ files preprocessing. Next-Generation sequencing machines usually produce FASTA or FASTQ files, containing multiple short-reads sequences (possibly with quality information). The main processing of such FASTA/FASTQ files is mapping (aka aligning) the sequences to reference genomes or other databases using specialized programs. Example of such mapping programs are: Blat, SHRiMP, LastZ, MAQ and many many others. However, It is sometimes more productive to preprocess the FASTA/FASTQ files before mapping the sequences to the genome - manipulating the sequences to produce better mapping results. The FASTX-Toolkit tools perform some of these preprocessing tasks.
This is an informal review. Formal review will follow. Critical issue: MUST: The package MUST successfully compile and build into binary rpms. SHOULD: The package builds in mock. $ mock --rebuild fastx_toolkit-0.0.13-1.fc12.src.rpm is failed because libgtextutils-devel which is set as BuildRequires is not available by Fedora. You should add libgtextutils-devel package to Fedora first. Issues: $ rpmlint fastx_toolkit.spec fastx_toolkit.spec:8: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 8) Please fix mixed usage of spaces and tabs. Lists confirmed: + rpmlint against SRPM returns spelling-error warning. However the words pointed by rpmlint are from official website and seem to be no problem. + Spec file name meets Packaging Guidelines. + License: AGPLv3 meets Licensing Guidelines. + Source file match with upstream one with md5sum and sha1sum. MUST: The package does not yet exist in Fedora. The Review Request is not a duplicate. OK MUST: The spec file for the package is legible and macros are used consistently. OK MUST: The package must be named according to the Package Naming Guidelines. OK MUST: The spec file name must match the base package %{name}. OK MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines. OK MUST: The License field in the package spec file must match the actual license. OK MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. OK MUST: The spec file MUST handle locales properly. N/A MUST: Clean section exists. OK MUST: Large documentation files must go in a -doc subpackage. N/A MUST: Buildroot cleaned before install. OK SHOULD: %{?dist} tag is used in release. OK SHOULD: If the package does not include license text(s) as separate files from upstream, the packager should query upstream to include it. OK The following item will be checked after the critical issue is solved. MUST: Optflags are used and time stamps preserved. MUST: Packages containing shared library files must call ldconfig. MUST: A package must own all directories that it creates or require the package that owns the directory. MUST: Files only listed once in %files listings. MUST: Debuginfo package is complete. MUST: Permissions on files must be set properly. MUST: All relevant items are included in %doc. Items in %doc do not affect runtime of application. MUST: Header files must be in a -devel package. MUST: Static libraries must be in a -static package. MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. MUST: If a package contains library files with a suffix then library files ending in .so must go in a -devel package. MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency. MUST: Packages does not contain any .la libtool archives. MUST: Desktop files are installed properly. MUST: No file conflicts with other packages and no general names.
(In reply to comment #1) > This is an informal review. Formal review will follow. > Thanks for taking a look. > Critical issue: > MUST: The package MUST successfully compile and build into binary rpms. > SHOULD: The package builds in mock. > $ mock --rebuild fastx_toolkit-0.0.13-1.fc12.src.rpm > is failed because libgtextutils-devel which is set as BuildRequires is not > available by Fedora. > > You should add libgtextutils-devel package to Fedora first. > Yes, that's right. I uploaded a bunch of new requests late on Friday, after having installed them locally. The request for libgtextutils is at: https://bugzilla.redhat.com/show_bug.cgi?id=598511 > > Issues: > $ rpmlint fastx_toolkit.spec > fastx_toolkit.spec:8: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: > line 8) > Please fix mixed usage of spaces and tabs. > Will take a look at that. > > Lists confirmed: > + rpmlint against SRPM returns spelling-error warning. However the words > pointed by rpmlint are from official website and seem to be no problem. > > + Spec file name meets Packaging Guidelines. > + License: AGPLv3 meets Licensing Guidelines. > + Source file match with upstream one with md5sum and sha1sum. > > > MUST: The package does not yet exist in Fedora. The Review Request is not a > duplicate. OK > MUST: The spec file for the package is legible and macros are used > consistently. OK > MUST: The package must be named according to the Package Naming Guidelines. OK > MUST: The spec file name must match the base package %{name}. OK > MUST: The package must be licensed with a Fedora approved license and meet the > Licensing Guidelines. OK > MUST: The License field in the package spec file must match the actual license. > OK > MUST: The sources used to build the package must match the upstream source, as > provided in the spec URL. OK > MUST: The spec file MUST handle locales properly. N/A > MUST: Clean section exists. OK > MUST: Large documentation files must go in a -doc subpackage. N/A > MUST: Buildroot cleaned before install. OK > SHOULD: %{?dist} tag is used in release. OK > SHOULD: If the package does not include license text(s) as separate files from > upstream, the packager should query upstream to include it. OK > > The following item will be checked after the critical issue is solved. > MUST: Optflags are used and time stamps preserved. > MUST: Packages containing shared library files must call ldconfig. > MUST: A package must own all directories that it creates or require the package > that owns the directory. > MUST: Files only listed once in %files listings. > MUST: Debuginfo package is complete. > MUST: Permissions on files must be set properly. > MUST: All relevant items are included in %doc. Items in %doc do not affect > runtime of application. > MUST: Header files must be in a -devel package. > MUST: Static libraries must be in a -static package. > MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. > MUST: If a package contains library files with a suffix then library files > ending in .so must go in a -devel package. > MUST: In the vast majority of cases, devel packages must require the base > package using a fully versioned dependency. > MUST: Packages does not contain any .la libtool archives. > MUST: Desktop files are installed properly. > MUST: No file conflicts with other packages and no general names.
> MUST: Optflags are used and time stamps preserved. Not entirely. The source code overrides Fedora's optflags. * Description of the -devel subpackage doesn't match its contents: | The fastx_toolkit-devel package contains libraries and | header files for developing applications that use fastx_toolkit. It doesn't contain any libraries and header files. It contains two M4 macro definition files from the GNU Autoconf Archive. It's questionable to assign them to this package and place them in the public aclocal directory. $ rpmls -p fastx_toolkit-devel-0.0.13-1.fc13.x86_64.rpm|grep local -rw-r--r-- /usr/share/aclocal/ax_c_long_long.m4 -rw-r--r-- /usr/share/aclocal/ax_cxx_header_stdcxx_tr1.m4 The only other contents of the -devel package is the big tree at: /usr/share/doc/fastx_toolkit-devel-0.0.13/galaxy Licensed differently (!). Lots of unusable Makefile* files. Icon files, XML files, not really any documentation about all that. Strange place to dump that into %doc and a -devel pkg.
Thanks for taking a look. Yes, the description is a bit generic - I thought I'd put in a note to this bug about the Galaxy files but clearly forgot. They're for integrating the package with a local Galaxy installation. I wasn't quite sure how to handle them. Perhaps a different subpackage - fastx_toolkit-galaxy? Would you recommend not packaging the M4 files?
> Would you recommend not packaging the M4 files? Yes. Those two files either belong into a comprehensive autoconf-archive package (for the scripts included in the GNU Autoconf Archive) or into the local autotools build framework of software that wants to use these macros (e.g. in a local "m4" subdir). If more packages started placing arbitrary M4 files from the Autoconf Archive into /usr/share/aclocal, this would increase the risk of causing conflicts.
rpmlint output: fastx_toolkit.src: W: spelling-error %description -l en_US preprocessing -> reprocessing, p reprocessing, preprocessed fastx_toolkit.src: W: spelling-error %description -l en_US preprocess -> reprocess, p reprocess, processors fastx_toolkit.src:8: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 8) fastx_toolkit.x86_64: W: spelling-error %description -l en_US preprocessing -> reprocessing, p reprocessing, preprocessed fastx_toolkit.x86_64: W: spelling-error %description -l en_US preprocess -> reprocess, p reprocess, processors fastx_toolkit.x86_64: W: no-manual-page-for-binary fasta_clipping_histogram.pl fastx_toolkit.x86_64: W: no-manual-page-for-binary fastq_quality_trimmer fastx_toolkit.x86_64: W: no-manual-page-for-binary fastx_quality_stats fastx_toolkit.x86_64: W: no-manual-page-for-binary fastq_quality_filter fastx_toolkit.x86_64: W: no-manual-page-for-binary fastx_trimmer fastx_toolkit.x86_64: W: no-manual-page-for-binary fastq_quality_boxplot_graph.sh fastx_toolkit.x86_64: W: no-manual-page-for-binary fastx_barcode_splitter.pl fastx_toolkit.x86_64: W: no-manual-page-for-binary fastq_quality_converter fastx_toolkit.x86_64: W: no-manual-page-for-binary fastx_nucleotide_distribution_line_graph.sh fastx_toolkit.x86_64: W: no-manual-page-for-binary fasta_formatter fastx_toolkit.x86_64: W: no-manual-page-for-binary fastx_collapser fastx_toolkit.x86_64: W: no-manual-page-for-binary fastx_renamer fastx_toolkit.x86_64: W: no-manual-page-for-binary fasta_nucleotide_changer fastx_toolkit.x86_64: W: no-manual-page-for-binary fastx_reverse_complement fastx_toolkit.x86_64: W: no-manual-page-for-binary fastq_to_fasta fastx_toolkit.x86_64: W: no-manual-page-for-binary fastx_nucleotide_distribution_graph.sh fastx_toolkit.x86_64: W: no-manual-page-for-binary fastq_masker fastx_toolkit.x86_64: W: no-manual-page-for-binary fastx_artifacts_filter fastx_toolkit.x86_64: W: no-manual-page-for-binary fastx_uncollapser fastx_toolkit.x86_64: W: no-manual-page-for-binary fastx_clipper fastx_toolkit-debuginfo.x86_64: W: spelling-error Summary(en_US) fastx -> fast, fasts, fast x fastx_toolkit-debuginfo.x86_64: W: spelling-error %description -l en_US fastx -> fast, fasts, fast x fastx_toolkit-devel.x86_64: W: spelling-error Summary(en_US) fastx -> fast, fasts, fast x fastx_toolkit-devel.x86_64: W: spelling-error %description -l en_US fastx -> fast, fasts, fast x fastx_toolkit-devel.x86_64: W: spurious-executable-perm /usr/share/doc/fastx_toolkit-devel-0.0.13/galaxy/tools/fastx_toolkit/fastx_barcode_splitter_galaxy_wrapper.sh 4 packages and 0 specfiles checked; 0 errors, 30 warnings. - Check your tabs vs spaces. - Placing the galaxy stuff somewhere else than %doc gets rid of the exec perm warning. When packaging the galaxy stuff, be sure not to include any Makefiles or the like. - Please use http://hannonlab.cshl.edu/fastx_toolkit/ as the URL. It's nicer not to have macros in the URL, so one can cut'n'paste from the spec. - Don't ship the m4 files, as instructed by Michael. ** MUST: The package does not yet exist in Fedora. The Review Request is not a duplicate. OK MUST: The spec file for the package is legible and macros are used consistently. OK MUST: The package must be named according to the Package Naming Guidelines. OK MUST: The spec file name must match the base package %{name}. OK MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines. OK MUST: The License field in the package spec file must match the actual license. NEEDSWORK - License is AGPLv3+, not AGPLv3. MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. OK 6d233ff4ae3d52c457d447179f073a56 fastx_toolkit-0.0.13.tar.bz2 6d233ff4ae3d52c457d447179f073a56 ../SOURCES/fastx_toolkit-0.0.13.tar.bz2 MUST: The package MUST successfully compile and build into binary rpms. OK MUST: The spec file MUST handle locales properly. N/A MUST: Optflags are used and time stamps preserved. NEEDSWORK - As already pointed out by Michael, the build process overrides the Fedora optimization flags. - Use make %{?_smp_mflags} CFLAGS="$RPM_OPT_FLAGS" CXXFLAGS="$RPM_OPT_FLAGS" to correct this. - Also, please use make install INSTALL="install -p" to keep the time stamps in %install. MUST: Packages containing shared library files must call ldconfig. N/A MUST: A package must own all directories that it creates or require the package that owns the directory. OK MUST: Files only listed once in %files listings. OK - Maybe again make things a bit more verbose with %{_bindir}/fast* ? MUST: Debuginfo package is complete. OK MUST: Permissions on files must be set properly. OK MUST: Large documentation files must go in a -doc subpackage. N/A MUST: All relevant items are included in %doc. Items in %doc do not affect runtime of application. OK MUST: Header files must be in a -devel package. N/A MUST: Static libraries must be in a -static package. N/A MUST: If a package contains library files with a suffix then library files ending in .so must go in a -devel package. N/A MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency. N/A MUST: Packages does not contain any .la libtool archives. N/A MUST: Desktop files are installed properly. N/A MUST: No file conflicts with other packages and no general names. OK SHOULD: %{?dist} tag is used in release. OK SHOULD: If the package does not include license text(s) as separate files from upstream, the packager should query upstream to include it. OK SHOULD: The package builds in mock. OK EPEL: Clean section exists. OK EPEL: Buildroot cleaned before install. OK EPEL: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. N/A
I'm sure that galaxy is not for development purpose, and m4 macros is completely useless for this package.
New version at: http://verdurin.org.uk/~verdurin/fedora/reviews/fastx_toolkit/fastx_toolkit.spec http://verdurin.org.uk/~verdurin/fedora/reviews/fastx_toolkit/fastx_toolkit-0.0.13-2.fc12.src.rpm As with libgtextutils, there's no apparent acceptable license tag "AGPLv3+". I've made a -galaxy subpackage for the galaxy/ directory.
Whoops, I'm terribly sorry that this has slipped under my radar, as I've been quite busy at $DAYJOB. I'll try to look through this ASAP.
rpmlint output now at: fastx_toolkit.src: W: spelling-error %description -l en_US preprocessing -> reprocessing, p reprocessing, preprocessed fastx_toolkit.src: W: spelling-error %description -l en_US preprocess -> reprocess, p reprocess, procession fastx_toolkit.x86_64: W: spelling-error %description -l en_US preprocessing -> reprocessing, p reprocessing, preprocessed fastx_toolkit.x86_64: W: spelling-error %description -l en_US preprocess -> reprocess, p reprocess, procession fastx_toolkit.x86_64: W: no-manual-page-for-binary fasta_clipping_histogram.pl fastx_toolkit.x86_64: W: no-manual-page-for-binary fastq_quality_trimmer fastx_toolkit.x86_64: W: no-manual-page-for-binary fastx_quality_stats fastx_toolkit.x86_64: W: no-manual-page-for-binary fastq_quality_filter fastx_toolkit.x86_64: W: no-manual-page-for-binary fastx_trimmer fastx_toolkit.x86_64: W: no-manual-page-for-binary fastq_quality_boxplot_graph.sh fastx_toolkit.x86_64: W: no-manual-page-for-binary fastx_barcode_splitter.pl fastx_toolkit.x86_64: W: no-manual-page-for-binary fastq_quality_converter fastx_toolkit.x86_64: W: no-manual-page-for-binary fastx_nucleotide_distribution_line_graph.sh fastx_toolkit.x86_64: W: no-manual-page-for-binary fasta_formatter fastx_toolkit.x86_64: W: no-manual-page-for-binary fastx_collapser fastx_toolkit.x86_64: W: no-manual-page-for-binary fastx_renamer fastx_toolkit.x86_64: W: no-manual-page-for-binary fasta_nucleotide_changer fastx_toolkit.x86_64: W: no-manual-page-for-binary fastx_reverse_complement fastx_toolkit.x86_64: W: no-manual-page-for-binary fastq_to_fasta fastx_toolkit.x86_64: W: no-manual-page-for-binary fastx_nucleotide_distribution_graph.sh fastx_toolkit.x86_64: W: no-manual-page-for-binary fastq_masker fastx_toolkit.x86_64: W: no-manual-page-for-binary fastx_artifacts_filter fastx_toolkit.x86_64: W: no-manual-page-for-binary fastx_uncollapser fastx_toolkit.x86_64: W: no-manual-page-for-binary fastx_clipper fastx_toolkit-debuginfo.x86_64: W: spelling-error Summary(en_US) fastx -> fast, fasts, fast x fastx_toolkit-debuginfo.x86_64: W: spelling-error %description -l en_US fastx -> fast, fasts, fast x fastx_toolkit-galaxy.x86_64: W: spelling-error Summary(en_US) fastx -> fast, fasts, fast x fastx_toolkit-galaxy.x86_64: W: spelling-error %description -l en_US fastx -> fast, fasts, fast x fastx_toolkit-galaxy.x86_64: W: spelling-error %description -l en_US metagenomic -> meta genomic, meta-genomic, metagenesis fastx_toolkit-galaxy.x86_64: W: no-documentation 4 packages and 0 specfiles checked; 0 errors, 30 warnings. These are OK. Most of the issues have been fixed. However, some still remain: NEEDSWORK: - The -galaxy package contains autotools Makefiles. Get rid of them. - The license is AGPLv3+ as in libgtextutils.
New version at: http://verdurin.fedorapeople.org/reviews/fastx_toolkit/fastx_toolkit.spec http://verdurin.fedorapeople.org/reviews/fastx_toolkit/fastx_toolkit-0.0.13-3.fc14.src.rpm
Ping Adam, what's the status?
Autoconf-archive is now in review at bug 663925.
Jussi, Have you had a change to look at the latest version?
Crap, I had forgotten all about this review. Taking a look shortly.
You're missing ownership of %{_datadir}/%{name} It should be owned by -galaxy. Otherwise things should be fine. Please fix the ownership before git import; what you need to do is change %{_datadir}/%{name}/galaxy/ to %{_datadir}/%{name}/ in -galaxy. This package has been APPROVED
Many thanks for the review - I've made that last change in -4 New Package SCM Request ======================= Package Name: fastx_toolkit Short Description: Tools to process short-reads FASTA/FASTQ files Owners: verdurin Branches: f14 f15 el5 el6 InitialCC:
Git done (by process-git-requests).
Two month have past but nothing has been imported in the git.
Oops - I forgot to import the package... Many thanks for spotting that. Have just done so and submitted updates for all four branches.