Bug 453684 - Review Request: python-nss - Python bindings for NSS
Review Request: python-nss - Python bindings for NSS
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Dan Horák
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-07-01 17:56 EDT by John Dennis
Modified: 2013-01-09 23:43 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-07-23 08:00:48 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
dan: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description John Dennis 2008-07-01 17:56:00 EDT
Spec URL: http://people.redhat.com/jdennis/python-nss.spec
SRPM URL: http://people.redhat.com/jdennis/python-nss-0.0-1.fc9.src.rpm
Description: Python bindings for Network Security Services (NSS) and Netscape Portable Runtime (NSPR)
Comment 1 Dan Horák 2008-07-02 02:35:40 EDT
review is here, see notes at the end

N/A*	source files match upstream:
OK	package meets naming and versioning guidelines.
OK*	specfile is properly named, is cleanly written and uses macros consistently.
OK	dist tag is present.
OK	build root is correct.
OK	license field matches the actual license.
OK*	license is open source-compatible (GPLv2). License text not included upstream.
N/A	latest version is being packaged.
OK	BuildRequires are proper.
OK	compiler flags are appropriate.
OK	%clean is present.
BAD*	package builds in mock (Rawhide/x86_64).
N/A	debuginfo package looks complete.
N/A*	rpmlint is silent.
N/A	final provides and requires look sane.
N/A	%check is present and all tests pass.
OK	no shared libraries are added to the regular linker search paths.
OK	owns the directories it creates.
OK	doesn't own any directories it shouldn't.
OK	no duplicates in %files.
OK	file permissions are appropriate.
OK	no scriptlets present.
OK	code, not content.
OK	documentation is small, so no -docs subpackage is necessary.
OK	%docs are not necessary for the proper functioning of the package.
OK	no headers.
OK	no pkgconfig files.
OK	no libtool .la droppings.
OK	not a GUI app.


* could you start a project on fedorahosted.org?
http://fedoraproject.org/wiki/Packaging/SourceURL#We_are_Upstream
* could be adding Provides: python-nspr useful for users?
* you should include license text in the package and install it as %doc
* you must use %python_sitearch instead of %python_sitelib
* rpmlint on binary package is not silent 
    - files %python_sitearch/nss/*.so are 0775, should be 0755
    - examples/ssl_example.py is executable and this leads into dependency on
/usr/bin/python => no executable scripts in %doc
Comment 2 John Dennis 2008-07-02 17:16:22 EDT
Thank you for your prompt and thorough review Dan. I have corrected each of the
issues you raised and uploaded new versions of the spec file and srpm at the
url's above.

Your suggestion of hosting this project on fedorahosted is one I would normally
agree with but in this instance the NSS folks would prefer the source code live
in the Mozilla CVS tree along with the rest of the upstream NSS code, which
seems reasonable to me. The upstream NSS folks and I just need to finish that
process, at which time I'll update the spec file with the proper URL (which by
the way is the only rpmlint warning at this point).
Comment 3 Dan Horák 2008-07-03 01:57:26 EDT
Oh, it is completely right to use Mozilla CVS as the upstream location. I just
wanted to assure that some upstream location exists.

I have also notice that -O0 is added at the end of the compile command, but the
reason is understandable. Just don't forget to remove it in the future :-)

What is the purpose of the added Provides: %{name}-%version}? I was talking
about python-nspr in my previous comment, but only when it can be useful.

And last note - please increase the release in each iteration, it makes tracking
the changes easier.
Comment 4 John Dennis 2008-07-03 09:48:05 EDT
I had misunderstood your comment about the Provides. I looked at all the other
python spec files and saw many of them included what looked like a Provides of
their own package which didn't make much sense to me, but I figured there was
some mechanism at play I was unaware of. More careful examination reveals in
those cases the names had subtle variations in spelling, hence the need for the
provides. You are correct the %{name}-%{version} is meaningless, as to whether
Provides: python-nspr would be useful is hard to say at this point. My
expectation is folks will only use the NSPR component if they are using NSS.
Why? Because NSPR is a platform independent wrapper around system services much
like Python and it's libraries are so there is little point in using the NSPR
component of the binding unless you're compelled to because NSS depends on it.
Thus I think the right thing is to remove the provides completely at this
juncture, which I have done.

I will update the files with a new revision number shortly, the only change from
a packaging perspective will be the removal of the Provides.
Comment 5 Dan Horák 2008-07-03 10:59:05 EDT
All issues are fixed or explained, so this PACKAGE is APPROVED.
Comment 6 John Dennis 2008-07-08 12:28:26 EDT
New Package CVS Request
=======================
Package Name: python-nss
Short Description: Python bindings for Network Security Services (NSS)
Owners: jdennis
Branches: F-9
InitialCC: jdennis
Cvsextras Commits: yes
Comment 7 Kevin Fenzi 2008-07-09 12:56:27 EDT
cvs done.
Comment 8 Dan Horák 2008-07-23 05:15:11 EDT
John, you should close the bug after importing and building.
Comment 9 Jesse Keating 2009-07-28 17:25:32 EDT
Discussed with John on IRC, requesting branch for EPEL.

Package Change Request
======================
Package Name: python-nss
New Branches: EL-5
Owners: jkeating
Comment 10 Jason Tibbitts 2009-07-28 17:33:36 EDT
CVS done.

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