GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Explain how to add an extension module by picnixz · Pull Request #1350 · python/devguide

Conversation

@picnixz

@picnixz picnixz commented Jul 13, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
picnixz added 10 commits July 13, 2024 21:13
@picnixz
explain how to add modules
086ded3
@picnixz
Update extension-modules.rst
96fb998
@picnixz
Update extension-modules.rst
5f8797c
@picnixz
Update extension-modules.rst
e5d41f8
@picnixz
Update extension-modules.rst
b740114
@picnixz
Update extension-modules.rst
c74df67
@picnixz
Update extension-modules.rst
e645869
@picnixz
Update extension-modules.rst
5a9a57c
@picnixz
Update extension-modules.rst
26a18eb
@picnixz
Update extension-modules.rst
1b4d73f
hugovk
hugovk reviewed Jul 14, 2024
View reviewed changes
Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

@picnixz
Address Hugo's feedback
94086dd
@picnixz

picnixz commented Jul 14, 2024

Copy link
Copy Markdown
Member Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

picnixz added 2 commits July 14, 2024 11:44
@picnixz
Update extension-modules.rst
9a78a3b
@picnixz
Update extension-modules.rst
e22c278
@hugovk

hugovk commented Jul 14, 2024

Copy link
Copy Markdown
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 add gh- prefixes here to PR titles, they're only needed on the CPython for Bedevere to link issues to PRs and backports.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz

picnixz commented Jul 14, 2024

Copy link
Copy Markdown
Member Author

Yes, that's what I observed afterwards. I thought that maybe 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
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hugovk hugovk changed the title gh-317: explain how to add an extension module Explain how to add an extension module Jul 14, 2024
ezio-melotti
ezio-melotti reviewed Jul 14, 2024
View reviewed changes
Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

@picnixz

picnixz commented Jul 14, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

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
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz
picnixz marked this pull request as draft July 15, 2024 07:13
picnixz added 9 commits July 15, 2024 13:43
@picnixz
improvements
1f51497
- 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
@picnixz
fixup! sphinx
bdf09e5
@picnixz
fixup! indents
ef7cf3e
@picnixz
fixup! warnings
1a405ba
@picnixz
improve sections
e39cbc2
@picnixz
fix markup
35f207f
@picnixz
improve titles
316b00d
@picnixz
improve presentation
523dece
@picnixz
fixup! markup
d1cdd1d
picnixz added 2 commits July 18, 2024 15:23
@picnixz
add podman tip
7d6c8d6
@picnixz
address rest of the review
01c25bc
@ncoghlan

ncoghlan commented Jul 18, 2024

Copy link
Copy Markdown
Contributor

OTOH, the only c prefix I can find is cmathmodule.c.

And that c 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
All reactions
  • 👀 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

picnixz
picnixz commented Jul 18, 2024
View reviewed changes
Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

ncoghlan
ncoghlan reviewed Jul 18, 2024
View reviewed changes

@ncoghlan ncoghlan left a comment

Copy link
Copy Markdown
Contributor

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ezio-melotti reacted with hooray emoji
All reactions
  • 🎉 1 reaction
Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

picnixz added 2 commits July 18, 2024 17:47
@picnixz
address Alyssa's review
56910fb
@picnixz
add details
3ee1cea
@erlend-aasland

erlend-aasland commented Jul 18, 2024

Copy link
Copy Markdown
Contributor

And that c stands for "complex", not "implemented in C".

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@erlend-aasland

erlend-aasland commented Jul 18, 2024

Copy link
Copy Markdown
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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@encukou

encukou commented Jul 19, 2024

Copy link
Copy Markdown
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
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz

picnixz commented Jul 20, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

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
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@encukou

encukou commented Aug 1, 2024

Copy link
Copy Markdown
Member

The limited API is documented 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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz

picnixz commented Aug 1, 2024

Copy link
Copy Markdown
Member Author

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 with Py_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 the Py_BUILD_CORE macro directly (the compiler message is literal "this requires Py_BUILD_CORE and I think it's a bit misleading since it does not need Py_BUILD_CORE itself but rather Py_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 the Py_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)).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@encukou

encukou commented Aug 2, 2024

Copy link
Copy Markdown
Member

Yes, I think mentioning Py_BUILD_CORE_BUILTIN in the user-facing documentation is a mistake. Users could read that page and think that they can define Py_BUILD_CORE_MODULE for their own dynamic libraries.
@vstinner, would you agree with moving the mentions of Py_BUILD_CORE* macros from configure 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 be either linked statically or be a shared library. The person who builds 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”, with Setup.stdlib.in treated as the normal and Setup.bootstrap.in as a less important special case.

The reasons for putting a module Modules/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 that must be built as shared libraries -- that's mostly modules that test the loading of shared libraries.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner

vstinner commented Aug 2, 2024

Copy link
Copy Markdown
Member

@vstinner, would you agree with moving the mentions of Py_BUILD_CORE* macros from configure 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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner
vstinner approved these changes Aug 2, 2024
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

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.

Nice work! LGTM.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@picnixz

picnixz commented Aug 2, 2024

Copy link
Copy Markdown
Member Author

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 for python/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)).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

picnixz added 7 commits August 4, 2024 11:40
@picnixz
address review
3d235f4
- Add details on `Py_BUILD_CORE_*` macros
- Add tips for `Py_LIMITED_API`
@picnixz
Make it easier to update the required ubuntu version
7b0b234
@picnixz
Merge remote-tracking branch 'upstream/main' into add-extensions-tuto…
7fa94bf
…rial

# Conflicts:
#	conf.py
@picnixz
fixup!
ec48fdb
@picnixz
fixup!
1843e3d
@picnixz
improve comment
18c4d91
@picnixz
use double quotes instead of single quotes
8224bbd
@picnixz picnixz mentioned this pull request Aug 21, 2024
gh-121445: C implementation for fnmatch.translate python/cpython#123181
Closed
willingc
willingc reviewed Sep 10, 2024
View reviewed changes

@willingc willingc left a comment

Copy link
Copy Markdown
Collaborator

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread developer-workflow/extension-modules.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

@picnixz
Address Carol's review.
2f63053
willingc
willingc approved these changes Sep 11, 2024
View reviewed changes

@willingc willingc left a comment

Copy link
Copy Markdown
Collaborator

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.

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. 💐 ☀️

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

picnixz reacted with hooray emoji
All reactions
  • 🎉 1 reaction
@willingc
willingc merged commit 3070b7c into python:main Sep 11, 2024
@picnixz
picnixz deleted the add-extensions-tutorial branch September 11, 2024 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@encukou encukou encukou left review comments
@ncoghlan ncoghlan ncoghlan left review comments
@hugovk hugovk hugovk left review comments
@erlend-aasland erlend-aasland erlend-aasland left review comments
@ezio-melotti ezio-melotti ezio-melotti left review comments
@vstinner vstinner vstinner approved these changes
@willingc willingc willingc approved these changes

Uh oh!

There was an error while loading. Please reload this page.

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.

Uh oh!

There was an error while loading. Please reload this page.

8 participants

@picnixz @hugovk @erlend-aasland @encukou @ncoghlan @vstinner @willingc @ezio-melotti
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.