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

[shihai1991]

https://bugs.python.org/issue43009

• Issue: Port curses capi pointer array to a struct #87175

[vstinner]

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 ;-)

[methane]

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

[vstinner]

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]

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

[vstinner]

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]

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.

[encukou]

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]

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]

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]

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]

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

[vstinner]

(oops, I closed the PR by mistake.)

[github-actions]

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

[vstinner]

The capsule object and the related C API is consumed by _curses_panel.c. It's a way to expose an API in Modules/_cursesmodule.c and use it in Modules/_curses_panel.c. The _curses_panel extension is separated since it's linked to the panel library (ex: libpanelw.so.6 on Linux), whereas the _curses extension is linked to curses (ex: libncursesw.so.6 on Linux).

I'm not sure what it was good idea at the beginning to make such API public. But well, when it was added 23 years ago (commit 0c7a0bd), we cared less about what should be public or private. The C API was mostly consumed by CPython itself.

I suggest to make the capsule object private and move the related API to the internal C API.

In Python 3.10, I directly removed the ucnhash_CAPI attribute of the unicodedata extension: I made it private (unicodedata._ucnhash_CAPI) and moved its C API to the internal C API. This work was related to convert the unicodedata to the multi-phase initialization and convert static types to heap types: see #86323 I would expect that there are more unicodedata users than _curses users, for their C API.

[vstinner]

These APIs are not tested, not documented, and only used by _cursesmodule.c and _curses_panel.c (in CPython):

• PyCursesWindow_Type
• PyCursesSetupTermCalled
• PyCursesInitialised
• PyCursesInitialisedColor

CPython:

$ git grep -l -E '(PyCursesWindow_Type|PyCursesSetupTermCalled|PyCursesInitialised|PyCursesInitialisedColor)'
Include/py_curses.h
Modules/_curses_panel.c
Modules/_cursesmodule.c
Modules/clinic/_curses_panel.c.h
Modules/clinic/_cursesmodule.c.h
Tools/c-analyzer/cpython/globals-to-fix.tsv

They are not used by any of the PyPI top 5000 projects. cffi has a reference but only in a comment:

$ ./search_pypi_top.py PYPI-2023-04-13/ '(PyCursesWindow_Type|PyCursesSetupTermCalled|PyCursesInitialised|PyCursesInitialisedColor)' -o curses -q
PYPI-2023-04-13/cffi-1.15.1.tar.gz: cffi-1.15.1/demo/_curses.py: # PyCursesInitialised;
PYPI-2023-04-13/cffi-1.15.1.tar.gz: cffi-1.15.1/demo/_curses.py: # PyCursesInitialised;
Found 2 matching lines in 1 projects

[shihai1991]

Thanks for you comment. I am amazing that Victor checked the details again after two years :)
This not exposed by python.h and the outside user can use the macros to get the field by using the macros in the py_curses.h if they need it. Or removed it into internal CAPI is OK if we haven't the outside user(I am not sure).

I'm not sure what it was good idea at the beginning to make such API public. But well, when it was added 23 years ago (commit 0c7a0bd), we cared less about
what should be public or private. The C API was mostly consumed by CPython itself.

Maybe we can bother this PR's creator to make sure the original purpose. @akuchling

I will rebase it tomorrow.

[arhadthedev]

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]

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]

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]

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]

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.

[shihai1991]

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]

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]

@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]

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]

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]

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