Bug 560456

Summary: Review Request: python3-smbpasswd - Python SMB Password Hash Generator Module
Product: [Fedora] Fedora Reporter: Jochen Schmitt <jochen>
Component: Package ReviewAssignee: Thomas Spura <tomspur>
Status: CLOSED DEFERRED QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: dmalcolm, fedora-package-review, notting, tomspur
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2010-02-14 11:07:40 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Bug Depends On:    
Bug Blocks: 530636    

Description Jochen Schmitt 2010-01-31 19:04:10 UTC
Spec URL: http://www.herr-schmitt.de/pub/python3-smbpasswd/python3-smbpasswd.spec
SRPM URL:  http://www.herr-schmitt.de/pub/python3-smbpasswd/python3-smbpasswd-1.0.1-1.fc12.src.rpm
Description: 
This package contains a python module, which is able to generate LANMAN and
NT password hashes suiteable to us with Samba.

This is a ported release for python3

Comment 1 Thomas Spura 2010-02-01 01:33:15 UTC
Hmm, is there any reason, why you want a different package for python3?

You could also use some macros and build this in the python-smbpasswd (what you already maintain).

See:
https://fedoraproject.org/wiki/PackagingDrafts/Python3


If you want to have this as an extra package, I'll review this. Just give me a ping ;)

But if I'd be in you place, I'd build it within the python2 package...

Comment 2 Dave Malcolm 2010-02-01 15:59:40 UTC
Thanks for putting this package together.  I noticed that Fedora ships a Python 2 version of this module; see:
https://admin.fedoraproject.org/pkgdb/packages/name/python-smbpasswd

We're still experimenting with our Python 3 packaging.  As Thomas notes, see:
https://fedoraproject.org/wiki/PackagingDrafts/Python3  That page is still a work in progress, so please let us know if there's anything there that seems wrong.

Where did python-smbpasswd-1.0.1-py3.patch come from?  If you did the porting work yourself, bravo!  Have you approached the upstream maintainer about Fedora distributing a Python 3 rpm of this code?  We want to work as closely as possible with upstream.    Be sure to tell upstream that you've got it working with Python 3, ideally with a simple example of usage (though looking at changelogs in the code, I wonder if the upstream project is still alive).

For what it's worth, this fragment from python-smbpasswd-1.0.1-py3.patch should work with both python 2.6 and python 3.1:

diff -up py-smbpasswd-1.0.1/smbpasswd.c.org py-smbpasswd-1.0.1/smbpasswd.c
--- py-smbpasswd-1.0.1/smbpasswd.c.org  2004-12-16 04:54:08.000000000 +0100
+++ py-smbpasswd-1.0.1/smbpasswd.c      2010-01-28 21:06:32.199624298 +0100
@@ -70,7 +70,7 @@ hash_to_string(char *tmp) 
         outbuffer[(i*2)+1] = HEXCHARS[   c   & 0x0f];
         }
         
-    return PyString_FromStringAndSize(outbuffer, 32);
+    return PyBytes_FromStringAndSize(outbuffer, 32);
     }
 
since /usr/include/python2.6/bytesobject.h provides:
  #define PyBytes_FromStringAndSize PyString_FromStringAndSize
and is included by /usr/include/python2.6/Python.h

So we could build both Python 2 and Python 3 versions of the module with that hunk from the patch, and the shebang fix could be done in a post-processing step.

Given that both Python 2 and Python 3 support can be built from identical sources, I feel that it would be better to integrate the Python 3 support into the pre-existing Python 2 SRPM as a new subpackage, rather than to create a new SRPM.  See the notes on 
https://fedoraproject.org/wiki/PackagingDrafts/Python3
for rationale, and our current ideas on the best ways of doing this.  Again, it's good to provide an example that shows the code working when requesting the change this time from the Fedora package maintainer.

Hope this is helpful

Comment 3 Dave Malcolm 2010-02-01 16:22:49 UTC
(In reply to comment #2)
> Thanks for putting this package together.  I noticed that Fedora ships a Python
> 2 version of this module; see:
> https://admin.fedoraproject.org/pkgdb/packages/name/python-smbpasswd

[snip]

> Given that both Python 2 and Python 3 support can be built from identical
> sources, I feel that it would be better to integrate the Python 3 support into
> the pre-existing Python 2 SRPM as a new subpackage, rather than to create a new
> SRPM.  See the notes on 
> https://fedoraproject.org/wiki/PackagingDrafts/Python3
> for rationale, and our current ideas on the best ways of doing this.  Again,
> it's good to provide an example that shows the code working when requesting the
> change this time from the Fedora package maintainer.

I looked at the specfile %changelog, and realize now that you're the Fedora maintainer for the Python 2 version of this package.

Given how minimal the changes needed to port this were, I still think that the best approach here is to add a python3- subpackage to the existing python-smbpasswd.spec (sending the upstream maintainer the patch).

Comment 4 Thomas Spura 2010-02-14 11:07:40 UTC
This is now build within the python-smbpasswd package.
http://koji.fedoraproject.org/koji/buildinfo?buildID=154154