FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix macOS SDK builds without ldap_init_fd by tiran · Pull Request #360 · python-ldap/python-ldap · GitHub

/ python-ldap Public

Fix macOS SDK builds without ldap_init_fd - #360

Merged
encukou merged 4 commits into
python-ldap:masterfrom
tiran:travis_macosx
Jun 29, 2020
ConversationCommits (4)ChecksFiles changed
Merged

Fix macOS SDK builds without ldap_init_fd#360
encukou merged 4 commits into
python-ldap:masterfrom
tiran:travis_macosx

Conversation

tiran commented Jun 20, 2020

Copy link
Copy Markdown
Member

macOS system libldap 2.4.28 does not have ldap_init_fd symbol. Disable
initialize_fd when Apple libldap 2.4.28 is detected.

Also run some macOS tests on Travis CI. Since the SDK does not ship
slapd, testing is rather limited.

Fixes: #359
Signed-off-by: Christian Heimes cheimes@redhat.com

tiran added the bug label Jun 20, 2020
tiran mentioned this pull request Jun 20, 2020
Closed

encukou commented Jun 22, 2020

Copy link
Copy Markdown
Member

Could you also document when the functionality is available?

tiran commented Jun 22, 2020 •
edited
Loading

Copy link
Copy Markdown
Member Author

Could you also document when the functionality is available?

I would like to but I don't know. OpenLDAP 2.4.28 has ldap_init_fd. It was introduced around 2.4.0. I have no clue why the symbol is missing in the Apple builds. It could be patched out or hidden by a downstream patch.

encukou commented Jun 22, 2020

Copy link
Copy Markdown
Member

From the patch, I gather that on a Mac, you won't get fileno support even if you compile OpenLDAP 2.4.28 yourself.

tiran commented Jun 22, 2020

Copy link
Copy Markdown
Member Author

From the patch, I gather that on a Mac, you won't get fileno support even if you compile OpenLDAP 2.4.28 yourself.

2.4.28 was released over 8 years ago. I don't we have to care about ancient versions except for Apple system builds.

I'm also open to better ways to detect system libldap on macOS. I don't have access to macOS to investigate the issue.

encukou commented Jun 22, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

I'm OK with the patch itself. I just think it needs a note in the documentation that says when the feature is available. From the current text, it seems that fileno is always usable.

Also, you've now rebased onto master, which I don't understand. Shouldn't this go into 3.3.1?

tiran commented Jun 23, 2020

Copy link
Copy Markdown
Member Author

I'm OK with the patch itself. I just think it needs a note in the documentation that says when the feature is available. From the current text, it seems that fileno is always usable.

I have updated documentation to mention INIT_FD_AVAIL and the macOS issue.

Also, you've now rebased onto master, which I don't understand. Shouldn't this go into 3.3.1?

The fix should land in master and 3.3.1. New features land in master and then are back-ported to bug fix branches.

tiran commented Jun 23, 2020

Copy link
Copy Markdown
Member Author

Please merge #361 and #362 before this PR.

encukou commented Jun 29, 2020

Copy link
Copy Markdown
Member

New features land in master and then are back-ported to bug fix branches.

That way makes it harder to merge. It is the CPython workflow, but this repo doesn't have Miss Islington like CPython.

tiran added 3 commits June 29, 2020 13:47
Fix macOS SDK builds without ldap_init_fd
da6e049 
macOS system libldap 2.4.28 does not have ldap_init_fd symbol. Disable
initialize_fd when Apple libldap 2.4.28 is detected.

Also run some macOS tests on Travis CI. Since the SDK does not ship
slapd, testing is rather limited.

Fixes: python-ldap#359
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Mention macOS issue in doc
29aebba
Debug macos build
b4a4c17

tiran commented Jun 29, 2020

Copy link
Copy Markdown
Member Author

That way makes it harder to merge. It is the CPython workflow, but this repo doesn't have Miss Islington like CPython.

I use the same workflow in all projects. Patches land in master first and then get backported to maintenance branches. The patch would have conflicted either way.

To make it easier for you I have created a backport branch, #363

encukou previously approved these changes Jun 29, 2020
View reviewed changes

encukou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thank you! All looks good except a missing word in the docs.

Comment thread Doc/reference/ldap.rst Outdated
encukou merged commit 605a34b into python-ldap:master Jun 29, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

encukou encukou approved these changes

Assignees

No one assigned

Labels

bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ImportError on MacOS 10.15.5

2 participants

Back | FazBrowse Home | New Git URL