Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Fix macOS SDK builds without ldap_init_fd#360

Merged
encukou merged 4 commits into
python-ldap:masterpython-ldap/python-ldap:masterfrom
tiran:travis_macosxtiran/python-ldap:travis_macosxCopy head branch name to clipboard
Jun 29, 2020
Merged

Fix macOS SDK builds without ldap_init_fd#360
encukou merged 4 commits into
python-ldap:masterpython-ldap/python-ldap:masterfrom
tiran:travis_macosxtiran/python-ldap:travis_macosxCopy head branch name to clipboard

Conversation

@tiran

@tiran 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 tiran added the bug label Jun 20, 2020
@tiran tiran mentioned this pull request Jun 20, 2020
Closed
@encukou

encukou commented Jun 22, 2020

Copy link
Copy Markdown
Member

Could you also document when the functionality is available?

@tiran

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

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

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

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

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

tiran commented Jun 23, 2020

Copy link
Copy Markdown
Member Author

Please merge #361 and #362 before this PR.

@encukou

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
@tiran
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>
@tiran
Mention macOS issue in doc
29aebba
@tiran
Debug macos build
b4a4c17
@tiran

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
encukou previously approved these changes Jun 29, 2020
View reviewed changes

@encukou 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.

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

Comment thread Doc/reference/ldap.rst Outdated
@encukou encukou merged commit 605a34b into python-ldap:master Jun 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@encukou 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

@tiran @encukou
Morty Proxy This is a proxified and sanitized view of the page, visit original site.