Skip to content
This repository has been archived by the owner on Dec 14, 2021. It is now read-only.

setup.py: Discover liblzma paths via pkg-config. #11

Closed
wants to merge 2 commits into from

Conversation

Bluehorn
Copy link

@Bluehorn Bluehorn commented Jun 5, 2015

I was unable to build backports.lzma on a RHEL5 system. RHEL5 does not include distribution packages for liblzma, therefore it was installed locally.

The usual way on Linux systems to discover libraries for building is to query the pkg-config tool. Unfortunately, the setup.py in backports.lzma does not do this yet.

This patch adds pkg-config integration. To stay compatible the pkg-config output is only added as last resort. IMHO it should be the first location to look at, but that's not for me to decide. With this patch backports.lzma builds fine for my case.

@peterjc
Copy link
Owner

peterjc commented Feb 7, 2017

Sorry @Bluehorn, I seem to have completely missed this pull request. I have never used pkg-config and am not familiar with it. As written this would also fail on the Mac.

See also #18, @jakirkham can you comment on this alternative idea?

@jakirkham
Copy link
Contributor

If you want to use pkg-config from Python, it might be worth considering pkgconfig (unfortunately very similarly named though different). This is a nice Python wrapper for using the pkg-config CLI to check these locations. Not sure if optionally allowing this dependency is a possibility.

@peterjc
Copy link
Owner

peterjc commented Feb 7, 2017

Adding a Python dependency does seem unfortunate, especially as anyone using this backport would be doing it on an older system (less likely to have pip etc).

@jakirkham
Copy link
Contributor

It just feels funny to try to reproduce the work that pkgconfig developers already went to by implementing our own copy here and potentially incurring maintenance burden by doing so.

This is why I was thinking this could be optional (i.e. try...except ImportError). That feels appropriate IMHO as this is a bit of a special case. So we would just expect users to have pkgconfig if they want to do this, but not bother if they don't.

If we want to ensure pkgconfig is always present and do not want to go the setuptools/setup_requires route, an alternative would be to include a vendorized pkgconfig simply for building purposes. We can even excluded it from being installed.

Perhaps a third option is just to add flags to setup.py that allow one to explicitly set the library and include paths to use for linking to LZMA. This avoids having a special relationship with pkg-config and allows a user to set these whatever way they wish.

@peterjc
Copy link
Owner

peterjc commented Feb 7, 2017

If there's a common or typical way to specify the optional paths which we can follow, that would be sensible.

@tlandschoff-scale
Copy link

If you want to use pkg-config from Python, it might be worth considering pkgconfig

Unfortunately adding any dependency to setup.py basically makes the package uninstallable. pip currently does not care and runs setup.py without any dependencies installed. This makes it impossible (for example) to use Cython for building extensions without including the generated C source files.

The relevant issue in pip is this one: pypa/pip#1820
This has been resolved by adding a new optional pyproject.toml configuration file. A package that requires e.g. pkgconfig for building can not declare this in that file.

However, this seems to be a quite new feature of pip and I'd rather not make use of it until it is standard on most Python installations. Our build system routinely upgrades pip before installing anything else, but not everybody is doing that.

And running pkg-config using subprocess.Popen is not really a big deal IMHO.

@peterjc
Copy link
Owner

peterjc commented Dec 14, 2021

I'm about to archive this repository, and so am closing the outstanding issues and pull requests. Thank you for your efforts though.

@peterjc peterjc closed this Dec 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants