Bug 700667

Summary: Review Request python26-crypto
Product: [Fedora] Fedora EPEL Reporter: Andy Grimm <agrimm>
Component: Package ReviewAssignee: Paul Howarth <paul>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: el5CC: dmalcolm, fedora-package-review, gholms, jgoulding, paul, tcallawa
Target Milestone: ---Flags: paul: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: python26-crypto-2.3-5.el5 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2011-07-08 16:01:49 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: 702677    
Attachments:
Description Flags
Some simplifications
none
Revised Simplifications none

Description Andy Grimm 2011-04-28 22:37:22 UTC
SPEC:
http://www.grimmslanding.org/rpms/python26-crypto.spec

SRPM:
http://www.grimmslanding.org/rpms/python26-crypto-2.0.1-4.el5.2.src.rpm

python-crypto already exists in EPEL, and this is simply an adaptation of that source to provide the same functionality for python 2.6.

I left the Release value as it was in the python-crypto package.  If this is not the correct thing to do, let me know.

Comment 1 Tom "spot" Callaway 2011-04-30 18:23:43 UTC
Is it practical to carry this as a separate package? Given that the version and code is identical, perhaps this could be added to the python-crypto package (in EPEL) as a subpackage, much in the same way that some python3 packages are generated.

Comment 2 Andy Grimm 2011-04-30 19:03:41 UTC
If that's an accepted practice in EPEL, then by all means, it can be added as a subpackage.  I do not expect the source of this package to diverge from python-crypto at all.  Should I just contact the maintainers of that package?

Comment 3 Tom "spot" Callaway 2011-04-30 21:25:58 UTC
Yeah, I would only pursue this route if the python-crypto maintainer objects.

Comment 4 Paul Howarth 2011-05-04 09:38:59 UTC
(python-crypto maintainer here)

Another possibility, given that python26-crypto doesn't currently exist and the EPEL-5 python-crypto package is quite old (2.0.1; current upstream is 2.3) would be to make the new python26-crypto package use the current upstream release, as that is likely to be more useful to python26 users?

That would of course need a new review as it would be a new package rather than a subpackage of the existing python-crypto package.

I've no objection to the subpackage approach other than that I think a more recent version would be a better option going forward given that it's essentially a new package.

Comment 5 Andy Grimm 2011-05-04 16:15:40 UTC
That's fine.  It makes little difference to me, as I only need it as a prerequisite for paramiko.  I can test out a 2.3 build and post that.

Comment 6 Andy Grimm 2011-05-05 16:53:59 UTC
Updated to 2.3 and posted here (and my apologies for Google's content-type issue on the spec file):

SRPM:
https://www.grimmslanding.org/rpms/python26-crypto-2.3-4.el5.src.rpm

SPEC:
https://www.grimmslanding.org/rpms/python26-crypto.spec

I tried to change the spec as little as possible from the current version in he python-crypto master branch.  However, filter_provides_in obviously doesn't do anything in rpm 4.4.x, and I wasn't sure whether I should leave that in.  Also, I thought about overriding _provides to white out the shared lib deps, but did not.  (This practice is mentioned in http://fedoraproject.org/wiki/Features/BetterRpmAutoReqProvFiltering , but it doesn't seem to be common in EPEL).
Thoughts?

Comment 7 Paul Howarth 2011-05-05 18:36:29 UTC
(In reply to comment #6)
> Updated to 2.3 and posted here (and my apologies for Google's content-type
> issue on the spec file):
> 
> SRPM:
> https://www.grimmslanding.org/rpms/python26-crypto-2.3-4.el5.src.rpm
> 
> SPEC:
> https://www.grimmslanding.org/rpms/python26-crypto.spec
> 
> I tried to change the spec as little as possible from the current version in he
> python-crypto master branch.  However, filter_provides_in obviously doesn't do
> anything in rpm 4.4.x, and I wasn't sure whether I should leave that in.  Also,
> I thought about overriding _provides to white out the shared lib deps, but did
> not.  (This practice is mentioned in
> http://fedoraproject.org/wiki/Features/BetterRpmAutoReqProvFiltering , but it
> doesn't seem to be common in EPEL).
> Thoughts?

To filter those you'd need to manually include the provides filter included in /usr/lib/rpm/redhat/macros (redhat-rpm-config package) in Fedora. I wouldn't bother, and I'd strip out the existing provides filtering code as it has no effect in EPEL < 6.

Comment 8 Paul Howarth 2011-05-05 20:38:30 UTC
Created attachment 497224 [details]
Some simplifications

I think there are some other simplifications that can be made too:

%pybasever is defined in the spec so its use doesn't need to be conditionalized

%pythonver can be replaced by %pybasever since they have the same value

No need to evaluate the python-abi value as we're defining it as %pybasever ourselves

No need to specify a version requirement for python26-devel as any version will be OK

Attached patch implements these suggestions.

Comment 9 Andy Grimm 2011-05-06 13:44:35 UTC
Created attachment 497358 [details]
Revised Simplifications

All of your suggestions make sense; the reason I left pythonver from the original recipe was to keep things as similar as possible to the python-crypto master (it might make merges easier later).  Rather than remove it and alter the %install section, I replaced pybasever with pythonver in this similar patch.

Comment 10 Andy Grimm 2011-05-11 10:13:13 UTC
I'm not sure what to do next on this issue.  Do I need to post a spec / srpm with either Paul's patch or mine applied?  Does anyone have feedback on my patch?  Thanks.

Comment 11 Paul Howarth 2011-05-11 13:51:29 UTC
I've updated python-crypto in Rawhide to use upstream's re-rolled tarball and to get rid of macros for system commands. Rebase your spec on that and then I'll review it when I can find a few spare jiffies.

Comment 12 Andy Grimm 2011-05-11 15:38:53 UTC
Ah, I did see the comment in email about macros longer than the things they replace.  :-)  So now we have:

SPEC:
http://www.grimmslanding.org/rpms/python26-crypto.spec

SRPM:
http://www.grimmslanding.org/rpms/python26-crypto-2.3-5.el5.src.rpm

Thanks.

Comment 13 Paul Howarth 2011-05-17 15:19:43 UTC
rpmlint output
==============
python26-crypto.x86_64: W: file-not-utf8 /usr/share/doc/python26-crypto-2.3/LEGAL/copy/stmts/Paul_Swartz.mbox
python26-crypto.x86_64: E: backup-file-in-package /usr/share/doc/python26-crypto-2.3/LEGAL/copy/LICENSE.orig

The first of these is a set of emails, with explicit encoding.
The second is a false positive - the file originates upstream and not from a patch.

review checklist
================
- rpmlint OK (see above)
- package and spec naming follows convention for python26 packages in EPEL
- package meets guidelines
- license is OK for Fedora and matches upstream (see upstream COPYRIGHT file)
- upstream licensing texts included in %doc
- spec file written in English and is legible
- source matches upstream
- package builds find in mock for EPEL-5
- buildreqs ok
- no locale data to worry about
- package does not include shared libraries in dynamic linker's default paths
- package does not bundle copies of system libraries
- package is not intended to be relocatable
- no directory ownership issues
- no duplicate files
- file permissions are fine
- macro usage is consistent
- code, not content
- no large docs included
- docs don't affect runtime
- no devel files included
- not a GUI app so no desktop file needed
- filenames are all valid UTF-8
- upstream test suite and benchmark script run in %check
- no scriptlets needed or included
- no subpackages warranted or created
- no file dependencies
- no binaries/scripts included, so no manpages warranted

All in all, no problems found.

APPROVED.

Comment 14 Garrett Holmstrom 2011-05-17 17:21:00 UTC
Andy, before you request any branches you need to get sponsored into the packager group.  Thankfully, Paul is a sponsor, so all you have to do is convince him to sponsor you.  :)

Blocking FE-NEEDSPONSOR...

Comment 15 Tom "spot" Callaway 2011-06-13 19:12:58 UTC
I just sponsored Andy. Lifting FE-NEEDSPONSOR

Comment 16 Andy Grimm 2011-06-13 20:36:16 UTC
New Package SCM Request
=======================
Package Name: python26-crypto
Short Description: Cryptography library for Python 
Owners: arg gholms
Branches: el5
InitialCC:

Comment 17 Gwyn Ciesla 2011-06-13 22:12:16 UTC
Git done (by process-git-requests).

Comment 18 Fedora Update System 2011-06-14 18:26:39 UTC
python26-crypto-2.3-5.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/python26-crypto-2.3-5.el5

Comment 19 Fedora Update System 2011-06-16 00:30:56 UTC
python26-crypto-2.3-5.el5 has been pushed to the Fedora EPEL 5 testing repository.

Comment 20 Fedora Update System 2011-07-08 16:01:42 UTC
python26-crypto-2.3-5.el5 has been pushed to the Fedora EPEL 5 stable repository.