-
Notifications
You must be signed in to change notification settings - Fork 64
Local regeneration of stubs via tox + Clang fixes #287
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
base: main
Are you sure you want to change the base?
Conversation
… matrix by mapping the env name) - do not cascade install prefixes for a pure-C++ demo library (fixes Mac OS issues) - fixes for misleading RTTI on Clang with anonymous namespaces
|
I decided to leave the test matrix intact, and just reuse the .sh scripts in the tox.ini. |
|
Thanks a lot for your work! Do you mind adding the "how do I test and regenerate stub files" as a new section in the README file for now? |
done |
…ubgen into feature/tests-via-tox
to development requirements
| ```shell | ||
| uv venv | ||
| source .venv/bin/activate | ||
| uv pip install -r requirements-dev.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be in the tox env only...
| uv pip install -r requirements-dev.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But that includes tox itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The uv tool install tox --with tox-uv line should add that now.
|
|
||
| During development, you may need to update the reference stubs in tests/stubs. This can be done locally using a convenience tox configuration. Ensure that all Python interpreters required by the test suite are available in your environment. For example, you can install them via uv: | ||
| ```shell | ||
| uv python install 3.10 3.11 3.12 3.13 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| uv python install 3.10 3.11 3.12 3.13 | |
| uv python install -r --managed-python 3.10 3.11 3.12 3.13 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree on the --managed-python flag, even though it should not matter much for the workflow. why -r though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I used -r/--reinstall to make sure it did a fresh install (of managed Pythons). Maybe not needed 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think --managed-python is technically also the default of uv from what I saw in the docs.
| flake8~=6.1.0 | ||
| isort==5.10.1 | ||
| numpy~=1.20 | ||
| scipy~=1.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| scipy~=1.0 | |
| pip | |
| scipy~=1.0 |
| py313-pb29-naa | ||
| py313-pb211-naa | ||
| py313-pb212-naa |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CMake logic in a few of these pybind versions is also a bit old and does not seem to work well with the tox/uv venvs -.-
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, let's see if downgrading CMake a little from 3.31 helps... I see
CMake Warning (dev) at /home/axel/src/pybind11-stubgen/tmp/pybind11-v2.13/install/share/cmake/pybind11/FindPythonLibsNew.cmake:101 (message):
Policy CMP0148 is not set: The FindPythonInterp and FindPythonLibs modules
are removed. Run "cmake --help-policy CMP0148" for policy details. Use
the cmake_policy command to set the policy and suppress this warning, or
preferably upgrade to using FindPython, either by calling it explicitly
before pybind11, or by setting PYBIND11_FINDPYTHON ON before pybind11.
Which was CMake 3.27+
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried adding a cmake pin to 3.21 in requirements-dev.txt and in tox.ini, removed from allowlist_externals, but does not help.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am on cmake version 4.2.1, I do see the warnings though, but it builds. I am sure we can somehow pin a specific version of cmake per pybind11 version in tox.
|
I am a bit stuck, cannot get this to compile with all Pythons on Ubuntu... |
| #include <demo/sublibA/ConsoleColors.h> | ||
|
|
||
| namespace { | ||
| namespace aliases_detail { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to pull those C++ fixes into a separate PR while we continue to iterate on the tox uv setup..
|
@sizmailov @gentlegiantJGC how are you updating the silver stub files? I have a hard time to refresh all stubs on Ubuntu 24.04 LTS but someone must have a workflow that works :D |
I do have a VPS with exactly the same ubuntu version, I will test today there myself. |
I was succesfully able to build this on Ubuntu 24.04 with a few extra steps, prior to install. Eigen is needed to build the demo project. so cmake == 3.28.3 |
|
@ax3l When I were working on a feature/issue I was mostly focused on single set of stubs. I've made a CI step to produce set of patches on failure which I applied locally. Usually that was the last step on finalizing the PR. I think such setup works for a pet project but not for community-maintained. |
namespace { struct Dummy {}; }in multiple translation units is treated as type redefinition by pybind11 on Clang (presumably due to mangling rules)