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

bpo-41861: Clean up sqlite3 header files wrt. PEP 384 #22419

Closed

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Sep 26, 2020

@erlend-aasland
Copy link
Contributor Author

@corona10, would you mind doing a review of this?

@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Oct 5, 2020

@vstinner, would you mind taking a look at this. After this cleanup, I think we can close bpo-41861.

@erlend-aasland erlend-aasland changed the title bpo-41861: Clean up sqlite3 header files bpo-41861: Clean up sqlite3 header files wrt. PEP 384 Oct 5, 2020
@erlend-aasland
Copy link
Contributor Author

Seems like the Travis CI is stuck…

@vstinner
Copy link
Member

vstinner commented Oct 6, 2020

PyMemberDef not part of the stable API

structmember.h is part of the limited C API. If it should not, I suggest to first exclude it from the limited C API, before changing _sqlite3 header files.

@erlend-aasland
Copy link
Contributor Author

PyMemberDef not part of the stable API

structmember.h is part of the limited C API. If it should not, I suggest to first exclude it from the limited C API, before changing _sqlite3 header files.

If it was, it would be sufficient to include Python.h, no?

@vstinner
Copy link
Member

vstinner commented Oct 6, 2020

I'm not sure why some header files like frameobject.h are not included by Python.h. Not being included by Python.h doesn't mean excluded from the limited C API. Or maybe I misunderstood something.

cc @serhiy-storchaka

@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Oct 6, 2020

I'm not sure why some header files like frameobject.h are not included by Python.h. Not being included by Python.h doesn't mean excluded from the limited C API. Or maybe I misunderstood something.

From PEP 384: "Applications shall only include the header file Python.h (before including any system headers), or, optionally, include pyconfig.h, and then Python.h."

I was under the impression that the "stable C API" and the "limited C API" was the same thing, however, that might not be the case?

@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Oct 6, 2020

PyMemberDef not part of the stable API

structmember.h is part of the limited C API. If it should not, I suggest to first exclude it from the limited C API, before changing _sqlite3 header files.

I think that the "not part of the stable API" comment was a reference to bpo-2897. According to PEP 384, PyMemberDef is indeed a part of the stable (limited?) C API, however PyMemberDef is defined in structmember.h, and PEP 384 states that you're allowed to include Python.h only. Referenced in the bpo, there's a (huge) PR (#20462) that deprecates structmember.h and moves its content to descrobject.h.

The current situation is such that PEP 384 is self-contradicting when it comes to header files and structures. However, I guess that PEP 384 compliance is not as strict for built-in modules as it is for third party modules?

So, should bpo-2897 be resolved first before continuing with this PR?

@erlend-aasland
Copy link
Contributor Author

Guessing that this PR is on hold until this is sorted out, @vstinner. Perhaps the bpo can be closed without this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants