Skip to content

feat(cmake): pkgconf-pypi entrypoint #5552

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 1 commit into from
Mar 6, 2025
Merged

Conversation

virtuald
Copy link
Contributor

@virtuald virtuald commented Mar 6, 2025

  • This allows pkgconf --cflags pybind11 and similar commands to work as expected if pkgconf is installed from pypi

Description

pkgconf is used by build tools such as meson to find pkg-config files. In a virtualenv, the system pkgconf cannot find pybind11 installed in the virtualenv (though meson has other ways to do this).

https://github.com/pypackaging-native/pkgconf-pypi packages pkgconf into a wheel, and uses entrypoints to extend the PKG_CONFIG_PATH so that .pc files can be found in the virtualenv (docs).

pybind11 already distributes a .pc file in the wheel. This PR adds an entrypoint so that it's usable with pkgconf-pypi.

Suggested changelog entry:

Added support for finding pybind11 using pkgconf distributed on pypi

@virtuald virtuald requested a review from henryiii as a code owner March 6, 2025 05:48
- This allows `pkgconf --cflags pybind11` and similar commands to work as expected
  if pkgconf is installed from pypi
@henryiii henryiii merged commit ded70fe into pybind:master Mar 6, 2025
80 checks passed
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Mar 6, 2025
@virtuald virtuald deleted the pkgconf-pypi branch March 7, 2025 02:46
@@ -144,6 +144,10 @@ def remove_output(*sources: str) -> Generator[None, None, None]:
stderr=sys.stderr,
)

# pkgconf-pypi needs pybind11/share/pkgconfig to be importable
Copy link

Choose a reason for hiding this comment

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

FYI, directories don't need a __init__.py to be importable. They are called namespace packages. They're slightly cursed, but they're a thing 😅

https://packaging.python.org/en/latest/guides/packaging-namespace-packages/#native-namespace-packages

Copy link
Contributor Author

@virtuald virtuald Mar 31, 2025

Choose a reason for hiding this comment

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

I'm pretty sure that I tried it and it didn't work. I believe namespace packages are only a thing at the top level package, not for subpackages? ... I don't remember why it didn't work, but it didn't work.

Copy link

@FFY00 FFY00 Mar 31, 2025

Choose a reason for hiding this comment

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

No, they should work for any. Perhaps the pkgconf-pypi code is broken, I was looking at it today, we'll see if I can push my changes and tests tomorrow.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think I've run into issues with importlib.resources on namespace packages, could be related?

Copy link

Choose a reason for hiding this comment

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

That would likely be because we only added support for namespace packages in importlib.resources in 3.10 (python/importlib_resources#196).

It shouldn't be affecting this since we aren't using importlib.resources. We did a while ago, but we were using the backport for < 3.10 (it was dropped in pypackaging-native/pkgconf-pypi@bb52fe1).

Copy link
Collaborator

Choose a reason for hiding this comment

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

I've had to add workarounds in #5598 to generate the __init__.py files, would be nice if I could get rid of them. :)

@henryiii henryiii changed the title Add pkgconf-pypi entrypoint feat(cmake): pkgconf-pypi entrypoint Mar 31, 2025
@henryiii henryiii removed the needs changelog Possibly needs a changelog entry label Apr 1, 2025
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.

3 participants