-
Notifications
You must be signed in to change notification settings - Fork 4
build and publish a wheel #65
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
Conversation
|
Unfortunately, this module cannot be distributed as a wheel in a practical way. On installation, it dynamically generates a CMake config-file package to bind If we were to publish a wheel, the > cd pytest-cmake/
> python -m build --wheel .
> unzip -Z1 dist/*.whl
...
pytest_cmake-1.2.0.data/data/share/Pytest/cmake/FindPytest.cmake
pytest_cmake-1.2.0.data/data/share/Pytest/cmake/PytestAddTests.cmake
pytest_cmake-1.2.0.data/data/share/Pytest/cmake/PytestConfig.cmake
pytest_cmake-1.2.0.data/data/share/Pytest/cmake/PytestConfigVersion.cmakeIn this example, the > unzip -p dist/*.whl pytest_cmake-1.2.0.data/data/share/Pytest/cmake/PytestConfigVersion.cmake
...
set(PACKAGE_VERSION "9.0.2")
...
> pytest --version
pytest 8.4.1 |
|
I see. That probably suggests that this is work you should be doing at runtime rather than at install time, but perhaps that is a bigger change. Eg |
|
Additionally: wheel building uses an isolated virtual environment. There is no guarantee that the pytest used to build the wheel matches the pytest in a user environment. I expect that the wheel will typically build with the latest pytest, regardless of what version the user has in their own project. |
|
Ok, so I was completely wrong. The pytest version is actually fetched dynamically by the FindPytest.cmake module, and the We only generate a So TL;DR, yes, publishing a wheel absolutely makes sense! I’m currently on paternity leave, so it may take me a few days to update the installation process within your MR, I'll probably do it this weekend. Thanks for catching this! |
|
Hmm.. scratch that, I was mistaken in my last message. It turns out the config version file is actually required, and the version check is not handled entirely by the module. https://github.com/python-cmake/pytest-cmake/actions/runs/20665733377 There may be another approach to make this work cleanly, I’ll look into it and follow up here. |
Keep PytestConfigVersion.cmake for config-mode checks without hard-coding a build-time pytest version. Version discovery remains dynamic via FindPytest.cmake. PytestConfig.cmake stays a thin entry point. Wheel distribution is supported.
512c2e2 to
674c4ea
Compare
|
Alright, I got this working by shipping a dummy set(PACKAGE_VERSION_COMPATIBLE TRUE)
set(PACKAGE_VERSION_EXACT TRUE)This satisfies CMake’s config-mode version checks without hard-coding a build-time pytest version, so releasing a wheel should now work correctly. Thanks for flagging this! |
All installation in python is from wheel: so if maintainers do not publish wheels then every user install must begin by building one.
That has a few (minor) disadvantages: it is slower, it is a thing that can go wrong. Security-conscious users will not want to run arbitrary code at install time.
Therefore best practice - even for pure python projects - is to publish both sdist and also wheel.