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
Search for libblosc2 with pkg-config if not in the python blosc2 installation #1000
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
cfbe1f5
search for libblosc2 with pkg-config if not in the python blosc2 inst…
bnavigator 31c5ddf
remove blosc2 from runtime requirements
bnavigator 4c4eb91
Update build and runtime requirement specs
bnavigator 3579028
only build one sdist for CI
bnavigator bbb02dd
Enhance blosc2 library search
bnavigator 02ea1ab
Only use pkg-config for Unix-like systems
bnavigator f16c76f
Make sure the blosc2 wheel or conda pkg is not installed
bnavigator ee726ff
Bump minimum c-blosc2 version
bnavigator File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,8 @@ | ||
# Keep in sync with tables/req_versions.py and | ||
# doc/source/usersguide/installation.rst *and* | ||
# Build requirements | ||
# Keep in sync with doc/source/usersguide/installation.rst *and* | ||
# pyproject.toml | ||
cython>=0.29.21 | ||
numpy>=1.19.0 | ||
numexpr>=2.6.2 | ||
# blosc2 wheel is actually only needed when compiling on conda envs. | ||
# Otherwise, lib comes bundled in PyTables wheels (but it doesn't hurt either). | ||
blosc2~=2.0.0 | ||
packaging | ||
py-cpuinfo | ||
# provide blosc2 wheel or c-blosc2 headers externally! |
This file contains 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
This file contains 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
This file contains 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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see a few problems:
find_library()
returns None if it doesn't find the library, andcdll.LoadLibrary(None)
doesn't DTRT:That loop needs an additional
if blosc2_lib
check.Loading of library from $PWD creates the obvious security issue where an attacker can place a file in $PWD and cause the user to load it. Loading from $PWD can be done in a test environment, but MUST NOT be done in production code.
find_library()
doesn't seem to fit the bill. Quoting the docs: "The purpose of the find_library() function is to locate a library in a way similar to what the compiler or runtime loader does (on platforms with several versions of a shared library the most recent should be loaded)". It would find the newest version of the library, which might or might not be what is needed. The whole point of the SONAME field (i.e. the thing that gives the .2 suffix) is to refer to a specific version from code using the library, so that when a future incompatible version appears, existing code is not broken.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://src.fedoraproject.org/rpms/python-tables/blob/rawhide/f/0007-Drop-misguided-check.patch
This is the patch I pushed in Fedora. It ignores non-Linux systems, so it's not suitable for inclusion upstream.