Bug 1241412 - Review Request: python-ldap3 - Strictly RFC 4511 conforming LDAP V3 pure Python client
Summary: Review Request: python-ldap3 - Strictly RFC 4511 conforming LDAP V3 pure Pyth...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Matej Stuchlik
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2015-07-09 07:48 UTC by Michal Cyprian
Modified: 2016-02-01 02:16 UTC (History)
4 users (show)

Fixed In Version: python-ldap3-0.9.8.6-1.fc23
Clone Of:
Environment:
Last Closed: 2015-08-10 10:07:00 UTC
Type: ---
Embargoed:
mstuchli: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)
Patch to try and fix build issues (3.79 KB, patch)
2015-07-15 18:47 UTC, Jason Tibbitts
no flags Details | Diff

Description Michal Cyprian 2015-07-09 07:48:09 UTC
Spec URL: https://mcyprian.fedorapeople.org/ldap3.spec
SRPM URL: https://mcyprian.fedorapeople.org/ldap3-0.9.8.6-1.fc22.src.rpm
Description: python-ldap3 is a strictly RFC 4511 conforming LDAP V3 pure Python client. The same codebase works with Python, Python 3, PyPy and PyPy3.
Fedora Account System Username: mcyprian

Comment 1 Michal Cyprian 2015-07-09 09:29:00 UTC
I've realized I forgot to update my Spec file name after I had changed name of the package. Here are updated links to the files:

Spec URL: https://mcyprian.fedorapeople.org/python-ldap3.spec
SRPM URL: https://mcyprian.fedorapeople.org/python-ldap3-0.9.8.6-1.fc22.src.rpm

Comment 2 Matej Stuchlik 2015-07-09 11:54:38 UTC
* You are mixing the use of spaces and tabs. For instance version is indented with spaces, whereas release is indented with tabs. To avoid this in future, I advise putting the following into your .vimrc:

:set tabstop=4
:set shiftwidth=4
:set expandtab

* Since the package does not contain any architecture specific files, it should be marked noarch

* The utils directory contains backport of ssl.match_hostname, ssl._dnsname_match_backport and CertificateError. You'll want to remove these, patch the code so that it uses the backports-ssl_match_hostname package instead, when necessary, and try to get the patch accepted upstream. Good luck! :)

Comment 3 Michal Cyprian 2015-07-09 15:07:36 UTC
Thank you, I've added "BuildArch:  noarch" to Spec file, I created patch that fixes backports problem and I've sent pull request to upstream.

Links to new versions of files:
Spec URL: https://mcyprian.fedorapeople.org/python-ldap3.spec
SRPM URL: https://mcyprian.fedorapeople.org/python-ldap3-0.9.8.6-1.fc22.src.rpm

Comment 4 Jason Tibbitts 2015-07-15 18:12:40 UTC
I really need a python3 ldap library and so pulled this, but unfortunately it doesn't build due to a bad patch.  Actually it doesn't even prep.  How did you get your package to compile?

+ /usr/bin/patch -p1 --fuzz=0
can't find file to patch at input line 5
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/docs/manual/source/ldap3.utils.rst b/docs/manual/source/ldap3.utils.rst
|index c29271f..29ef33a 100644
|--- a/docs/manual/source/ldap3.utils.rst
|+++ b/docs/manual/source/ldap3.utils.rst
--------------------------
File to patch:
Skip this patch? [y]
Skipping patch.
1 out of 1 hunk ignored
The next patch would delete the file docs/manual/source/ldap3.utils.tls_backport.rst,
which does not exist!  Assume -R? [n]
Apply anyway? [n]
Skipping patch.
1 out of 1 hunk ignored
patching file ldap3/core/tls.py
patching file ldap3/utils/tls_backport.py
RPM build errors:
error: Bad exit status from /var/tmp/rpm-tmp.0IUyQh (%prep)

Comment 5 Jason Tibbitts 2015-07-15 18:46:12 UTC
I'll attach a patch which I used to get this to sort of build and run checks.  Or it would actually do all of that, except that the module requires pyasn1 >= 0.1.8 and Fedora has only 0.1.7.

I'll see about pushing an update for that today, but I'm still not sure how you were able to build and test this package at all.

Comment 6 Jason Tibbitts 2015-07-15 18:47:36 UTC
Created attachment 1052408 [details]
Patch to try and fix build issues

My editor may have stripped some whitespace, too.  Sorry about that.  I didn't bump the release or add a changelog, so you'll need to do that before posting another package.

Comment 7 Jason Tibbitts 2015-07-16 01:39:40 UTC
My apologies; the %check section I included is completely wrong.  The tests can be run using nosetest-2.7 and nosetest-3.4, but most of them will fail because they must connect to an ldap server.  There are probably some tests in there which we could run but it would be a bit of work to patch out the ones which we can't.

Also, I did update pyasn1 but got yelled at for it.  Oh, well; it will be in rawhide and F23 soon unless the maintainer decides to revert my trivial update and untag the package.  No idea if he'll update the release branches, either.

Comment 8 Michal Cyprian 2015-07-16 08:51:51 UTC
It was really not possible to compile this package, thank you for the notice. Patch was made for GitHub sources and I tried to apply it to ldap3 sources from PyPI. I didn't test it properly, I am sorry. I think %check section is not necessary here. I've made new patch that works with PyPI sources.

Spec URL: https://mcyprian.fedorapeople.org/python-ldap3.spec
SRPM URL: https://mcyprian.fedorapeople.org/python-ldap3-0.9.8.6-1.fc22.src.rpm

It should be alright now.

Comment 9 Matej Stuchlik 2015-07-29 11:52:09 UTC
Looks good to me now, good job :) and thanks for the input, Jason, I did not notice that issue either :)

Comment 10 Michal Cyprian 2015-07-29 12:06:38 UTC
New Package SCM Request
=======================
Package Name: python-ldap3
Short Description: Strictly RFC 4511 conforming LDAP V3 pure Python client
Upstream URL: https://pypi.python.org/pypi/ldap3/0.9.8.7
Owners: mcyprian
Branches: f23

Comment 11 Gwyn Ciesla 2015-07-29 16:51:31 UTC
Git done (by process-git-requests).

Comment 12 Jason Tibbitts 2015-07-29 17:15:11 UTC
No problem; I just happened to really need an LDAP library for python3 and was amazed that we didn't have one packaged.  I've been using this for a while (on F21, with updated dependencies of course).  Thanks for finishing the review.  And thanks to Michal for maintaining it in the first place.

Comment 13 Fedora Update System 2015-07-30 08:53:58 UTC
python-ldap3-0.9.8.6-1.fc23 has been submitted as an update for Fedora 23.
https://admin.fedoraproject.org/updates/python-ldap3-0.9.8.6-1.fc23

Comment 14 Fedora Update System 2015-07-30 17:30:12 UTC
python-ldap3-0.9.8.6-1.fc23 has been pushed to the Fedora 23 testing repository.

Comment 15 Fedora Update System 2015-08-10 10:07:00 UTC
python-ldap3-0.9.8.6-1.fc23 has been pushed to the Fedora 23 stable repository.


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