Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix library/header detection in system default paths #1073

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nega0
Copy link

@nega0 nega0 commented Nov 1, 2023

setup.py uses compiler.has_function() to test if a dependency exists in the system's "default paths". Unfortunately it immediately discards the result of that test. This PR fixes that and adds a corresponding output message.

This has the added benefit that bzip2 detection "just works" now.

setup.py uses `compiler.has_function()` to test if a dependency exists
in the system's "default paths". Unfortunately it immediately discards
the result of that test. This commit fixes that and adds a corresponding
output message.
@nega0 nega0 marked this pull request as ready for review November 1, 2023 21:45
if hdrdir is True and libdir is True:
print(f"* Found {package.name} in the standard system search dirs.")
# set header dir to one of the default include dirs to fudge checks below
hdrdir = default_dirs.header[0]
Copy link
Member

Choose a reason for hiding this comment

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

Not totally sure that this approach is safe.
E.g. on Debian systems (and derivatives) it is possible to install the library (.so) without header files (which are normally provided in a different Debian package).
This kind of situation IMHO would result in an error at build time, and, considering that PyTables takes a while for building, it would be better to raise an error as early as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants