Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Explain how to add an extension module#1350

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
willingc merged 46 commits intopython:mainfrompicnixz:add-extensions-tutorial
Sep 11, 2024

Conversation

picnixz
Copy link
Member

@picnixzpicnixz commentedJul 13, 2024
edited
Loading

This is a first proposal for explaining how to add an extension module. Feel free to correct me, tell me that it's too verbose or some places need more information!

Fixes#317


📚 Documentation preview 📚:https://cpython-devguide--1350.org.readthedocs.build/developer-workflow/extension-modules/

@picnixz
Copy link
MemberAuthor

Thank you Hugo! do you have comments on the content itself, or parts that were not clear enough?

@hugovk
Copy link
Member

Thank you Hugo! do you have comments on the content itself, or parts that were not clear enough?

Content looks okay to me, but let's wait for someone who is more familiar with adding extension modules to CPython to also review.

By the way, we don't need to addgh- prefixes here to PR titles, they're only needed on the CPython for Bedevere to link issues to PRs and backports.

@picnixz
Copy link
MemberAuthor

Yes, that's what I observed afterwards. I thought thatmaybe there would be Bedevere that would update the issue automatically but in the end I needed to add the "Fix #..." myself.

hugovk and ezio-melotti reacted with thumbs up emoji

@hugovkhugovk changed the titlegh-317: explain how to add an extension moduleExplain how to add an extension moduleJul 14, 2024
@picnixz
Copy link
MemberAuthor

picnixz commentedJul 14, 2024
edited
Loading

I was reading what I wrote and I forgot something about the configure script and bootstrapping so I'll update the tutorial.

EDIT: won't have time today anymore!
EDIT 2: converting to a draft until I resolved my own thinking
EDIT 3: updated

ezio-melotti reacted with thumbs up emoji

@picnixzpicnixz marked this pull request as draftJuly 15, 2024 07:13
- use distinct names for files to highlight differences in configurations- be more precise on the terminology of an extension module- explain how to build a required module (always built-in) vs an optional module (built-in or dynamic)- address Ezio's review
@ncoghlan
Copy link
Contributor

OTOH, the onlyc prefix I can find iscmathmodule.c.

And thatc stands for "complex", not "implemented in C". Best to continue the underscore-prefixed convention (new C modules will typically be accelerators for pure Python modules anyway).

erlend-aasland reacted with eyes emoji

Copy link
Contributor

@ncoghlanncoghlan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This is mostly looking good, just some comments inline about header files and naming conventions.

ezio-melotti reacted with hooray emoji
@erlend-aasland
Copy link
Contributor

And thatc stands for "complex", not "implemented in C".

Oh, TIL! I never looked in that corner of the code base (yet) :)

@erlend-aasland
Copy link
Contributor

I don't know a lot about the limited API and I think the tutorial should not necessarily have too much things as an introduction.

It may make sense to separate out some concepts, such as the limited API, into how-to guides instead of baking them into the tutorial.

@encukou
Copy link
Member

I don't know a lot about the limited API and I think the tutorial should not necessarily have too much things as an introduction.

@vstinner has been converting modules to limited API, maybe he's a better person to argue that it should be in the initial example, as a default to start with :)
For starters, it makes the module friendlier to non-CPython implementations, as the limited API is (nowadays) designed to avoid CPython-specific details.
Removing the define gives you access to more API.

Py_BUILD_CORE_MODULE is kind of the opposite: gives you access to internals, which might change frequently. That coupling makes both the module and the internals harder to maintain, but you get more power and speed.

erlend-aasland reacted with thumbs up emoji

@picnixz
Copy link
MemberAuthor

picnixz commentedJul 20, 2024
edited
Loading

Thank you for the explanation on the Limited API! IIUC:

  • With Limited API +#include "Python.h" -> only expose a partial set of the API
  • only#include "Python.h" -> do not expose internals
  • Py_BUILD_CORE_MODULE +#include "Python.h" -> expose internals

Ideally, I don't want to duplicate the explanation on the macros themselves (I don't remember where it is) but I can a small paragraph summarizing the above? What remains to add on the page is (in separate PRs):

  • configuration variables (without explaining how to use autoconf syntax, I'll just link a reference to the autoconf page). Typically useful when adding stuff related to maths.
  • frozen modules, but I don't really know about them so I'll leave it to someone else (I didn't find any definition of them...)
erlend-aasland reacted with thumbs up emoji

@encukou
Copy link
Member

The limited API isdocumented publicly, but Py_BUILD_CORE_MODULE is internal. I think this would be a good place to mention it, since it's only useful for core modules. But, it doesn't need much more than that bullet point.

@picnixz
Copy link
MemberAuthor

I don't really want to add it in the main section because it could disrupt the reading flow.

So I think I can add something in the Troubleshooting section when the compiler complains withPy_BUILD_CORE being missing when you include some headers for some functions (and you don't know that you actually needed that macro). That way, people wouldn't add thePy_BUILD_CORE macro directly (the compiler message is literal "this requiresPy_BUILD_CORE and I think it's a bit misleading since it does notneedPy_BUILD_CORE itself but ratherPy_BUILD_CORE_MODULE).

Actually, there is this page:https://docs.python.org/3/using/configure.html#c-extensions which explains somewhat those macros. Should it be moved to the devguide? it also explains the difference with thePy_BUILD_CORE_BUILTIN macro.

(By the way, I don't like the naming "module" and "built-in" and I was confused a lot at first! I would have preferred something like "static" and "shared" because it's much more explanatory... but I think we cannot change this =/ (unless there are some other conditions I am not aware of)).

@encukou
Copy link
Member

Yes, I think mentioningPy_BUILD_CORE_BUILTIN in the user-facing documentation is a mistake. Users could read that page and think that they can definePy_BUILD_CORE_MODULE for their own dynamic libraries.
@vstinner, would you agree with moving the mentions ofPy_BUILD_CORE* macros fromconfigure docs to the devguide?

I don't like the naming "module" and "built-in" and I was confused a lot at first! I would have preferred something like "static" and "shared" because it's much more explanatory...

Maybe some of the confusion remains?
Most core extension modules can beeither linked statically or be a shared library. The person whobuilds Python, not the one that writes the module code, makes that choice.

IMO, the distinction isn't all that important, and it could moved all the way down to “Configuring the CPython project”, withSetup.stdlib.in treated as the normal andSetup.bootstrap.in as a less important special case.

The reasons for putting a moduleModules/Setup.bootstrap.in are:

  • the module defines C APIs that are used in core
  • the module is used by importlib, deepfreeze, freeze, runpy, or sysconfig
  • it's a “commonly used core module”

If you're adding your first module, these aren't very likely to apply.

A similar special case, then, is modules thatmust be built as shared libraries -- that's mostly modules that test the loading of shared libraries.

@vstinner
Copy link
Member

@vstinner, would you agree with moving the mentions of Py_BUILD_CORE* macros fromconfigure docs to the devguide?

I would prefer to keep the doc in the configure docs. You can duplicate the doc in the devguide if you want.

Copy link
Member

@vstinnervstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Nice work! LGTM.

@picnixz
Copy link
MemberAuthor

Maybe some of the confusion remains?

No.... the confusing is because I'm tired and I forgot about the*shared* and*static* markers... (I even wrote a section about them!!)

I'll add the paragraph on the macros. I'm waiting forpython/cpython#122544 to be decided because the troubleshooting section will need to be updated (and I'm taking holidays next week so it's better to wait for the correct image to be chosen before merging (it saves us 1 PR if we merge it before)).

Copy link
Collaborator

@willingcwillingc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Overall, fantastic job@picnixz. I've made several suggestions to help improve the flow and comprehension of the example. Thank you!

Copy link
Collaborator

@willingcwillingc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thank you@picnixz for a helpful addition to the devguide. You did a lovely job with this. I'm going to merge this and any further edits can be made in future PRs. 💐 ☀️

picnixz reacted with hooray emoji
@willingcwillingc merged commit3070b7c intopython:mainSep 11, 2024
4 checks passed
@picnixzpicnixz deleted the add-extensions-tutorial branchSeptember 11, 2024 14:09
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@encukouencukouencukou left review comments

@ncoghlanncoghlanncoghlan left review comments

@hugovkhugovkhugovk left review comments

@erlend-aaslanderlend-aaslanderlend-aasland left review comments

@ezio-melottiezio-melottiezio-melotti left review comments

@vstinnervstinnervstinner approved these changes

@willingcwillingcwillingc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add a section on how to add an extension module to the stdlib.
8 participants
@picnixz@hugovk@erlend-aasland@encukou@ncoghlan@vstinner@willingc@ezio-melotti

[8]ページ先頭

©2009-2025 Movatter.jp