Bug 609295
Summary: | Review Request: python-cement - CLI Application Framework for Python | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | BJ Dierkes <derks> |
Component: | Package Review | Assignee: | Jason Tibbitts <j> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | fedora-package-review, notting |
Target Milestone: | --- | Flags: | j:
fedora-review+
j: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | python-cement-0.8.14-5.fc13 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2011-01-20 19:53:00 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: |
Description
BJ Dierkes
2010-06-29 21:29:27 UTC
Updated: SPEC: http://5dollarwhitebox.org/tmp/cement.spec SRPM: http://5dollarwhitebox.org/tmp/cement-0.8.12-1.fc14.src.rpm $ rpmlint -i SPECS/cement.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. $ rpmlint -i RPMS/noarch/cement*0.8.12*.rpm 2 packages and 0 specfiles checked; 0 errors, 0 warnings. Updated, upstream now includes -devtools and -test under the primary source. The spec now performs nose testing as well. Also, renamed as python-cement as it is more appropriate. SPEC: http://5dollarwhitebox.org/tmp/python-cement.spec SRPM: http://5dollarwhitebox.org/tmp/python-cement-0.8.14-1.fc14.src.rpm $ rpmlint -i SPECS/python-cement.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. $ rpmlint -i RPMS/noarch/python-cement-* python-cement-devtools.noarch: E: zero-length /usr/lib/python2.7/site-packages/cement/paste/templates/cementapp/LICENSE python-cement-devtools.noarch: E: zero-length /usr/lib/python2.7/site-packages/cement/paste/templates/cementplugin/LICENSE 3 packages and 0 specfiles checked; 2 errors, 0 warnings. Note the warning are because the -devtools package includes application templates... the LICENSE and README in those templates are left blank by design. *** Bug 661179 has been marked as a duplicate of this bug. *** A few comments: Do I recall correctly that you are the upstream developer of this software? It would probably have benefited you to let us know about that. Are you sure you still need to pass --single-version-externally-managed to setuptools? http://fedoraproject.org/wiki/Packaging:Python_Eggs indicates that this should no longer be required, and that document is a few years old now. Nothing seems to own %{python_sitelib}/%{real_name}. You include a few subdirectories but not the directory itself. And, what does %{real_name} actually buy you, anyway? Isn't it shorter to simply type "cement"? It looks like you have a bunch of nasty macroification at the top which is unnecessary for Fedora (or even EL6). You could dispense with %pyver just by using globs in your file lists, and I'm not sure what the %python macro is used for at all. Some of the spec is so buried in pointless macros that it becomes quite difficult to understand. Thank you for your feedback. I can agree with the use of 'real_name' making things more cluttered. I've cleaned that up. I've also removed --single-version-externally-managed. As for the use of %{python} this was done to make porting to other python stacks easier. Rather than changing all references of 'python' to 'python3' or what-have-you, I can simply change the python macro. As for %{pyver}, using wild cards in the files list for 'egg-info' seems it would be less efficient... again when considering that this package may very well be ported to another python stack (via side-by-side python installs, etc).... where pyver might be 2.6 or 3.1 depending on the stack. I don't see any reason to change that at this point. That said the updated files are at: SPEC: http://5dollarwhitebox.org/tmp/python-cement.spec SRPM: http://5dollarwhitebox.org/tmp/python-cement-0.8.14-2.fc14.src.rpm $ rpmlint -i SPECS/python-cement.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. $ rpmlint -i RPMS/noarch/python-cement-* python-cement-devtools.noarch: E: zero-length /usr/lib/python2.7/site-packages/cement/paste/templates/cementapp/LICENSE python-cement-devtools.noarch: E: zero-length /usr/lib/python2.7/site-packages/cement/paste/templates/cementplugin/LICENSE 3 packages and 0 specfiles checked; 2 errors, 0 warnings. Oh by the way, yes I am also the upstream author. I didn't think to mention that initially, but will with any future submissions. Thanks. Unfortunately the %{python} macro doesn't save you all the trouble, because you still need your build dependencies to include python2-devel on Fedora (instead of python-devel). Also, if you intend to do the python3 thing, did you consider using the method in the current guidelines that builds packages for both python 2 and 3 from a single package? It's even more loaded with macros but gets you the whole thing at once. You're right about the python2-devel bit. I've removed the %{python} macro now... thanks for your patience. As for building for both python2 and python3, yes it was a thought. However upstream the project is not ready for python3 yet.... the next version of cement upstream will be targeted at python3 and therefore will probably have cement-0.X on python2, and the next version on python3. SPEC: http://5dollarwhitebox.org/tmp/python-cement.spec SRPM: http://5dollarwhitebox.org/tmp/python-cement-0.8.14-3.fc14.src.rpm $ rpmlint -i SPECS/python-cement.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. $ rpmlint -i RPMS/noarch/python-cement-* python-cement-devtools.noarch: E: zero-length /usr/lib/python2.7/site-packages/cement/paste/templates/cementapp/LICENSE python-cement-devtools.noarch: E: zero-length /usr/lib/python2.7/site-packages/cement/paste/templates/cementplugin/LICENSE 3 packages and 0 specfiles checked; 2 errors, 0 warnings. Thanks again. Everything does look much cleaner, thanks. (Although you don't have to make changes just because I suggest that they might look cleaner; it's a dialogue and I'm not making any demands.) I still think that pyver is unnecessary and can be replaced by globs, but it isn't a really big deal. Can you explain what pkguytil.py is for? I'm trying to understand why it needs to be there (since all non-EOL Fedora releases have python >= 2.6, as does EL6), whether it gets used and whether it could be removed. The question is whether it runs afoul of our bundled library policy. Note that the License: tag stuff should be from the perspective of the built rpms, so when listing the license bits you should give the location of the file as it's installed, instead of where it appears in the source tree. * source files match upstream. sha256sum: 4053518c5fe884f9b7d8b363ee06f3273b2274aff7be66cedcc9e25a26aa83ba cement-0.8.14.tar.gz * package meets naming and versioning guidelines. * specfile is properly named, is cleanly written and uses macros consistently. * summary is OK. * description is OK. * dist tag is present. * license field matches the actual license. * license is open source-compatible. * license text included in package. * latest version is being packaged. * BuildRequires are proper. * package builds in mock (rawhide, x86_64). * package installs properly. * rpmlint has acceptable complaints. * final provides and requires are sane: python-cement-0.8.14-3.fc15.noarch.rpm python-cement = 0.8.14-3.fc15 = python python(abi) = 2.7 python-configobj python-genshi python-jsonpickle python-cement-devtools-0.8.14-3.fc15.noarch.rpm python-cement-devtools = 0.8.14-3.fc15 = python(abi) = 2.7 python-cement = 0.8.14-3.fc15 python-paste-script python-tempita python-cement-doc-0.8.14-3.fc15.noarch.rpm python-cement-doc = 0.8.14-3.fc15 = (none) * %check is present and all tests pass: Ran 72 tests in 0.122s OK ? pkgutil.py might be bundled. * owns the directories it creates. * doesn't own any directories it shouldn't. * no duplicates in %files. * file permissions are appropriate. * no generically named files Thanks, and I appreciate the discussion and recommendations. I don't take it as a demand for changes or anything. I've updated the License comment. Regarding pkgutil.py ... basically it is required to achieve python 2.4 compatibility in RHEL 5 (this is destined for EPEL 5 as well as Fedora). The pkgutil library was introduced after Python 2.4 and is required by Cement to handle Genshi text file templating. I would be more than happy to remove it in %prep for Python >= 2.6 ... but that wouldn't change the license or any functionality (as it isn't used when Python >= 2.6). SPEC: http://5dollarwhitebox.org/tmp/python-cement.spec SRPM: http://5dollarwhitebox.org/tmp/python-cement-0.8.14-4.fc14.src.rpm Sorry for the delay; it's a busy time of year. The primary issue with pkgutil.py is its interaction with our guidelines against bundling code that should be packaged separately (or is already packaged separately). Those guidelines are at http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries and https://fedoraproject.org/wiki/Packaging:Treatment_Of_Bundled_Libraries https://fedoraproject.org/wiki/Packaging:Treatment_Of_Bundled_Libraries#Conditionalized_functionality seems to me to cover the current situation explicitly. Can you verity that the bundled pkgutil.py is not used at all for sufficiently new python, and add the comment required by that guideline? With that, I think we're done. I also apologize for the delay do to the holidays. I've updated the spec with a comment regarding the bundled 'pkgutil': SPEC: http://5dollarwhitebox.org/tmp/python-cement.spec SRPM: http://5dollarwhitebox.org/tmp/python-cement-0.8.14-5.fc14.src.rpm $ rpmlint -i SPECS/python-cement.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. $ rpmlint -i /var/lib/mock/fedora-14-x86_64/result/python-cement*0.8.14-5*.rpm python-cement-devtools.noarch: E: zero-length /usr/lib/python2.7/site-packages/cement/paste/templates/cementapp/LICENSE python-cement-devtools.noarch: E: zero-length /usr/lib/python2.7/site-packages/cement/paste/templates/cementplugin/LICENSE 4 packages and 0 specfiles checked; 2 errors, 0 warnings.. Thanks, looks good. APPROVED New Package SCM Request ======================= Package Name: python-cement Short Description: CLI Application Framework for Python Owners: derks Branches: f13 f14 el5 el6 InitialCC: Git done (by process-git-requests). python-cement-0.8.14-5.fc14 has been submitted as an update for Fedora 14. https://admin.fedoraproject.org/updates/python-cement-0.8.14-5.fc14 python-cement-0.8.14-5.fc13 has been submitted as an update for Fedora 13. https://admin.fedoraproject.org/updates/python-cement-0.8.14-5.fc13 python-cement-0.8.14-5.fc13 has been pushed to the Fedora 13 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update python-cement'. You can provide feedback for this update here: https://admin.fedoraproject.org/updates/python-cement-0.8.14-5.fc13 python-cement-0.8.14-5.fc14 has been pushed to the Fedora 14 stable repository. If problems still persist, please make note of it in this bug report. python-cement-0.8.14-5.fc13 has been pushed to the Fedora 13 stable repository. If problems still persist, please make note of it in this bug report. |