Bug 716384 - Review Request: filo - Useful FILe and stream Operations
Summary: Review Request: filo - Useful FILe and stream Operations
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2011-06-24 09:21 UTC by Adam Huffman
Modified: 2021-05-22 00:45 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2021-05-22 00:45:29 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Adam Huffman 2011-06-24 09:21:59 UTC
Spec URL: http://verdurin.fedorapeople.org/reviews/filo/filo.spec
SRPM URL: http://verdurin.fedorapeople.org/reviews/filo/filo-1.1.0-2.fc16.src.rpm
Description: 
Filo consists of various useful file and stream operations, including
some that were originally part of BEDTools.

Comment 1 Ben Boeckel 2011-06-30 00:35:51 UTC
I'll take this. Looks like some interesting tools (though shuffle is "sort -R")

Package Review
==============

Key:
- = N/A
x = Check
! = Problem
? = Not evaluated

=== REQUIRED ITEMS ===
[x]  Package is named according to the Package Naming Guidelines. [1]
[x]  Spec file name must match the base package %{name}, in the format %{name}.spec.
[x]  Spec file is legible and written in American English.
[x]  Spec file lacks Packager, Vendor, PreReq tags.
[x]  Spec uses macros instead of hard-coded directory names.
[x]  Package consistently uses macros.
[x]  Macros in Summary, %description expandable at SRPM build time.
[x]  PreReq is not used.
[-]  Requires correct, justified where necessary.
[x]  All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [2]
[-]  Package use %makeinstall only when ``make install DESTDIR=...'' doesn't work.
[-]  The spec file handles locales properly.
[-]  Changelog in prescribed format.
[x]  Rpmlint output is silent.

filo.x86_64: W: no-manual-page-for-binary stats
filo.x86_64: W: no-manual-page-for-binary shuffle
filo.x86_64: W: no-manual-page-for-binary groupBy
3 packages and 0 specfiles checked; 0 errors, 3 warnings.

Manpages would be nice to have.

[!]  License field in the package spec file matches the actual license.

I see no indication of the license in the package. Contact upstream to ship a LICENSE or COPYING file.

[-]  If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc.
[-]  License file installed when any subpackage combination is installed.
[!]  Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [3,4]

No licensing specified.

[!]  Sources contain only permissible code or content.

No license.

[x]  Sources used to build the package matches the upstream source, as provided in the spec URL.
     MD5SUM this package     : d8b495af3a5b68f8796374a2ce15c44d  arq5x-filo-f56efe3.tar.gz
     MD5SUM upstream package : d8b495af3a5b68f8796374a2ce15c44d  arq5x-filo-f56efe3.tar.gz.orig
[x]  Compiler flags are appropriate.
[x]  %build honors applicable compiler flags or justifies otherwise.

Flags seem to be set correctly, but not all compile commands are in the logs.

[-]  ldconfig called in %post and %postun if required.
[-]  Package must own all directories that it creates.
[x]  Package does not own files or directories owned by other packages.
[-]  Package requires other packages for directories it uses.
[x]  Package does not contain duplicates in %files.
[x]  Permissions on files are set properly.

chmod -x would probably be better for the source files (getting upstream to remove the perms would also be nice).

[x]  Each %files section contains %defattr.
[x]  No %config files under /usr.
[-]  %config files are marked noreplace or the reason is justified.
[-]  Package contains a properly installed %{name}.desktop using desktop-file-install file if it is a GUI application. [5]
[-]  Package contains a valid .desktop file.
[x]  Package contains code, or permissable content.
[x]  File names are valid UTF-8.
[-]  Large documentation files are in a -doc subpackage, if required.
[x]  Package uses nothing in %doc for runtime.
[!]  Package contains no bundled libraries.

Seems to ship with a bundled gzstream

http://www.cs.unc.edu/Research/compgeom/gzstream/

[-]  Header files in -devel subpackage, if present.
[-]  Static libraries in -static subpackage, if present.
[x]  Package contains no static executables.
[-]  Package requires pkgconfig, if .pc files are present.
[-]  Development .so files in -devel subpackage, if present.
[-]  Fully versioned dependency in subpackages, if present.
[-]  Package does not contain any libtool archives (.la).
[x]  Useful -debuginfo package or justification otherwise.
[x]  Rpath absent or only used for internal libs.
[x]  Package does not generate any conflict.
[x]  Package does not contains kernel modules.
[x]  Package is not relocatable.
[x]  Package successfully compiles and builds into binary rpms on at least one supported architecture.
[x]  Package is not known to require ExcludeArch.
[x]  Package installs properly.
[x]  Package obeys FHS, except libexecdir and /usr/target.
[!]  Package meets the Packaging Guidelines. [6]

=== SUGGESTED ITEMS ===
[?]  Package functions as described.
[x]  Latest version is packaged.
[!]  Package does not include license text files separate from upstream.
[!]  If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it.
[x]  Description and summary sections in the package spec file contains translations for supported Non-English languages, if available.
[x]  SourceX is a working URL.
[!]  SourceX / PatchY prefixed with %{name}.

This is a github thing.

[x]  Final provides and requires are sane (rpm -q --provides and rpm -q --requires).
[-]  %check is present and all tests pass.
[-]  Usually, subpackages other than devel should require the base package using a fully versioned dependency.
[x]  Reviewer should test that the package builds in mock.
[?]  Package should compile and build into binary rpms on all supported architectures.
[x]  Dist tag is present.
[-]  Spec use %global instead of %define.
[-]  Scriptlets must be sane, if used.
[-]  The placement of pkgconfig(.pc) files are correct.
[x]  No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]  Packages should try to preserve timestamps of original installed files.
[-]  File based requires are sane.
[!]  Man pages included for all executables.
[x]  Uses parallel make.
[-]  Patches link to upstream bugs/comments/lists or are otherwise justified.

(Copied from above for convenience)

=== Issues ===
- BuildRoot tag is no longer necessary.
- Since this is a snapshot (no canonical tarball given), the Release should reflect that.
- Manpages would be nice to have.
- I see no indication of the license in the package. Contact upstream to ship a LICENSE or COPYING file.
- Flags seem to be set correctly, but not all compile commands are in the logs.
- chmod -x would probably be better for the source files (getting upstream to remove the perms would also be nice).
- Seems to ship with a bundled gzstream (URL above)

=== Final Notes ===
- rm -rf %{buildroot} isn't necessary anymore.
- %clean can also be removed.
- Macro-ize the commit hash used for easier maintainence.

[1] https://fedoraproject.org/wiki/Packaging:NamingGuidelines
[2] https://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2
[3] https://fedoraproject.org/wiki/Packaging:LicensingGuidelines
[4] https://fedoraproject.org/wiki/Licensing:Main
[5] https://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files
[6] https://fedoraproject.org/wiki/Packaging:Guidelines

Comment 2 Adam Huffman 2011-06-30 21:56:27 UTC
Just a quick comment - the line "Licenced under the GNU General Public License 2.0 license." in some of the source files is why I put the license as GPLv2.  There should really be a file to confirm this, though, I agree.  I'll contact upstream.

Re. the BuildRoot and %clean parts - I intend to support this in EPEL5, for which I believe those are still required.

I'll look into the other issues, including the bundled library.

Comment 3 Ben Boeckel 2011-07-26 02:14:15 UTC
Ping?

Comment 4 Adam Huffman 2011-07-26 06:55:04 UTC
Upstream doesn't want to unbundle gzstream, so I'l have to deal with that myself.  I've made a start on a gzstream package.  Will update this one later in the week.

Comment 5 Adam Huffman 2012-01-11 23:33:27 UTC
I've had time to work on this again and have made progress on a gzstream package but it's not ready for review yet.

Comment 6 Ben Boeckel 2013-06-15 02:09:20 UTC
Any progress on this?

Comment 7 Package Review 2020-07-10 00:45:43 UTC
This is an automatic check from review-stats script.

This review request ticket hasn't been updated for some time, but it seems
that the review is still being working out by you. If this is right, please
respond to this comment clearing the NEEDINFO flag and try to reach out the
submitter to proceed with the review.

If you're not interested in reviewing this ticket anymore, please clear the
fedora-review flag and reset the assignee, so that a new reviewer can take
this ticket.

Without any reply, this request will shortly be resetted.

Comment 8 Ben Boeckel 2020-07-10 12:32:38 UTC
No progress here. Unassigning myself.

Comment 9 Otto Liljalaakso 2021-04-21 09:27:11 UTC
Do you still intend to complete this package? If so, I can review. You should update the specfile and srpm first, as they are so old and (while either unmaintained or complete now) upstream has changed license and added some stuff since. If you are not interested any more, please either close this issue, or do nothing, in which case automation should close the issue in one month.

Comment 10 Package Review 2021-05-22 00:45:29 UTC
This is an automatic action taken by review-stats script.

The ticket submitter failed to clear the NEEDINFO flag in a month.
As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
we consider this ticket as DEADREVIEW and proceed to close it.


Note You need to log in before you can comment on or make changes to this bug.