Bug 756443
| Summary: | Review Request: python-cagraph - A PyGTK widget for plotting charts and graphs | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Juan Orti <jorti> |
| Component: | Package Review | Assignee: | Tom "spot" Callaway <tcallawa> |
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | unspecified | ||
| Version: | rawhide | CC: | jorti, notting, package-review, tcallawa, volker27 |
| Target Milestone: | --- | Flags: | tcallawa:
fedora-review+
gwync: fedora-cvs+ |
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | python-cagraph-1.2-10.fc16 | Doc Type: | Bug Fix |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2012-04-25 04:57:38 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
| Bug Depends On: | |||
| Bug Blocks: | 756445 | ||
|
Description
Juan Orti
2011-11-23 15:40:40 UTC
According to the files, license is GPLv3+, not GPLv3
rm -rf %{buildroot}/*, clean section and buildroot definition are not necessary, if you're not aiming for EPEL 5: http://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#BuildRoot_tag
Defining sitearch or sitelib is only necessary for EPEL 5 as well, see http://fedoraproject.org/wiki/Packaging:Python
The if clause for Suse is no use in Fedora. I guess you're trying to keep a single spec for Suse and Fedora. I don't know Suse's guidelines, but that might not work.
Don't install the documentation on your own. Only use %doc in the files section: http://fedoraproject.org/wiki/Packaging:Guidelines#Documentation
FSF postal address is wrong:
[makerpm@desktop SPECS]$ rpmlint /home/makerpm/rpmbuild/RPMS/noarch/cagraph-1.2-7.fc16.noarch.rpm
cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/axis/taxis.py
cagraph.noarch: E: incorrect-fsf-address /usr/share/doc/cagraph-1.2/examples/example99.py
cagraph.noarch: E: incorrect-fsf-address /usr/share/doc/cagraph-1.2/examples/example6.py
cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/axis/xaxis.py
...
Please contact the developers on that and feel free to correct it.
I think this package should have "python" in the name: http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Addon_Packages_.28python_modules.29
I suppose you can use a single chmod to change all the py file's permissions.
Did you look for a sponsor yet? Please find further information below: http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group (In reply to comment #1) Hello, I have updated the package, the new files are here: Spec URL: https://raw.github.com/jorti/cagraph-fedora-package/v1.2-8/SPECS/python-cagraph.spec SRPM URL: https://github.com/jorti/cagraph-fedora-package/raw/v1.2-8/SRPMS/python-cagraph-1.2-8.fc16.src.rpm > According to the files, license is GPLv3+, not GPLv3 Fixed. > rm -rf %{buildroot}/*, clean section and buildroot definition are not > necessary, if you're not aiming for EPEL 5: > http://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#BuildRoot_tag I would like to be able to compile it for EPEL, because this library is used by tor-arm, a monitor for Tor routers which I also submited to review in bug #756445 > The if clause for Suse is no use in Fedora. I guess you're trying to keep a > single spec for Suse and Fedora. I don't know Suse's guidelines, but that might > not work. All references to Suse removed. > Don't install the documentation on your own. Only use %doc in the files > section: http://fedoraproject.org/wiki/Packaging:Guidelines#Documentation Fixed. > FSF postal address is wrong: Fixed. I include a patch and also submitted it upstream > I think this package should have "python" in the name: > http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Addon_Packages_.28python_modules.29 Renamed. > I suppose you can use a single chmod to change all the py file's permissions. Done. Now the output of rpmlint is clean: $ rpmlint SPECS/python-cagraph.spec RPMS/noarch/python-cagraph-1.2-8.fc16.noarch.rpm SRPMS/python-cagraph-1.2-8.fc16.src.rpm 2 packages and 1 specfiles checked; 0 errors, 0 warnings. (In reply to comment #2) > Did you look for a sponsor yet? Please find further information below: > > http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group I announced myself in the devel list, but I haven't find anyone. All sponsors are welcome! :) I have improved the SPEC file by removing the python shebang from the libraries, the new files are here: Spec URL: https://raw.github.com/jorti/cagraph-fedora-package/v1.2-9/SPECS/python-cagraph.spec SRPM URL: https://github.com/jorti/cagraph-fedora-package/raw/v1.2-9/SRPMS/python-cagraph-1.2-9.fc16.src.rpm Now the rpmlint output is: $ rpmlint SPECS/python-cagraph.spec RPMS/noarch/python-cagraph-1.2-9.fc16.noarch.rpm SRPMS/python-cagraph-1.2-9.fc16.src.rpm SPECS/python-cagraph.spec: W: invalid-url Source0: http://cagraph.googlecode.com/files/cagraph-1.2.tar.gz HTTP Error 404: Not Found python-cagraph.src: W: invalid-url Source0: http://cagraph.googlecode.com/files/cagraph-1.2.tar.gz HTTP Error 404: Not Found 2 packages and 1 specfiles checked; 0 errors, 2 warnings. The invalid-url error is a known problem with googlecode.com already reported in bug 767739 Notice, you need a sponsor.
Package Review
==============
Key:
- = N/A
x = Pass
! = Fail
? = Not evaluated
==== Generic ====
[x]: MUST Package is licensed with an open-source compatible license and meets
other legal requirements as defined in the legal section of Packaging
Guidelines.
[x]: MUST Package successfully compiles and builds into binary rpms on at
least one supported primary architecture.
[-]: MUST %build honors applicable compiler flags or justifies otherwise.
[x]: MUST All build dependencies are listed in BuildRequires, except for any
that are listed in the exceptions section of Packaging Guidelines.
[-]: MUST Buildroot is not present
Note: Buildroot is not needed unless packager plans to package for EPEL5
[x]: MUST Package contains no bundled libraries.
[x]: MUST Changelog in prescribed format.
[-]: MUST Package has no %clean section with rm -rf %{buildroot} (or
$RPM_BUILD_ROOT)
Note: Clean is needed only if supporting EPEL
[x]: MUST Sources contain only permissible code or content.
[!]: MUST Each %files section contains %defattr if rpm < 4.4
Note: defattr(....) present in %files section. This is OK if packaging
for EPEL5. Otherwise not needed
This comment was created by a script. As far as I know, defattr was only necessary in EPEL 4, but somebody might prove me wrong. I don't mind it -- it doesn't hurt.
[-]: MUST Macros in Summary, %description expandable at SRPM build time.
[-]: MUST Package requires other packages for directories it uses.
[x]: MUST Package uses nothing in %doc for runtime.
[-]: MUST Package is not known to require ExcludeArch.
[x]: MUST Permissions on files are set properly.
[x]: MUST Package does not contain duplicates in %files.
[x]: MUST Spec file lacks Packager, Vendor, PreReq tags.
[-]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
beginning of %install.
Note: rm -rf is only needed if supporting EPEL5
[-]: MUST Large documentation files are in a -doc subpackage, if required.
[x]: MUST 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.
[x]: MUST License field in the package spec file matches the actual license.
[x]: MUST Package consistently uses macros (instead of hard-coded directory
names).
[x]: MUST Package is named according to the Package Naming Guidelines.
[x]: MUST Package does not generate any conflict.
[x]: MUST Package obeys FHS, except libexecdir and /usr/target.
[x]: MUST Package must own all directories that it creates.
[x]: MUST Package does not own files or directories owned by other packages.
[x]: MUST Package installs properly.
[x]: MUST Requires correct, justified where necessary.
[x]: MUST Rpmlint output is silent.
rpmlint python-cagraph-1.2-9.fc18.noarch.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
rpmlint python-cagraph-1.2-9.fc18.src.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
[x]: MUST Sources used to build the package match the upstream source, as
provided in the spec URL.
/media/speicher1/makerpm/756443/cagraph-1.2.tar.gz :
MD5SUM this package : 4b0e024cfb4b94d80a57128d1cae82fc
MD5SUM upstream package : 4b0e024cfb4b94d80a57128d1cae82fc
[x]: MUST Spec file is legible and written in American English.
[x]: MUST Spec file name must match the spec package %{name}, in the format
%{name}.spec.
[-]: MUST Package contains a SysV-style init script if in need of one.
[x]: MUST File names are valid UTF-8.
[-]: MUST Useful -debuginfo package or justification otherwise.
[x]: SHOULD Reviewer should test that the package builds in mock.
[-]: SHOULD 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]: SHOULD Dist tag is present.
[x]: SHOULD No file requires outside of /etc, /bin, /sbin, /usr/bin,
/usr/sbin.
[x]: SHOULD Final provides and requires are sane (rpm -q --provides and rpm -q
--requires).
[?]: SHOULD Package functions as described.
[x]: SHOULD Latest version is packaged.
[x]: SHOULD Package does not include license text files separate from
upstream.
[!]: SHOULD Patches link to upstream bugs/comments/lists or are otherwise
justified.
Please contact the developer on the shebangs. While at it, he should also complete the information in PKG-INFO.
[!]: SHOULD SourceX / PatchY prefixed with %{name}.
Note: Patch0: python-cagraph-fsf_address_fix.patch (python-cagraph-
fsf_address_fix.patch)
[x]: SHOULD SourceX is a working URL.
[-]: SHOULD Description and summary sections in the package spec file contains
translations for supported Non-English languages, if available.
[-]: SHOULD Package should compile and build into binary rpms on all supported
architectures.
[-]: SHOULD %check is present and all tests pass.
[x]: SHOULD Packages should try to preserve timestamps of original installed
files.
[!]: SHOULD Spec use %global instead of %define.
Note: %define libname cagraph
You could drop cairo from Requires, as pygtk2 will drag it in anyway. You could ship the examples directory as documentation. Neither of the two is a blocker.
Make the first line "%if 0%{?rhel} <= 5", as we don't support old Fedora anymore.
Thank you for your review. I have fixed what you have told me, but I have decided to drop the patch to fix the FSF postal address, because based on other reviews I learned to not touch license information. These are the bugs reported upstream: FSF address: http://code.google.com/p/cagraph/issues/detail?id=3 PKG-INFO: http://code.google.com/p/cagraph/issues/detail?id=4 Shebangs: http://code.google.com/p/cagraph/issues/detail?id=5 SPEC file: https://raw.github.com/jorti/cagraph-fedora-package/v1.2-10.1/SPECS/python-cagraph.spec SRPM file: https://github.com/jorti/cagraph-fedora-package/raw/v1.2-10.1/SRPMS/python-cagraph-1.2-10.fc16.src.rpm Rpmlint output: SPECS/python-cagraph.spec: W: invalid-url Source0: http://cagraph.googlecode.com/files/cagraph-1.2.tar.gz HTTP Error 404: Not Found python-cagraph.noarch: E: incorrect-fsf-address /usr/share/doc/python-cagraph-1.2/examples/example99.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/ca_graph.py python-cagraph.noarch: E: incorrect-fsf-address /usr/share/doc/python-cagraph-1.2/examples/example7.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/axis/taxis.py python-cagraph.noarch: E: incorrect-fsf-address /usr/share/doc/python-cagraph-1.2/examples/example4.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/axis/yaxis.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/series/area.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/series/dna.py python-cagraph.noarch: E: incorrect-fsf-address /usr/share/doc/python-cagraph-1.2/examples/example3.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/series/series.py python-cagraph.noarch: E: incorrect-fsf-address /usr/share/doc/python-cagraph-1.2/examples/example8.py python-cagraph.noarch: E: incorrect-fsf-address /usr/share/doc/python-cagraph-1.2/examples/example2.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/axis/xaxis.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/series/line.py python-cagraph.noarch: E: incorrect-fsf-address /usr/share/doc/python-cagraph-1.2/examples/example9.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/axis/axis.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/series/labels.py python-cagraph.noarch: E: incorrect-fsf-address /usr/share/doc/python-cagraph-1.2/examples/example5.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/ca_graph_file.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/series/bar.py python-cagraph.noarch: E: incorrect-fsf-address /usr/share/doc/python-cagraph-1.2/examples/example1.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/series/hbar.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/ca_graph_grid.py python-cagraph.noarch: E: incorrect-fsf-address /usr/share/doc/python-cagraph-1.2/examples/test.py python-cagraph.noarch: E: incorrect-fsf-address /usr/share/doc/python-cagraph-1.2/examples/example6.py python-cagraph.src: W: invalid-url Source0: http://cagraph.googlecode.com/files/cagraph-1.2.tar.gz HTTP Error 404: Not Found 2 packages and 1 specfiles checked; 25 errors, 2 warnings. == Review == - rpmlint checks return: python-cagraph.src: W: invalid-url Source0: http://cagraph.googlecode.com/files/cagraph-1.2.tar.gz HTTP Error 404: Not Found python-cagraph.noarch: E: incorrect-fsf-address /usr/share/doc/python-cagraph-1.2/examples/example99.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/ca_graph.py python-cagraph.noarch: E: incorrect-fsf-address /usr/share/doc/python-cagraph-1.2/examples/example7.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/axis/taxis.py python-cagraph.noarch: E: incorrect-fsf-address /usr/share/doc/python-cagraph-1.2/examples/example4.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/axis/yaxis.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/series/area.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/series/dna.py python-cagraph.noarch: E: incorrect-fsf-address /usr/share/doc/python-cagraph-1.2/examples/example3.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/series/series.py python-cagraph.noarch: E: incorrect-fsf-address /usr/share/doc/python-cagraph-1.2/examples/example8.py python-cagraph.noarch: E: incorrect-fsf-address /usr/share/doc/python-cagraph-1.2/examples/example2.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/axis/xaxis.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/series/line.py python-cagraph.noarch: E: incorrect-fsf-address /usr/share/doc/python-cagraph-1.2/examples/example9.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/axis/axis.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/series/labels.py python-cagraph.noarch: E: incorrect-fsf-address /usr/share/doc/python-cagraph-1.2/examples/example5.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/ca_graph_file.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/series/bar.py python-cagraph.noarch: E: incorrect-fsf-address /usr/share/doc/python-cagraph-1.2/examples/example1.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/series/hbar.py python-cagraph.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/cagraph/ca_graph_grid.py python-cagraph.noarch: E: incorrect-fsf-address /usr/share/doc/python-cagraph-1.2/examples/test.py python-cagraph.noarch: E: incorrect-fsf-address /usr/share/doc/python-cagraph-1.2/examples/example6.py 2 packages and 0 specfiles checked; 25 errors, 1 warnings. All safe to ignore (one is broken webservers at google, the other 25 are incorrect address). - package meets naming guidelines - package meets packaging guidelines - license (GPLv3+) OK, text in %doc, matches source - spec file legible, in am. english - source matches upstream (a6928f07adb8f8d4b0076e01c0ec264e1acaaa6db21376c854fa827c9b04e3f3) - package compiles on f17 (x86_64) - no missing BR - no unnecessary BR - no locales - not relocatable - owns all directories that it creates - no duplicate files - permissions ok - macro use consistent - code, not content - no need for -docs - nothing in %doc affects runtime - no need for .desktop file APPROVED. New Package SCM Request ======================= Package Name: python-cagraph Short Description: A PyGTK widget for plotting charts and graphs Owners: jorti Branches: f15 f16 f17 InitialCC: Git done (by process-git-requests). python-cagraph-1.2-10.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/python-cagraph-1.2-10.fc17 python-cagraph-1.2-10.fc17 has been pushed to the Fedora 17 testing repository. python-cagraph-1.2-10.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/python-cagraph-1.2-10.fc16 python-cagraph-1.2-10.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/python-cagraph-1.2-10.fc15 python-cagraph-1.2-10.fc17 has been pushed to the Fedora 17 stable repository. python-cagraph-1.2-10.fc15 has been pushed to the Fedora 15 stable repository. python-cagraph-1.2-10.fc16 has been pushed to the Fedora 16 stable repository. |