FIX: Use dynamic_lookup for Python Library in macOS #98
Conversation
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
… bewithgaurav/fix_mac_python_lib_linker
There was a problem hiding this comment.
Pull Request Overview
This PR simplifies and standardizes the linking process for Python symbols on macOS by always using the dynamic lookup flag, eliminating conditional logic previously required to link against the Python framework or a Python library.
- Removed conditional branching for library vs. framework linking.
- Always applies the -undefined dynamic_lookup flag to support various Python environments.
Comments suppressed due to low confidence (2)
mssql_python/pybind/CMakeLists.txt:244
- Include a brief note clarifying why eliminating the conditional logic improves compatibility across Python environments to assist future maintainers.
message(STATUS "Using dynamic lookup for Python symbols on macOS (best practice for Python extensions)")
mssql_python/pybind/CMakeLists.txt:245
- Review whether linking against 'dl' (done on the subsequent line) is still required when using dynamic_lookup on macOS, and remove it if redundant.
set_target_properties(ddbc_bindings PROPERTIES LINK_FLAGS "-undefined dynamic_lookup")
jahnvi480
approved these changes
Jun 27, 2025
gargsaumya
approved these changes
Jun 27, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This pull request simplifies and standardizes the linking process for Python symbols on macOS in the
mssql_python/pybind/CMakeLists.txtfile. The change ensures compatibility across all Python environments, including virtual environments, by always using dynamic lookup for Python symbols.MacOS-specific changes:
mssql_python/pybind/CMakeLists.txt: Removed conditional logic for linking against the Python framework or a specific Python library. Instead, the code now always uses the-undefined dynamic_lookupflag for Python symbols, ensuring better compatibility across Python environments.Issue Reference
Fixes AB#37773
Fixes the GH Issue - #99
###Additional context
The root cause is that the C extension binary was built with a hardcoded path to the Python framework at
/Library/Frameworks/Python.framework/Versions/3.13/Python. This can be verified usingotool -Lon the .so file:The fix is to modify
CMakeLists.txtto use the-undefined dynamic_lookuplinking approach, which is the recommended practice for Python extensions on macOS. This ensures the extension works regardless of where or how Python is installed.