Skip to content

gh-87175: Port curses capi pointer array to a struct. - #24304

Open
shihai1991 wants to merge 12 commits into
python:mainfrom
shihai1991:bpo_43009
Open

shihai1991 wants to merge 12 commits into
python:mainfrom
shihai1991:bpo_43009

Conversation

@shihai1991

@shihai1991 shihai1991 commented Jan 23, 2021 •

Copy link
Copy Markdown
Member

Comment thread Include/py_curses.h

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hum, this change is backward incompatible. I don't know how to proceed. Is this API used outside Python, in 3rd party projects?

@encukou @serhiy-storchaka @methane: Is this change reasonable?

Note: I asked @shihai1991 to write this change ;-)

Comment thread Include/py_curses.h Outdated
@methane

methane commented Jan 26, 2021

Copy link
Copy Markdown
Member

@vstinner

Copy link
Copy Markdown
Member

https://foss.heptapod.net/pypy/cffi/-/blob/cea8f1260587a4cf6ad230a00148cabea0f61572/demo/_curses.py#L972

It's a demo and it's commented, so it seems ok.

Moreover, the demo uses PyCursesInitialised; which is not affected by this change.

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please document the incompatible change with a NEWS entry (blurb) and document in C API > Porting to Python 3.10 of:
https://docs.python.org/dev/whatsnew/3.10.html#id2

Comment thread Modules/_cursesmodule.c Outdated
@vstinner

Copy link
Copy Markdown
Member

The only user I found is this.
https://foss.heptapod.net/pypy/cffi/-/blob/cea8f1260587a4cf6ad230a00148cabea0f61572/demo/_curses.py#L972

@methane: Did you check for PyCurses_API and PyCurses_API_pointers usage?

@shihai1991

Copy link
Copy Markdown
Member Author

The only user I found is this.
https://foss.heptapod.net/pypy/cffi/-/blob/cea8f1260587a4cf6ad230a00148cabea0f61572/demo/_curses.py#L972

@methane: Did you check for PyCurses_API and PyCurses_API_pointers usage?

I use https://searchcode.com/ to check who will use it. But I am not find the useful info in here.

Comment thread Doc/whatsnew/3.10.rst Outdated
@encukou

encukou commented Jan 26, 2021

Copy link
Copy Markdown
Member

Hum, this change is backward incompatible. I don't know how to proceed.

Well, PEP 387 is quite clear on this one, I think. PyCurses_API is obviously an API, so it can't be changed without a deprecation warning period.
IMO, this should be done by adding a new capsule and deprecating the old static one.

@shihai1991

Copy link
Copy Markdown
Member Author

Hum, this change is backward incompatible. I don't know how to proceed.

Well, PEP 387 is quite clear on this one, I think. PyCurses_API is obviously an API, so it can't be changed without a deprecation warning period.
IMO, this should be done by adding a new capsule and deprecating the old static one.

Hm, it's more friendly to users. What's your suggestion? victor :) @vstinner

@serhiy-storchaka

Copy link
Copy Markdown
Member

Why do you want to do this?

If we will do that, I think it is safe to replace an array with a structure as long as offsets of structure fields matches offsets of array items. I believe it is true on all supported platforms, because all fields are pointers. PyCurses_API_pointers should be kept for compatibility (but deprecated).

@vstinner

Copy link
Copy Markdown
Member

Why do you want to do this?

I proposed this change (and @shihai1991 implemented it) since I don't see a raw array of pointers as an "API".

If we will do that, I think it is safe to replace an array with a structure as long as offsets of structure fields matches offsets of array items. I believe it is true on all supported platforms, because all fields are pointers. PyCurses_API_pointers should be kept for compatibility (but deprecated).

Even if PyCurses_API_pointers (4) is part of the public Include/py_curses.h header file (and it doesn't exclude anything from the limited C API), it seems like the API is mostly used through these 4 macros:

#define PyCursesWindow_Type (*(PyTypeObject *) PyCurses_API[0])
#define PyCursesSetupTermCalled  {if (! ((int (*)(void))PyCurses_API[1]) () ) return NULL;}
#define PyCursesInitialised      {if (! ((int (*)(void))PyCurses_API[2]) () ) return NULL;}
#define PyCursesInitialisedColor {if (! ((int (*)(void))PyCurses_API[3]) () ) return NULL;}

Not directly through static void **PyCurses_API;.

Maybe this API was only exposed for the _curses_panel module and is not intended to be used outside Python.

@vstinner

Copy link
Copy Markdown
Member

Note: Include/py_curses.h is not included by Python.h.

@vstinner vstinner closed this Jan 27, 2021
@vstinner vstinner reopened this Jan 27, 2021
@vstinner

Copy link
Copy Markdown
Member

(oops, I closed the PR by mistake.)

@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Feb 27, 2021
@arhadthedev

Copy link
Copy Markdown
Member

I will rebase it tomorrow.

Could you target Doc/whatsnew/3.13.rst, please? 3.12 no longer accepts new features because of final preparations for 3.12b1.

@vstinner

Copy link
Copy Markdown
Member

The least risky approach is to:

  • Add a new internal C API, maybe using a struct rather than an array.
  • Deprecate the old C API for 2 Python releases.

@shihai1991

Copy link
Copy Markdown
Member Author

I will rebase it tomorrow.

Could you target Doc/whatsnew/3.13.rst, please? 3.12 no longer accepts new features because of final preparations for 3.12b1.

So 3.12 is prepare to release? I missed a lot of things recently.

@arhadthedev

Copy link
Copy Markdown
Member

So 3.12 is prepare to release? I missed a lot of things recently.

Moreover, its release date was moved (python/peps#3139) from this Monday to May 22.

@erlend-aasland

Copy link
Copy Markdown
Contributor

So 3.12 is prepare to release? I missed a lot of things recently.

No, only the beta (feature freeze). The release is not until this fall.

Anyway, I'm not sure how PEP-387 plays with stuff that's not exposed by Python.h.

Conflicts:
	Doc/whatsnew/3.10.rst
	Include/py_curses.h
@shihai1991
shihai1991 requested a review from a team as a code owner May 16, 2023 19:03
@shihai1991

Copy link
Copy Markdown
Member Author

So 3.12 is prepare to release? I missed a lot of things recently.

Moreover, its release date was moved (python/peps#3139) from this Monday to May 22.

I will rebase it tomorrow.

Could you target Doc/whatsnew/3.13.rst, please? 3.12 no longer accepts new features because of final preparations for 3.12b1.

Hi, I updated this PR(add the internal APIs) and the old APIs in py_curses.h hasn't been removed. So I think I add a note in 3.12 will be ok?

@arhadthedev

arhadthedev commented May 17, 2023 •

Copy link
Copy Markdown
Member

So I think I add a note in 3.12 will be ok?

Probably yes, however, it's up to the core developers to decide.

Tests / Check if generated files are up to date (pull_request) Failing after 7m

checking analysis results...
Include/internal/pycore_curses.h   	-                                  	PyCurses_C_API                          	> not supported (mutable)           	static PyCurses_CAPI*
-------------------------
total failures: 1
done checking
Categorized by storage:


static-module-global
  Include/internal/pycore_curses.h   	-                                  	PyCurses_C_API
subtotal: 1

@erlend-aasland Should we store the new PyCurses_CAPI in the module state or the interpreter state?

@shihai1991

Copy link
Copy Markdown
Member Author

@erlend-aasland Should we store the new PyCurses_CAPI in the module state or the interpreter state?

It's not common variable in interpreter(just the curses_panel extension module use this Capsule API), so saved in interpreter state is OK.

@erlend-aasland

Copy link
Copy Markdown
Contributor

It's not common variable in interpreter(just the curses_panel extension module use this Capsule API), so saved in interpreter state is OK.

I assume you meant module state, no?

@shihai1991

Copy link
Copy Markdown
Member Author

It's not common variable in interpreter(just the curses_panel extension module use this Capsule API), so saved in interpreter state is OK.

I assume you meant module state, no?

Oh, thanks for correcting my wrong writing.

In addition, Use the ignored.csv to ignore the warning maybe OK in this case.

@github-actions

github-actions Bot commented Apr 9, 2026

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 9, 2026
@bedevere-app bedevere-app Bot added the type-feature A feature request or enhancement label Oct 22, 2026
@read-the-docs-community

Copy link
Copy Markdown

Documentation build overview

📚 cpython-previews | 🛠️ Build #35337813 | 📁 Comparing 40e296d against main (c6661bd)

  🔍 Preview build  

4 files changed
± library/shutil.html
± whatsnew/3.12.html
± whatsnew/3.16.html
± whatsnew/changelog.html

@github-actions github-actions Bot removed the stale Stale PR or inactive for long period of time. label Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting core review type-feature A feature request or enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.