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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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. :)

Path("pybind11/share/__init__.py").touch()
Path("pybind11/share/pkgconfig/__init__.py").touch()

txt = get_and_replace(setup_py, version=version, extra_cmd=extra_cmd)
code = compile(txt, setup_py, "exec")
exec(code, {"SDist": SDist})
2 changes: 2 additions & 0 deletions tests/extra_python_package/test_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@
"commands.py",
"py.typed",
"setup_helpers.py",
"share/__init__.py",
"share/pkgconfig/__init__.py",
}

headers = main_headers | conduit_headers | detail_headers | eigen_headers | stl_headers
Expand Down
6 changes: 5 additions & 1 deletion tools/setup_main.py.in
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ setup(
"pybind11.include.pybind11.detail",
"pybind11.include.pybind11.eigen",
"pybind11.include.pybind11.stl",
"pybind11.share",
"pybind11.share.cmake.pybind11",
"pybind11.share.pkgconfig",
],
Expand All @@ -40,7 +41,10 @@ setup(
],
"pipx.run": [
"pybind11 = pybind11.__main__:main",
]
],
"pkg_config": [
"pybind11 = pybind11.share.pkgconfig",
],
},
cmdclass=cmdclass
)
Loading