Skip to content
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

tests: add ABI check #4282

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Conversation

henryiii
Copy link
Collaborator

Description

Adding tests for ABI. Just a very basic implementation at the moment. Didn't add it the tests because we don't run nox on most jobs. Maybe could move it to pytest-virtualenv.

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
@EthanSteinberg
Copy link
Collaborator

One suggestion: Have this test assert that the sizeof()s all the major structures are the same

So internals, type_info, instance, etc.

Even better would be to compare the raw bytes and verify those are equivalent.

(Simply write the bytes of interals to a file or something and then compare the files).

@henryiii
Copy link
Collaborator Author

Ahh, can't do an ABI check against and older version of pybind11 and Python 3.11, duh. :)

/tmp/tmpco8vd2na/.env/lib/python3.11/site-packages/pybind11/include/pybind11/detail/type_caster_base.h:473:36: error: invalid use of incomplete type ‘PyFrameObject’ {aka ‘struct _frame’}
[232](https://github.com/pybind/pybind11/actions/runs/3316298068/jobs/5477937033#step:25:233)
        473 |                 "  " + handle(frame->f_code->co_filename).cast<std::string>() +
[233](https://github.com/pybind/pybind11/actions/runs/3316298068/jobs/5477937033#step:25:234)
            |                                    ^~

One suggestion

Hmm, interesting, okay, I'll try to add that soon. Won't have too much time for a while, though.

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.

2 participants