Skip to content

Build: Added HDF5PLUGIN_SYSTEM_LIBRARIES env. var. to build using system libraries #333

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

Merged
merged 12 commits into from
Mar 26, 2025

Conversation

t20100
Copy link
Member

@t20100 t20100 commented Mar 14, 2025

This PR reworks the management of C libraries in setup.py to:

@t20100 t20100 added this to the Next release milestone Mar 14, 2025
@t20100 t20100 marked this pull request as draft March 14, 2025 15:01
@t20100 t20100 marked this pull request as ready for review March 21, 2025 15:48
@t20100
Copy link
Member Author

t20100 commented Mar 21, 2025

Tested with conda: no longer link with available system libs if not requested + use them if requested through the HDF5PLUGIN_SYSTEM_LIBRARIES env. var. (the corresponding pkg-config .pc file must be available).

@t20100
Copy link
Member Author

t20100 commented Mar 21, 2025

setuptools is really not appropriate for building pure-C/C++ dynamic libs such as the hdf5 filter plugins.

@vasole
Copy link
Member

vasole commented Mar 21, 2025

Comment on lines +702 to +703
if field in ('extra_link_args', 'libraries'):
return []
Copy link
Member

Choose a reason for hiding this comment

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

'side question' not sure I get this part. Why is this returning a list ? I would expect an empty dict.

Copy link
Member Author

Choose a reason for hiding this comment

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

If field is None, it returns a dict which contains the build config for the static lib which is passed to BuildCLib setuptools command.
If field is provided, it returns the value of the field. It is called with way from the hdf5 plugin code using the compression lib to get what needs to be passed for linking with the compression lib to the setuptools Extension. Extension expects lists for 'extra_link_args' and 'libraries'.

It cannot be all stored in a dict since 'extra_link_args' and 'libraries' are not expected by BuildCLib.
It would probably be best to use a class/dataclass/namedtuple here, or a function to extract what's relevant to BuildCLib. Here I've minimised the rewrite, but I agree it makes things complex

@t20100 t20100 marked this pull request as draft March 25, 2025 09:51
@t20100
Copy link
Member Author

t20100 commented Mar 25, 2025

Let's merge PR #335 first

@t20100 t20100 force-pushed the use-system-libs branch 2 times, most recently from 95b258f to 750b8ab Compare March 25, 2025 16:10
@t20100 t20100 marked this pull request as ready for review March 26, 2025 13:46
@t20100 t20100 merged commit 80a70eb into silx-kit:main Mar 26, 2025
7 checks passed
@t20100 t20100 deleted the use-system-libs branch March 26, 2025 14:22
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.

hdf5plugin build against dynamic system libraries instead of embedded static ones Add a way to build with system libraries
3 participants